Re: [PATCH] iommu/amd: Fix extended features logging

From: Alexander Monakov
Date: Sun Apr 11 2021 - 07:22:19 EST


On Sun, 11 Apr 2021, Paul Menzel 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").

> 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.

pr_info("") will also prepend 'AMD-Vi:' to the feature list, which is nice.

Best regards,
Alexander