linux-kernel - Re: [PATCH v2] iommu/amd: Fix extended features logging

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <84e443d76c4c450770ade83aa48cc631c101d383.camel@perches.com>
Date:   Mon, 19 Apr 2021 14:59:59 -0700
From:   Joe Perches <joe@...ches.com>
To:     Alexander Monakov <amonakov@...ras.ru>
Cc:     linux-kernel@...r.kernel.org, Paul Menzel <pmenzel@...gen.mpg.de>,
        Joerg Roedel <jroedel@...e.de>,
        Suravee Suthikulpanit <suravee.suthikulpanit@....com>,
        iommu@...ts.linux-foundation.org
Subject: Re: [PATCH v2] iommu/amd: Fix extended features logging

On Mon, 2021-04-19 at 22:23 +0300, Alexander Monakov wrote:
> On Sun, 11 Apr 2021, Joe Perches wrote:
> 
> > > v2: avoid pr_info(""), change pci_info() to pr_info() for a nicer
> > > solution
> > > 
> > >  drivers/iommu/amd/init.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> > > index 596d0c413473..62913f82a21f 100644
> > > --- a/drivers/iommu/amd/init.c
> > > +++ b/drivers/iommu/amd/init.c
> > > @@ -1929,8 +1929,8 @@ static void print_iommu_info(void)
> > >  		pci_info(pdev, "Found IOMMU cap 0x%hx\n", iommu->cap_ptr);
> > >  
> > > 
> > >  		if (iommu->cap & (1 << IOMMU_CAP_EFR)) {
> > > -			pci_info(pdev, "Extended features (%#llx):",
> > > -				 iommu->features);
> > > +			pr_info("Extended features (%#llx):", iommu->features);
> > > +
> > >  			for (i = 0; i < ARRAY_SIZE(feat_str); ++i) {
> > >  				if (iommu_feature(iommu, (1ULL << i)))
> > >  					pr_cont(" %s", feat_str[i]);
> > 
> > How about avoiding all of this by using a temporary buffer
> > and a single pci_info.
> 
> I think it is mostly up to the maintainers, but from my perspective, it's not
> good to conflate such a simple bugfix with the substantial rewrite you are
> proposing (which also increases code complexity).

You and I have _significant_ differences in the definition of substantial.

Buffering the output is the preferred code style in preference to
pr_cont.

Do remember pr_cont should _only_ be used when absolutely necessary
as interleaving of other messages from other processes/threads can
and does occur.