Messages in this thread | | | Subject | Re: [PATCH v6 5/9] vsprintf: Factor out %pV handler as va_format() | From | Rasmus Villemoes <> | Date | Tue, 12 Feb 2019 21:22:39 +0100 |
| |
On 12/02/2019 18.58, Joe Perches wrote: > On Tue, 2019-02-12 at 14:00 +0100, Petr Mladek wrote: >> On Fri 2019-02-08 09:11:17, Joe Perches wrote: >>> On Fri, 2019-02-08 at 16:23 +0100, Petr Mladek wrote: >>>> Move the code from the long pointer() function. We are going to improve >>>> error handling that will make it more complicated. >>>> >>>> This patch does not change the existing behavior. >>> >>> But doesn't this increase stack use? >>> %pV is recursive and increasing the stack is undesired >>> for this use. >> >> %pV handler is stack sensitive because the entire vsnprintf() >> machinery is called recursively. This one extra call does >> not make it much worse. > > That's an argument?. > > Refactoring is good, but you need to add > __always_inline here. > >
No. No no no. Please at least try to send the code through a compiler. Petr's patch actually makes things uniformly better, since gcc ends up doing a tail call from pointer().
2e65: 4c 89 cf mov %r9,%rdi 2e68: e9 23 05 00 00 jmpq 3390 <va_format>
$ scripts/stackusage -o before.su lib/vsprintf.o $ cp lib/vsprintf.o lib/vsprintf.o.0 $ # apply patch $ scripts/stackusage -o after.su lib/vsprintf.o $ cp lib/vsprintf.o lib/vsprintf.o.1 $ scripts/stackdelta before.su after.su ./lib/vsprintf.c pointer 40 8 -32 $ grep va_format after.su after.su:./lib/vsprintf.c:1437 va_format 40 static # Icing $ scripts/bloat-o-meter lib/vsprintf.o.0 lib/vsprintf.o.1 add/remove: 1/0 grow/shrink: 0/1 up/down: 77/-186 (-109) Function old new delta va_format - 77 +77 pointer 634 448 -186
Of course, all of this depends very much on compiler version, architecture (if the calling convention says to pass arguments on stack, tail calls are a lot harder to come by), CONFIG_STACKPROTECTOR etc. etc., but nothing suggests that forcing the va_format logic into pointer() would be a good thing.
Rasmus
| |