On 2021-04-11, Alexander Monakov <[email protected]> wrote:
>>> The second line is emitted via 'pr_cont', which causes it to have a
>>> different ('warn') loglevel compared to the previous line ('info').
>>>
>>> Commit 9a295ff0ffc9 attempted to rectify this by removing the newline
>>> from the pci_info format string, but this doesn't work, as pci_info
>>> calls implicitly append a newline anyway.
>>
>> Hmm, did I really screw that up during my testing? I am sorry about that.
>>
>> I tried to wrap my head around, where the newline is implicitly appended, and
>> only found the definitions below.
>>
>> include/linux/pci.h:#define pci_info(pdev, fmt, arg...)
>> dev_info(&(pdev)->dev, fmt, ##arg)
>>
>> include/linux/dev_printk.h:#define dev_info(dev, fmt, ...)
>> \
>> include/linux/dev_printk.h: _dev_info(dev, dev_fmt(fmt),
>> ##__VA_ARGS__)
>>
>> include/linux/dev_printk.h:__printf(2, 3) __cold
>> include/linux/dev_printk.h:void _dev_info(const struct device *dev, const
>> char *fmt, ...);
>>
>> include/linux/compiler_attributes.h:#define __printf(a, b)
>> __attribute__((__format__(printf, a, b)))
>
> Yeah, it's not obvious: it happens in kernel/printk/printk.c:vprintk_store
> where it does
>
> if (dev_info)
> lflags |= LOG_NEWLINE;
>
> It doesn't seem to be documented; I think it prevents using pr_cont with
> "rich" printk facilities that go via _dev_info.
>
> I suspect it quietly changed in commit c313af145b9bc ("printk() - isolate
> KERN_CONT users from ordinary complete lines").
Yes, this behavior has been around for a while. I see no reason why it
should be that way. These days printk does not care if there is dev_info
included or not.
>> In the discussion *smpboot: CPU numbers printed as warning* [1] John wrote:
>>
>>> It is supported to provide loglevels for CONT messages. The loglevel is
>>> then only used if the append fails:
>>>
>>> pr_cont(KERN_INFO "message part");
>>>
>>> I don't know if we want to go down that path. But it is supported.
>
> Yeah, I saw that, but decided to go with the 'pr_info("")' solution, because
> it is less magic, and already used in two other drivers.
Note that what I was suggesting was to fix a different issue: If the
pr_cont() caller is interrupted by another printk user, then the
following pr_cont() calls will use the default loglevel. By explicitly
specifying the loglevel in pr_cont(), you can be sure that those pieces
get the desired loglevel, even if those pieces get separated off because
of an interrupting printk user.
So even if we fix dev_info to allow pr_cont continuation, it still may
be desirable to specify the loglevel in the pr_cont pieces.
> pr_info("") will also prepend 'AMD-Vi:' to the feature list, which is
> nice.
I'd rather fix dev_info callers to allow pr_cont and then fix any code
that is using this workaround.
And if the print maintainers agree it is OK to encourage
pr_cont(LOGLEVEL "...") usage, then people should really start using
that if the loglevel on those pieces is important.
John Ogness
