Re: [PATCH 2/6] net: pad skb data and shinfo as a whole rather than individually

On Fri, 2012-01-06 at 12:33 +0000, Eric Dumazet wrote:

Le vendredi 06 janvier 2012 à 11:20 +0000, Ian Campbell a écrit :

quoted

It doesn't fit in a single cache line today.

It really does, thanks to your (net: pack skb_shared_info more
efficiently) previous patch.

I dont understand your numbers, very hard to read.

Current net-next :

offsetof(struct skb_shared_info, nr_frags)=0x0
offsetof(struct skb_shared_info, frags[1])=0x40   (0x30 on 32bit arches)

I see 0x48 here at cset 6386994e03ebbe60338ded3d586308a41e81c0dc:
(gdb) print &((struct skb_shared_info *)0)->nr_frags
$1 = (short unsigned int *) 0x0
(gdb) print &((struct skb_shared_info *)0)->frags[1]
$2 = (skb_frag_t *) 0x48
(gdb) print &((struct skb_shared_info *)0)->frags[0]
$3 = (skb_frag_t *) 0x38

(it's 0x34 and 0x2c on 32 bit) and these numbers match what I posted for
v3.1 (and I imagine earlier since this stuff doesn't seem to change very
often).

I provided the offsets of each field in struct skb_shared_info, which
one do you think is wrong?

Remember that the end shared info is explicitly aligned (to the end of
the allocation == a cache line) while the front just ends up at wherever
the size dictates so you can't measure the alignment of the fields from
the beginning  of the struct, you need to measure from the end.

Ian.

So _all_ fields, including one frag, are included in a single cache line
on most machines (64-bytes cache line), IF struct skb_shared_info is
aligned.

Your patch obviously breaks this on 64bit arches, unless you make sure
sizeof(struct skb_shared_info) is a multiple of cache line.

[BTW, it is incidentaly the case after your 1/6 patch]

fields reordering is not going to change anything on this.

Or maybe I misread your patch ?

At least you claimed in Changelog : 

<quote>
 Reducing this overhead means that sometimes the tail end of
 the data can end up in the same cache line as the beginning of the shinfo but
 in many cases the allocation slop means that there is no overlap.
</quote>