LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: "Ilpo Järvinen" <ilpo.jarvinen@helsinki.fi>
To: Jan Engelhardt <jengelh@computergmbh.de>
Cc: Patrick McHardy <kaber@trash.net>,
	Netdev <netdev@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	David Miller <davem@davemloft.net>,
	Arnaldo Carvalho de Melo <acme@redhat.com>
Subject: Re: [RFC PATCH 3/8] [NET]: uninline dev_alloc_skb, de-bloats a lot
Date: Thu, 21 Feb 2008 00:18:49 +0200 (EET)	[thread overview]
Message-ID: <Pine.LNX.4.64.0802202135080.32171@kivilampi-30.cs.helsinki.fi> (raw)
In-Reply-To: <Pine.LNX.4.64.0802201730240.26109@fbirervta.pbzchgretzou.qr>

On Wed, 20 Feb 2008, Jan Engelhardt wrote:

> 
> On Feb 20 2008 17:27, Patrick McHardy wrote:
> >> Striking. How can this even happen? A callsite which calls
> >> 
> >>  dev_alloc_skb(n)
> >> 
> >> is just equivalent to
> >> 
> >>  __dev_alloc_skb(n, GFP_ATOMIC);
> >> 
> >> which means there's like 4 (or 8 if it's long) bytes more on the
> >> stack. For a worst case, count in another 8 bytes for push and pop or mov on
> >> the stack. But that still does not add up to 23 kb.

I think you misunderstood the results, if I uninlined dev_alloc_skb(), it 
_alone_ was uninlined which basically means that __dev_alloc_skb() that is 
inline as well is included inside that uninlined function.

When both were inlined, they add up to everywhere, and uninlining 
dev_alloc_skb alone mitigates that for both(!) of them in every place 
where dev_alloc_skb is being called. Because __dev_alloc_skb call sites 
are few, most benefits show up already with dev_alloc_skb uninlining 
alone. On the other hand, if __dev_alloc_skb is uninlined, the size 
reasoning you used above applies to dev_alloc_skb callsites, and that
is definately less than 23kB.

> > __dev_alloc_skb() is also an inline function which performs
> > some extra work. Which raises the question - if dev_alloc_skb()
> > is uninlined, shouldn't __dev_alloc_skb() be uninline as well?

Of course that could be done as well, however, I wouldn't be too keen to 
deepen callchain by both of them, ie., uninlined dev_alloc_skb would just 
contain few bytes which perform the call to __dev_alloc_skb which has the 
bit larger content due to that "extra work". IMHO the best solution would 
duplicate the "extra work" to both of them on binary level (obviously 
not on the source level), e.g., by adding static inline ___dev_alloc_skb() 
to .h which is inlined to both of the variants. I'm not too sure if inline 
to __dev_alloc_skb() alone is enough in .c file to result in inlining of 
__dev_alloc_skb to dev_alloc_skb (with all gcc versions and relevant 
optimization settings).

> I'd like to see the results when {__dev_alloc_skb is externed
> and dev_alloc_skb remains inlined}.

The results are right under your nose already... ;-)
See from the list of the series introduction:

  http://marc.info/?l=linux-netdev&m=120351526210711&w=2

IMHO more interesting number (which I currently don't have) is the 
_remaining_ benefits of uninlining __dev_alloc_skb after
dev_alloc_skb was first uninlined.


-- 
 i.

  reply	other threads:[~2008-02-20 22:19 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-20 13:47 [RFC PATCH 0/8]: uninline & uninline Ilpo Järvinen
2008-02-20 13:47 ` [RFC PATCH 1/8] [NET]: uninline skb_put, de-bloats a lot Ilpo Järvinen
2008-02-20 13:47   ` [RFC PATCH 2/8] [NET]: uninline skb_pull, " Ilpo Järvinen
2008-02-20 13:47     ` [RFC PATCH 3/8] [NET]: uninline dev_alloc_skb, " Ilpo Järvinen
2008-02-20 13:47       ` [RFC PATCH 4/8] [NET]: uninline skb_push, " Ilpo Järvinen
2008-02-20 13:47         ` [RFC PATCH 5/8] [NET]: uninline dst_release Ilpo Järvinen
2008-02-20 13:47           ` [RFC PATCH 6/8] [NET]: uninline skb_trim, de-bloats Ilpo Järvinen
2008-02-20 13:47             ` [RFC PATCH 7/8] [SCTP]: uninline sctp_add_cmd_sf Ilpo Järvinen
2008-02-20 13:47               ` [RFC PATCH 8/8] Jhash in too big for inlining, move under lib/ Ilpo Järvinen
2008-02-23  8:02                 ` Andrew Morton
2008-02-23 10:05                   ` Ilpo Järvinen
2008-02-23 18:21                     ` Andrew Morton
2008-02-23 13:06                   ` Andi Kleen
2008-02-20 22:16               ` [RFC PATCH 7/8] [SCTP]: uninline sctp_add_cmd_sf Vlad Yasevich
2008-02-20 22:34                 ` Ilpo Järvinen
2008-02-21 15:27                   ` Vlad Yasevich
2008-02-20 16:19       ` [RFC PATCH 3/8] [NET]: uninline dev_alloc_skb, de-bloats a lot Jan Engelhardt
2008-02-20 16:27         ` Patrick McHardy
2008-02-20 16:30           ` Jan Engelhardt
2008-02-20 22:18             ` Ilpo Järvinen [this message]
2008-03-12 15:27               ` Ilpo Järvinen
2008-02-20 13:54   ` [RFC PATCH 1/8] [NET]: uninline skb_put, " Patrick McHardy
2008-02-20 13:57   ` Eric Dumazet
2008-02-23  8:02 ` [RFC PATCH 0/8]: uninline & uninline Andrew Morton
2008-02-23 10:11   ` Ilpo Järvinen
2008-02-23 13:15   ` Andi Kleen
2008-02-23 18:06     ` Ilpo Järvinen
2008-02-23 18:55     ` Andrew Morton
2008-02-23 19:58       ` Hua Zhong
2008-02-23 21:02         ` Andi Kleen
2008-02-27 19:08       ` profile-likely patch (was " Valdis.Kletnieks

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Pine.LNX.4.64.0802202135080.32171@kivilampi-30.cs.helsinki.fi \
    --to=ilpo.jarvinen@helsinki.fi \
    --cc=acme@redhat.com \
    --cc=davem@davemloft.net \
    --cc=jengelh@computergmbh.de \
    --cc=kaber@trash.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --subject='Re: [RFC PATCH 3/8] [NET]: uninline dev_alloc_skb, de-bloats a lot' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).