linux-kernel - Re: [PATCH RESEND] fpga: dfl: omit unneeded casts of u64 values for dev

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <1956417c03e8cf5b51c2edd3342b6c0b657ac0a1.camel@intel.com>
Date: Tue, 2 Apr 2024 19:24:00 +0000
From: "Colberg, Peter" <peter.colberg@...el.com>
To: "yilun.xu@...ux.intel.com" <yilun.xu@...ux.intel.com>
CC: "Xu, Yilun" <yilun.xu@...el.com>, "linux-fpga@...r.kernel.org"
	<linux-fpga@...r.kernel.org>, "mdf@...nel.org" <mdf@...nel.org>,
	"andriy.shevchenko@...ux.intel.com" <andriy.shevchenko@...ux.intel.com>, "Wu,
 Hao" <hao.wu@...el.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "russ.weight@...ux.dev"
	<russ.weight@...ux.dev>, "Pagani, Marco" <marpagan@...hat.com>, "Rix, Tom"
	<trix@...hat.com>, "matthew.gerlach@...ux.intel.com"
	<matthew.gerlach@...ux.intel.com>
Subject: Re: [PATCH RESEND] fpga: dfl: omit unneeded casts of u64 values for
 dev_dbg()

On Tue, 2024-04-02 at 11:29 +0800, Xu Yilun wrote:
> On Thu, Mar 28, 2024 at 08:04:29PM -0400, Peter Colberg wrote:
> > Omit unneeded casts of u64 values to unsigned long long for use with
> > printk() format specifier %llx. Unlike user space, the kernel defines
> > u64 as unsigned long long for all architectures; see commit 2a7930bd77fe
> > ("Documentation/printk-formats.txt: No casts needed for u64/s64").
> 
> The change is OK. But I suggest just delete the unnecessary dev_dbg()
> since now people normally don't want these "Hello, I'm here!" info.

Do you have specific commits in mind as a precedent that remove similar
occurrences? This patch was intended to be cosmetic, whereas dropping
dev_dbg() would introduce functional changes in a strict sense.

> > 
> > These changes are cosmetic only; no functional changes.
> > 
> > Suggested-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
> > Signed-off-by: Peter Colberg <peter.colberg@...el.com>
> > ---
> > This is an unmodified resend of the second patch only from the series
> > "fpga: dfl: clean up string formatting for sysfs_emit() and dev_dbg()".
> 
> Why only pick this patch out of the series?

The two patches were cleanup patches but otherwise unrelated. I still
have to carry out the perf testing of the other patch as you suggested.

> 
> [...]
> 
> > diff --git a/drivers/fpga/dfl-fme-mgr.c b/drivers/fpga/dfl-fme-mgr.c
> > index ab228d8837a0..da3cb9c35de5 100644
> > --- a/drivers/fpga/dfl-fme-mgr.c
> > +++ b/drivers/fpga/dfl-fme-mgr.c
> > @@ -150,7 +150,7 @@ static int fme_mgr_write_init(struct fpga_manager *mgr,
> >  	priv->pr_error = fme_mgr_pr_error_handle(fme_pr);
> >  	if (priv->pr_error)
> >  		dev_dbg(dev, "previous PR error detected %llx\n",
> > -			(unsigned long long)priv->pr_error);
> > +			priv->pr_error);
> 
> I'm not sure if this is an real problem. Maybe we could keep it.
> 
> >  
> >  	dev_dbg(dev, "set PR port ID\n");
> >  
> > @@ -242,8 +242,7 @@ static int fme_mgr_write_complete(struct fpga_manager *mgr,
> >  	dev_dbg(dev, "PR operation complete, checking status\n");
> >  	priv->pr_error = fme_mgr_pr_error_handle(fme_pr);
> >  	if (priv->pr_error) {
> > -		dev_dbg(dev, "PR error detected %llx\n",
> > -			(unsigned long long)priv->pr_error);
> > +		dev_dbg(dev, "PR error detected %llx\n", priv->pr_error);
> 
> This is a real problem, is it? Change to dev_err()?

Thanks; yes, dev_err() seems reasonable and is consistent with the
earlier dev_err() after readq_poll_timeout().

Peter

> 
> Thanks,
> Yilun
> 
> >  		return -EIO;
> >  	}
> >  
> > -- 
> > 2.44.0
> > 
> >