From: "Verma, Vishal L" <vishal.l.verma@intel.com>
To: "Williams, Dan J" <dan.j.williams@intel.com>,
"Jiang, Dave" <dave.jiang@intel.com>,
"osalvador@suse.de" <osalvador@suse.de>,
"akpm@linux-foundation.org" <akpm@linux-foundation.org>
Cc: "david@redhat.com" <david@redhat.com>,
"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
"Huang, Ying" <ying.huang@intel.com>,
"linux-cxl@vger.kernel.org" <linux-cxl@vger.kernel.org>,
"dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-mm@kvack.org" <linux-mm@kvack.org>,
"willy@infradead.org" <willy@infradead.org>,
"nvdimm@lists.linux.dev" <nvdimm@lists.linux.dev>
Subject: Re: [PATCH v7 1/5] dax/bus.c: replace driver-core lock usage by a local rwsem
Date: Tue, 12 Mar 2024 20:20:17 +0000 [thread overview]
Message-ID: <ebc79448a1fdb47ff16144c12cd5d8dfc733dee8.camel@intel.com> (raw)
In-Reply-To: <65f0b5ef41817_aa222941a@dwillia2-mobl3.amr.corp.intel.com.notmuch>
On Tue, 2024-03-12 at 13:07 -0700, Dan Williams wrote:
> Vishal Verma wrote:
> > The dax driver incorrectly used driver-core device locks to protect
> > internal dax region and dax device configuration structures. Replace the
> > device lock usage with a local rwsem, one each for dax region
> > configuration and dax device configuration. As a result of this
> > conversion, no device_lock() usage remains in dax/bus.c.
> >
> > Cc: Dan Williams <dan.j.williams@intel.com>
> > Reported-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> > ---
> > drivers/dax/bus.c | 220 ++++++++++++++++++++++++++++++++++++++----------------
> > 1 file changed, 157 insertions(+), 63 deletions(-)
> >
> > diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c
> > index 1ff1ab5fa105..cb148f74ceda 100644
> > --- a/drivers/dax/bus.c
> > +++ b/drivers/dax/bus.c
> > @@ -12,6 +12,18 @@
> >
> > static DEFINE_MUTEX(dax_bus_lock);
> >
> > +/*
> > + * All changes to the dax region configuration occur with this lock held
> > + * for write.
> > + */
> > +DECLARE_RWSEM(dax_region_rwsem);
> > +
> > +/*
> > + * All changes to the dax device configuration occur with this lock held
> > + * for write.
> > + */
> > +DECLARE_RWSEM(dax_dev_rwsem);
> > +
> > #define DAX_NAME_LEN 30
> > struct dax_id {
> > struct list_head list;
> > @@ -180,7 +192,7 @@ static u64 dev_dax_size(struct dev_dax *dev_dax)
> > u64 size = 0;
> > int i;
> >
> > - device_lock_assert(&dev_dax->dev);
> > + WARN_ON_ONCE(!rwsem_is_locked(&dax_dev_rwsem));
>
> Apologies for the late review, but...
>
> All of these WARN_ON_ONCE() usages should be replaced with
> lockdep_assert_held() and lockdep_assert_held_write() where appropriate.
Makes sense - I can send a patch post -rc1 to change these if that's okay Andrew?
next prev parent reply other threads:[~2024-03-12 20:20 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-24 20:03 [PATCH v7 0/5] Add DAX ABI for memmap_on_memory Vishal Verma
2024-01-24 20:03 ` [PATCH v7 1/5] dax/bus.c: replace driver-core lock usage by a local rwsem Vishal Verma
2024-01-26 21:12 ` Alison Schofield
2024-03-12 20:07 ` Dan Williams
2024-03-12 20:20 ` Verma, Vishal L [this message]
2024-03-12 20:22 ` Andrew Morton
2024-01-24 20:03 ` [PATCH v7 2/5] dax/bus.c: replace several sprintf() with sysfs_emit() Vishal Verma
2024-01-26 21:14 ` Alison Schofield
2024-01-24 20:03 ` [PATCH v7 3/5] Documentatiion/ABI: Add ABI documentation for sys-bus-dax Vishal Verma
2024-01-24 20:03 ` [PATCH v7 4/5] mm/memory_hotplug: export mhp_supports_memmap_on_memory() Vishal Verma
2024-01-24 20:03 ` [PATCH v7 5/5] dax: add a sysfs knob to control memmap_on_memory behavior Vishal Verma
2024-01-26 21:40 ` Alison Schofield
2024-01-26 1:29 ` [PATCH v7 0/5] Add DAX ABI for memmap_on_memory Andrew Morton
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=ebc79448a1fdb47ff16144c12cd5d8dfc733dee8.camel@intel.com \
--to=vishal.l.verma@intel.com \
--cc=akpm@linux-foundation.org \
--cc=dan.j.williams@intel.com \
--cc=dave.hansen@linux.intel.com \
--cc=dave.jiang@intel.com \
--cc=david@redhat.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-cxl@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=nvdimm@lists.linux.dev \
--cc=osalvador@suse.de \
--cc=willy@infradead.org \
--cc=ying.huang@intel.com \
/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: link
Be 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.