From patchwork Thu Jan 5 19:19:24 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Ian Campbell X-Patchwork-Id: 134538 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 48FB7B6FBE for ; Fri, 6 Jan 2012 06:19:32 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932459Ab2AETT1 (ORCPT ); Thu, 5 Jan 2012 14:19:27 -0500 Received: from smtp.ctxuk.citrix.com ([62.200.22.115]:21814 "EHLO SMTP.EU.CITRIX.COM" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932235Ab2AETT0 (ORCPT ); Thu, 5 Jan 2012 14:19:26 -0500 X-IronPort-AV: E=Sophos;i="4.71,463,1320624000"; d="scan'208";a="9845264" Received: from lonpmailmx01.citrite.net ([10.30.203.162]) by LONPIPO01.EU.CITRIX.COM with ESMTP/TLS/RC4-MD5; 05 Jan 2012 19:19:25 +0000 Received: from [127.0.0.1] (10.80.16.67) by smtprelay.citrix.com (10.30.203.162) with Microsoft SMTP Server id 8.3.213.0; Thu, 5 Jan 2012 19:19:25 +0000 Subject: Re: [PATCH 2/6] net: pad skb data and shinfo as a whole rather than individually From: Ian Campbell To: Eric Dumazet CC: "netdev@vger.kernel.org" , David Miller In-Reply-To: <1325785607.4759.12.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC> References: <1325783399.25206.413.camel@zakaz.uk.xensource.com> <1325783624-14601-2-git-send-email-ian.campbell@citrix.com> <1325785607.4759.12.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC> Organization: Citrix Systems, Inc. Date: Thu, 5 Jan 2012 19:19:24 +0000 Message-ID: <1325791164.29084.8.camel@dagon.hellion.org.uk> MIME-Version: 1.0 X-Mailer: Evolution 2.32.3 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Thu, 2012-01-05 at 17:46 +0000, Eric Dumazet wrote: > Le jeudi 05 janvier 2012 à 17:13 +0000, Ian Campbell a écrit : > > diff --git a/net/core/skbuff.c b/net/core/skbuff.c > > index da0c97f..b6de604 100644 > > --- a/net/core/skbuff.c > > +++ b/net/core/skbuff.c > > @@ -189,8 +189,7 @@ struct sk_buff *__alloc_skb(unsigned int size, gfp_t gfp_mask, > > * aligned memory blocks, unless SLUB/SLAB debug is enabled. > > * Both skb->head and skb_shared_info are cache line aligned. > > */ > > - size = SKB_DATA_ALIGN(size); > > - size += SKB_DATA_ALIGN(sizeof(struct skb_shared_info)); > > + size = SKB_DATA_ALIGN(size + sizeof(struct skb_shared_info)); > > data = kmalloc_node_track_caller(size, gfp_mask, node); > > if (!data) > > goto nodata; > > Unfortunately this makes the frequently use part of skb_shared_info not > any more in a single cache line. > > This will slow down many operations like kfree_skb() of cold skbs (TX > completion for example) I've been toying with the following which moves the infrequently used destructor_arg field to the front. With 32 or 64 byte cache lines this is then the only field which ends up on a different cache line. With 128 byte cache lines it is all already on the same line. Does that alleviate your concern? If so I can clean it up and include it in the series. Cheers, Ian. --- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h index cbc815c..533c2ea 100644 --- a/include/linux/skbuff.h +++ b/include/linux/skbuff.h @@ -249,6 +249,11 @@ struct ubuf_info { * the end of the header data, ie. at skb->end. */ struct skb_shared_info { + /* Intermediate layers must ensure that destructor_arg + * remains valid until skb destructor */ + void * destructor_arg; + + /* Warning all fields from here until dataref are cleared in __alloc_skb() */ unsigned char nr_frags; __u8 tx_flags; unsigned short gso_size; @@ -264,9 +269,6 @@ struct skb_shared_info { */ atomic_t dataref; - /* Intermediate layers must ensure that destructor_arg - * remains valid until skb destructor */ - void * destructor_arg; /* must be last field, see pskb_expand_head() */ skb_frag_t frags[MAX_SKB_FRAGS]; diff --git a/net/core/skbuff.c b/net/core/skbuff.c index e942a86..592b8a8 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -219,7 +219,10 @@ struct sk_buff *__alloc_skb(unsigned int size, gfp_t gfp_mask, /* make sure we initialize shinfo sequentially */ shinfo = skb_shinfo(skb); - memset(shinfo, 0, offsetof(struct skb_shared_info, dataref)); + + memset(&shinfo->nr_frags, 0, + offsetof(struct skb_shared_info, dataref) + - offsetof(struct skb_shared_info, nr_frags)); atomic_set(&shinfo->dataref, 1); kmemcheck_annotate_variable(shinfo->destructor_arg);