Re: [PATCH v6 2/4] dax/bus: Use guard(device) in sysfs attribute helpers

[gregkh@xxxxxxxxxxxxxxxxxxx]

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