Re: [PATCH] printk: rename vprintk_func to vprintk

From: John Ogness
Date: Tue Mar 30 2021 - 09:00:29 EST


On 2021-03-30, Petr Mladek <pmladek@xxxxxxxx> wrote:
> On Tue 2021-03-23 15:42:01, Rasmus Villemoes wrote:
>> The printk code is already hard enough to understand. Remove an
>> unnecessary indirection by renaming vprintk_func to vprintk (adding
>> the asmlinkage annotation), and removing the vprintk definition from
>> printk.c. That way, printk is implemented in terms of vprintk as one
>> would expect, and there's no "vprintk_func, what's that? Some function
>> pointer that gets set where?"
>>
>> The declaration of vprintk in linux/printk.h already has the
>> __printf(1,0) attribute, there's no point repeating that with the
>> definition - it's for diagnostics in callers.
>>
>> linux/printk.h already contains a static inline {return 0;} definition
>> of vprintk when !CONFIG_PRINTK.
>>
>> Since the corresponding stub definition of vprintk_func was not marked
>> "static inline", any translation unit including internal.h would get a
>> definition of vprintk_func - it just so happens that for
>> !CONFIG_PRINTK, there is precisely one such TU, namely printk.c. Had
>> there been more, it would be a link error; now it's just a silly waste
>> of a few bytes of .text, which one must assume are rather precious to
>> anyone disabling PRINTK.
>>
>> $ objdump -dr kernel/printk/printk.o
>> 00000330 <vprintk_func>:
>> 330: 31 c0 xor %eax,%eax
>> 332: c3 ret
>> 333: 8d b4 26 00 00 00 00 lea 0x0(%esi,%eiz,1),%esi
>> 33a: 8d b6 00 00 00 00 lea 0x0(%esi),%esi
>>
>> Signed-off-by: Rasmus Villemoes <linux@xxxxxxxxxxxxxxxxxx>
>
> Nice clean up!
>
> Reviewed-by: Petr Mladek <pmladek@xxxxxxxx>
>
> John,
>
> it conflicts with the patchset removing printk safe buffers[1].
> Would you prefer to queue this into the patchset?
> Or should I push it into printk/linux.git, printk-rework and you would
> base v2 on top of it?

Please push it to printk-rework. I will base my v2 on top of it.

Thanks.

John