public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Joe Perches <joe@perches.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Nick Andrew <nick@nick-andrew.net>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Greg Kroah-Hartman <gregkh@suse.de>,
	netdev@vger.kernel.org, Bjorn Helgaas <bjorn.helgaas@hp.com>
Subject: Re: [PATCH] vsprintf.c: Reduce sizeof struct printf_spec from 24 to 8 bytes
Date: Sat, 6 Mar 2010 18:03:33 -0800 (PST)	[thread overview]
Message-ID: <alpine.LFD.2.00.1003061755400.23804@localhost.localdomain> (raw)
In-Reply-To: <1267924214.1937.12.camel@Joe-Laptop.home>



On Sat, 6 Mar 2010, Joe Perches wrote:
> 
> Reducing the size of struct printf_spec is a good thing
> because multiple instances are commonly passed on stack.

Sadly, this is not enough.

the 'pointer()' function has a 200+ byte stack footprint on x86-64. And 
vsnprintf itself is about 100+ bytes. So that stack depth is way bigger 
than I would have expected.

I'm not sure _why_ that stack footprint for 'pointer()' is so big, but I 
bet it's due to some simple inlining, and gcc (once more) sucking at it 
and not being able to combine stack frames. It's a damn shame.

I suspect it's mac_address_string() having 20-odd bytes of temporary data, 
ip6_compressed_string() having some more, ip6_addr_string() having 50 
bytes, uuid_string() adding thirty-odd bytes etc. Inline them all, suck up 
merging stack slots, and 200 bytes is easy.

So no, I don't think we can do the recursion as things stand. I've applied 
your cleanup patch, along with the two from Bjorn Helgaas (which he did 
for his pnp set), but they don't help this fundamental problem.

A few noinlines might be appropriate. As would a good gcc cluestick about 
inlining and stack usage. The latter is unlikely to materialize, I guess.

		Linus

  reply	other threads:[~2010-03-07  2:03 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <cover.1267682641.git.joe@perches.com>
2010-03-04  6:27 ` [PATCH 1/2] vsprintf: Recursive vsnprintf: Add "%pV", struct va_format Joe Perches
2010-03-04  6:27 ` [PATCH 2/2] device.h drivers/base/core.c Convert dev_<level> macros to functions Joe Perches
2010-03-04 22:38 ` [RESEND PATCH 0/2] Make functions of dev_<level> macros, recursive vsnprintf Andrew Morton
2010-03-04 23:06   ` Linus Torvalds
2010-03-06 21:36     ` Joe Perches
2010-03-06 22:03       ` Linus Torvalds
2010-03-06 22:30         ` Joe Perches
2010-03-06 22:52           ` Linus Torvalds
2010-03-06 22:57             ` Linus Torvalds
2010-03-06 23:35               ` Joe Perches
2010-03-06 23:46                 ` Linus Torvalds
2010-03-06 23:48                   ` Linus Torvalds
2010-03-06 23:57                     ` Joe Perches
2010-03-06 23:58                     ` Linus Torvalds
2010-03-07  1:10                       ` [PATCH] vsprintf.c: Reduce sizeof struct printf_spec from 24 to 8 bytes Joe Perches
2010-03-07  2:03                         ` Linus Torvalds [this message]
2010-03-07  2:24                           ` Linus Torvalds
2010-03-07  2:33                           ` [PATCH] vsprintf.c: Use noinline_for_stack Joe Perches
2010-03-08 23:39                             ` Joe Perches
2010-03-13  0:25                             ` Andrew Morton
2010-03-13 15:35                               ` Linus Torvalds
2010-03-13 17:44                                 ` Joe Perches
2010-03-13 19:54                                   ` [PATCH] vsprintf.c: remove stack variable ksym from Joe Perches
2010-03-15 15:01                                     ` Paulo Marques
     [not found] ` <f2b24d484347c083fa87856a75c1d96102af9005.1267682641.git.joe@perches.com>
2010-03-05  0:56   ` [PATCH 2/2] device.h drivers/base/core.c Convert dev_<level> macros to functions Andrew Morton
2010-03-05  1:00     ` Andrew Morton
2010-03-05  2:46       ` Joe Perches

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=alpine.LFD.2.00.1003061755400.23804@localhost.localdomain \
    --to=torvalds@linux-foundation.org \
    --cc=akpm@linux-foundation.org \
    --cc=bjorn.helgaas@hp.com \
    --cc=gregkh@suse.de \
    --cc=joe@perches.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=nick@nick-andrew.net \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox