Received-SPF: pass (google.com: domain of linux-kernel+bounces-1274-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Fri, 15 Dec 2023 17:14:07 +0100
From: "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>
To: "Verma, Vishal L" <vishal.l.verma@intel.com>
Cc: "willy@infradead.org" <willy@infradead.org>,
	"david@redhat.com" <david@redhat.com>,
	"joao.m.martins@oracle.com" <joao.m.martins@oracle.com>,
	"Jiang, Dave" <dave.jiang@intel.com>,
	"akpm@linux-foundation.org" <akpm@linux-foundation.org>,
	"osalvador@suse.de" <osalvador@suse.de>,
	"linux-cxl@vger.kernel.org" <linux-cxl@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Williams, Dan J" <dan.j.williams@intel.com>,
	"dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
	"Huang, Ying" <ying.huang@intel.com>,
	"nvdimm@lists.linux.dev" <nvdimm@lists.linux.dev>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>
Subject: Re: [PATCH v6 2/4] dax/bus: Use guard(device) in sysfs attribute
 helpers
Message-ID: <2023121547-goldfish-snowplow-345f@gregkh>
References: <20231214-vv-dax_abi-v6-0-ad900d698438@intel.com>
 <20231214-vv-dax_abi-v6-2-ad900d698438@intel.com>
 <ZXvqjIqvcGkumNXf@casper.infradead.org>
 <d3eef0c19e949825ad0990a03eb55cc5f3033720.camel@intel.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <d3eef0c19e949825ad0990a03eb55cc5f3033720.camel@intel.com>

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