From: Johannes Thumshirn <jthumshirn@suse.de> To: Dan Williams <dan.j.williams@intel.com> Cc: linux-kernel@vger.kernel.org, linux-nvdimm@lists.01.org Subject: Re: [PATCH 5/8] dax: refactor locking out of size calculation routines Date: Wed, 14 Dec 2016 16:01:30 +0100 [thread overview] Message-ID: <20161214150130.GE6279@linux-x5ow.site> (raw) In-Reply-To: <148143773119.10950.13840765929901351011.stgit@dwillia2-desk3.amr.corp.intel.com> Hi Dan, On Sat, Dec 10, 2016 at 10:28:51PM -0800, Dan Williams wrote: > In preparation for other callers of these routines make the locking the > responsibility of the caller. > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > --- > drivers/dax/dax.c | 30 ++++++++++++++++++++++++------ > 1 file changed, 24 insertions(+), 6 deletions(-) > > diff --git a/drivers/dax/dax.c b/drivers/dax/dax.c > index d878a56cf3e3..5b65eaff6ace 100644 > --- a/drivers/dax/dax.c > +++ b/drivers/dax/dax.c > @@ -90,11 +90,11 @@ static unsigned long long dax_region_avail_size( > unsigned long long size; > struct resource *res; > > - mutex_lock(&dax_region->lock); > + WARN_ON_ONCE(!mutex_is_locked(&dax_region->lock)); I'd prefer it a lockdep_assert_held(&dex_region->lock). This of cause has the drawback that it won't trigger w/o lockdep but enabled, but I don't think it's the responibility of a production kernel to have this warnings anyway. On the flip side you get all the lockdep beauty for free with it. > + > size = resource_size(&dax_region->res); > for_each_dax_region_resource(dax_region, res) > size -= resource_size(res); > - mutex_unlock(&dax_region->lock); > > return size; > } [...] > +static unsigned long long dax_dev_size(struct dax_dev *dax_dev) > { > - struct dax_dev *dax_dev = to_dax_dev(dev); > + struct dax_region *dax_region = dax_dev->region; > unsigned long long size = 0; > int i; > > + WARN_ON_ONCE(!mutex_is_locked(&dax_region->lock)); > + See above > for (i = 0; i < dax_dev->num_resources; i++) > size += resource_size(dax_dev->res[i]); > > + return size; > +} > + Byte, Johannes _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
WARNING: multiple messages have this Message-ID (diff)
From: Johannes Thumshirn <jthumshirn@suse.de> To: Dan Williams <dan.j.williams@intel.com> Cc: linux-nvdimm@ml01.01.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 5/8] dax: refactor locking out of size calculation routines Date: Wed, 14 Dec 2016 16:01:30 +0100 [thread overview] Message-ID: <20161214150130.GE6279@linux-x5ow.site> (raw) In-Reply-To: <148143773119.10950.13840765929901351011.stgit@dwillia2-desk3.amr.corp.intel.com> Hi Dan, On Sat, Dec 10, 2016 at 10:28:51PM -0800, Dan Williams wrote: > In preparation for other callers of these routines make the locking the > responsibility of the caller. > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > --- > drivers/dax/dax.c | 30 ++++++++++++++++++++++++------ > 1 file changed, 24 insertions(+), 6 deletions(-) > > diff --git a/drivers/dax/dax.c b/drivers/dax/dax.c > index d878a56cf3e3..5b65eaff6ace 100644 > --- a/drivers/dax/dax.c > +++ b/drivers/dax/dax.c > @@ -90,11 +90,11 @@ static unsigned long long dax_region_avail_size( > unsigned long long size; > struct resource *res; > > - mutex_lock(&dax_region->lock); > + WARN_ON_ONCE(!mutex_is_locked(&dax_region->lock)); I'd prefer it a lockdep_assert_held(&dex_region->lock). This of cause has the drawback that it won't trigger w/o lockdep but enabled, but I don't think it's the responibility of a production kernel to have this warnings anyway. On the flip side you get all the lockdep beauty for free with it. > + > size = resource_size(&dax_region->res); > for_each_dax_region_resource(dax_region, res) > size -= resource_size(res); > - mutex_unlock(&dax_region->lock); > > return size; > } [...] > +static unsigned long long dax_dev_size(struct dax_dev *dax_dev) > { > - struct dax_dev *dax_dev = to_dax_dev(dev); > + struct dax_region *dax_region = dax_dev->region; > unsigned long long size = 0; > int i; > > + WARN_ON_ONCE(!mutex_is_locked(&dax_region->lock)); > + See above > for (i = 0; i < dax_dev->num_resources; i++) > size += resource_size(dax_dev->res[i]); > > + return size; > +} > + Byte, Johannes
next prev parent reply other threads:[~2016-12-14 15:01 UTC|newest] Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-12-11 6:28 [PATCH 0/8] device-dax: sub-division support Dan Williams 2016-12-11 6:28 ` Dan Williams 2016-12-11 6:28 ` [PATCH 1/8] dax: add region-available-size attribute Dan Williams 2016-12-11 6:28 ` Dan Williams 2016-12-14 14:38 ` Johannes Thumshirn 2016-12-14 14:38 ` Johannes Thumshirn 2016-12-14 15:53 ` Dan Williams 2016-12-14 15:53 ` Dan Williams 2016-12-15 6:47 ` Dan Williams 2016-12-15 6:47 ` Dan Williams 2016-12-11 6:28 ` [PATCH 2/8] dax: add region 'id', 'size', and 'align' attributes Dan Williams 2016-12-11 6:28 ` Dan Williams 2016-12-11 6:28 ` [PATCH 3/8] dax: register seed device Dan Williams 2016-12-11 6:28 ` Dan Williams 2016-12-11 6:28 ` [PATCH 4/8] dax: use multi-order radix for resource lookup Dan Williams 2016-12-11 6:28 ` Dan Williams 2016-12-11 6:28 ` [PATCH 5/8] dax: refactor locking out of size calculation routines Dan Williams 2016-12-11 6:28 ` Dan Williams 2016-12-14 15:01 ` Johannes Thumshirn [this message] 2016-12-14 15:01 ` Johannes Thumshirn 2016-12-14 15:55 ` Dan Williams 2016-12-14 15:55 ` Dan Williams 2016-12-11 6:28 ` [PATCH 6/8] dax: sub-division support Dan Williams 2016-12-11 6:28 ` Dan Williams 2016-12-11 6:29 ` [PATCH 7/8] dax: add / remove dax devices after provisioning Dan Williams 2016-12-11 6:29 ` Dan Williams 2016-12-11 6:29 ` [PATCH 8/8] dax: add debug for region available_size Dan Williams 2016-12-11 6:29 ` Dan Williams [not found] ` <148143770485.10950.13227732273892953675.stgit-p8uTFz9XbKj2zm6wflaqv1nYeNYlB/vhral2JQCrhuEAvxtiuMwx3w@public.gmane.org> 2016-12-12 17:15 ` [PATCH 0/8] device-dax: sub-division support Jeff Moyer 2016-12-12 17:15 ` Jeff Moyer [not found] ` <x497f75hxwp.fsf-RRHT56Q3PSP4kTEheFKJxxDDeQx5vsVwAInAS/Ez/D0@public.gmane.org> 2016-12-12 18:46 ` Dan Williams 2016-12-12 18:46 ` Dan Williams 2016-12-13 23:46 ` Jeff Moyer 2016-12-13 23:46 ` Jeff Moyer [not found] ` <x494m278kam.fsf-RRHT56Q3PSP4kTEheFKJxxDDeQx5vsVwAInAS/Ez/D0@public.gmane.org> 2016-12-14 1:17 ` Dan Williams 2016-12-14 1:17 ` Dan Williams [not found] ` <CAPcyv4hzCHqF9ALYFYCpv=WyHoMOFDoeWOd38b4sKwmkkmSgLQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2016-12-15 16:50 ` Jeff Moyer 2016-12-15 16:50 ` Jeff Moyer [not found] ` <x49shpp3zn8.fsf-RRHT56Q3PSP4kTEheFKJxxDDeQx5vsVwAInAS/Ez/D0@public.gmane.org> 2016-12-15 23:48 ` Dan Williams 2016-12-15 23:48 ` Dan Williams [not found] ` <CAPcyv4gWW0NfiP6iGpaB5hAyiD8SMbeAfAKvezDLH1GzH_HqTA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2016-12-16 2:33 ` Dan Williams 2016-12-16 2:33 ` Dan Williams
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20161214150130.GE6279@linux-x5ow.site \ --to=jthumshirn@suse.de \ --cc=dan.j.williams@intel.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-nvdimm@lists.01.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.