[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2023121547-goldfish-snowplow-345f@gregkh>
Date: Fri, 15 Dec 2023 17:14:07 +0100
From: "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>
To: "Verma, Vishal L" <vishal.l.verma@...el.com>
Cc: "willy@...radead.org" <willy@...radead.org>,
"david@...hat.com" <david@...hat.com>,
"joao.m.martins@...cle.com" <joao.m.martins@...cle.com>,
"Jiang, Dave" <dave.jiang@...el.com>,
"akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
"osalvador@...e.de" <osalvador@...e.de>,
"linux-cxl@...r.kernel.org" <linux-cxl@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"Williams, Dan J" <dan.j.williams@...el.com>,
"dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
"Huang, Ying" <ying.huang@...el.com>,
"nvdimm@...ts.linux.dev" <nvdimm@...ts.linux.dev>,
"linux-mm@...ck.org" <linux-mm@...ck.org>
Subject: Re: [PATCH v6 2/4] dax/bus: Use guard(device) in sysfs attribute
helpers
On Fri, Dec 15, 2023 at 06:33:58AM +0000, Verma, Vishal L wrote:
> On Fri, 2023-12-15 at 05:56 +0000, Matthew Wilcox wrote:
> > On Thu, Dec 14, 2023 at 10:25:27PM -0700, Vishal Verma wrote:
> > > @@ -294,13 +294,10 @@ static ssize_t available_size_show(struct device *dev,
> > > struct device_attribute *attr, char *buf)
> > > {
> > > struct dax_region *dax_region = dev_get_drvdata(dev);
> > > - unsigned long long size;
> > >
> > > - device_lock(dev);
> > > - size = dax_region_avail_size(dax_region);
> > > - device_unlock(dev);
> > > + guard(device)(dev);
> > >
> > > - return sprintf(buf, "%llu\n", size);
> > > + return sprintf(buf, "%llu\n", dax_region_avail_size(dax_region));
> > > }
> >
> > Is this an appropriate use of guard()? sprintf is not the fastest of
> > functions, so we will end up holding the device_lock for longer than
> > we used to.
>
> Hi Matthew,
>
> Agreed that we end up holding the lock for a bit longer in many of
> these. I'm inclined to say this is okay, since these are all user
> configuration paths through sysfs, not affecting any sort of runtime
> performance.
Why does the lock have to be taken at all? You have a valid reference,
isn't that all you need?
thanks,
greg k-h
Powered by blists - more mailing lists