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

On Wed, 2012-01-25 at 12:51 +0000, Eric Dumazet wrote:

Le mercredi 25 janvier 2012 à 12:27 +0000, Ian Campbell a écrit :

quoted

This reduces the minimum overhead required for this allocation such that the
shinfo can be grown in the following patch without overflowing 2048 bytes for a
1500 byte frame.

Reducing this overhead while also growing the shinfo means that sometimes the
tail end of the data can end up in the same cache line as the beginning of the
shinfo. Specifically in the case of the 64 byte cache lines on a 64 bit system
the first 8 bytes of shinfo can overlap the tail cacheline of the data. In many
cases the allocation slop means that there is no overlap.

Signed-off-by: Ian Campbell <redacted>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <redacted>
---

Hmm... you missed build_skb()

Presumably build_skb should be using SKB_WITH_OVERHEAD instead of open
coding it? (which I think is how I managed to miss it)

 and all places we use
SKB_DATA_ALIGN(sizeof(struct skb_shared_info))

(for example in some drivers)

I'm not sure how I missed these with my grep. Obviously it is necessary
to catch them all in order to avoid the performance drop you mention
below. Fortunately there is fewer than a dozen instances in half a dozen
drivers to check.

If you want to see possible performance impact of your changes, see
commit e52fcb2462ac (bnx2x: uses build_skb() in receive path), and
expect a drop from 820.000 pps to 720.000 pps

Can you elaborate on the specific benchmark you used there?

Thanks,
Ian