Re: + net-uninline-skb_put.patch added to -mm tree

Re: + net-uninline-skb_put.patch added to -mm tree

[David Miller]

From: akpm@osdl.org
Date: Fri, 03 Nov 2006 14:18:29 -0800

> Subject: net: uninline skb_put()
> From: Andrew Morton <akpm@osdl.org>
> 
> It has 34 callsites for a total of 2650 bytes.
> 
> Cc: Arnaldo Carvalho de Melo <acme@conectiva.com.br>
> Signed-off-by: Andrew Morton <akpm@osdl.org>

A more accurate figure would probably be:

davem@sunset:~/src/GIT/net-2.6$ git grep skb_put | grep -v __skb_put | wc -l
1167

:-)

Half of the cost of this interface are the assertions, which while
useful are obviously over the top for such an oft-used routine in
packet processing.

Without the assertion checks it's merely:

	unsigned char *tmp = skb->tail;
	skb->tail += len;
	skb->len  += len;
	return tmp;

And even with 1167 call sites that is definitely something which
should be inlined.

Re: + net-uninline-skb_put.patch added to -mm tree

[Andrew Morton]

On Mon, 06 Nov 2006 14:42:33 -0800 (PST)
David Miller <davem@davemloft.net> wrote:

> From: akpm@osdl.org
> Date: Fri, 03 Nov 2006 14:18:29 -0800
> 
> > Subject: net: uninline skb_put()
> > From: Andrew Morton <akpm@osdl.org>
> > 
> > It has 34 callsites for a total of 2650 bytes.
> > 
> > Cc: Arnaldo Carvalho de Melo <acme@conectiva.com.br>
> > Signed-off-by: Andrew Morton <akpm@osdl.org>
> 
> A more accurate figure would probably be:
> 
> davem@sunset:~/src/GIT/net-2.6$ git grep skb_put | grep -v __skb_put | wc -l
> 1167
> 
> :-)

True.  I'm not sure what .config Arnaldo was using..

> Half of the cost of this interface are the assertions, which while
> useful are obviously over the top for such an oft-used routine in
> packet processing.
> 
> Without the assertion checks it's merely:
> 
> 	unsigned char *tmp = skb->tail;
> 	skb->tail += len;
> 	skb->len  += len;
> 	return tmp;
> 
> And even with 1167 call sites that is definitely something which
> should be inlined.

Yes.

Tricky.  I guess one suitable approach would be to create a standalone
skb-debugging config option.  There's quite a lot of debug stuff in there
which could be made conditional on that option.

But otoh, skb-debugging finds bugs.

Re: + net-uninline-skb_put.patch added to -mm tree

[Arnaldo Carvalho de Melo]

On Mon, Nov 06, 2006 at 02:57:57PM -0800, Andrew Morton wrote:
> On Mon, 06 Nov 2006 14:42:33 -0800 (PST)
> David Miller <davem@davemloft.net> wrote:
> 
> > From: akpm@osdl.org
> > Date: Fri, 03 Nov 2006 14:18:29 -0800
> > 
> > > Subject: net: uninline skb_put()
> > > From: Andrew Morton <akpm@osdl.org>
> > > 
> > > It has 34 callsites for a total of 2650 bytes.
> > > 
> > > Cc: Arnaldo Carvalho de Melo <acme@conectiva.com.br>
> > > Signed-off-by: Andrew Morton <akpm@osdl.org>
> > 
> > A more accurate figure would probably be:
> > 
> > davem@sunset:~/src/GIT/net-2.6$ git grep skb_put | grep -v __skb_put | wc -l
> > 1167
> > 
> > :-)
> 
> True.  I'm not sure what .config Arnaldo was using..

The first one, for the top50 was a normal config for a qemu machine, not
sure right now if there was debugging, it may well be the case, the
second one, for a top100, as said in the message was with 'make
allyesconfig', as I was testing how scalable was the thing in the
current state, and as David Woodhouse and Adrian Bunk said it is
unrealistic, best thing is to built it with the config distros ship +
debug info, which I'll do in the next few days.

- Arnaldo