Messages in this thread | | | Date | Wed, 15 Jun 2022 11:09:35 +0200 | Subject | Re: [PATCH v3 03/33] vsprintf: Convert to printbuf | From | Rasmus Villemoes <> |
| |
On 04/06/2022 21.30, Kent Overstreet wrote: > This converts vsnprintf() to printbufs: instead of passing around raw > char * pointers for current buf position and end of buf, we have a real > type! > > This makes the calling convention for our existing pretty printers a lot > saner and less error prone, plus printbufs add some new helpers that > make the code smaller and more readable, with a lot less crazy pointer > arithmetic. > > There are a lot more refactorings to be done: this patch tries to stick > to just converting the calling conventions, as that needs to be done all > at once in order to avoid introducing a ton of wrappers that will just > be deleted. > > Thankfully we have good unit tests for printf, and they have been run > and are all passing with this patch.
So, as the primary author of those tests, a somewhat active contributor to vsprintf.c and being listed as R: for both files, why wasn't I cc'ed on this?
Anyway, my main concern with this is that performance goes down the drain and the generated code will be awful. Have you done any measurements and/or looked at disassembly? Thanks to -fno-strict-aliasing (or perhaps just because we're writing through a char* pointer which IIRC may alias anything), I think the compiler will be forced to reload prt->pos and prt->size over and over and over. I may be wrong, of course, that happens often. Perhaps __restrict could help, IDK.
> --- > include/linux/kernel.h | 4 +
Please don't expand that dumping ground. Please, if printbufs will become a thing (whether or not vsprintf internally will be refactored to use them), add a new linux/printf.h where these things can go, and the declarations of vsprintf() and close friends can eventually be moved.
Rasmus
| |