linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jason Gunthorpe <jgg@nvidia.com>
To: Dan Williams <dan.j.williams@intel.com>
Cc: linux-cxl@vger.kernel.org,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Vishal L Verma <vishal.l.verma@intel.com>,
	"Weiny, Ira" <ira.weiny@intel.com>,
	"Schofield, Alison" <alison.schofield@intel.com>
Subject: Re: [PATCH v2 2/4] cxl/mem: Fix synchronization mechanism for device removal vs ioctl operations
Date: Tue, 30 Mar 2021 16:51:43 -0300	[thread overview]
Message-ID: <20210330195143.GB1430856@nvidia.com> (raw)
In-Reply-To: <CAPcyv4h1qgQAP1JT88rWOf9xZz_1o2yPvMFETNY2dFn17uFwhw@mail.gmail.com>

On Tue, Mar 30, 2021 at 12:43:15PM -0700, Dan Williams wrote:

> Ok, so this is the disagreement. Strict adherence to the model where
> it does not matter in practice.

The basic problem is that it is hard to know if it does not matter in
practice because you never know what the compiler might decide to do
...

It is probably fine, but then again I've seen a surprising case in the
mm where the compiler did generate double loads and it wasn't fine.

Now with the new data race detector it feels like marking all
concurrent data access with READ_ONCE / WRITE_ONCE (or a stronger
atomic) is the correct thing to do.

> > > There's no race above. The rule is that any possible observation of
> > > ->state_in_sysfs == 1, or rcu_dereference() != NULL, must be
> > > flushed.
> >
> > It is not about the flushing.
> 
> Ok, it's not about the flushing it's about whether the store to
> state_in_sysfs can leak past up_write(). If that store can leak then
> neither arrangement of:

up_write() and synchronize_srcu() are both store barriers, so the
store must be ordered.

It is the reader that is the problem. This ordering:

> down_write(...):
> cdev_device_del(...);
> up_write(...);

Prevents state_in_sysfs from being unstable during read as the write
lock prevents it from being written while a reader is active. No
READ_ONCE needed.

Jason

  reply	other threads:[~2021-03-30 19:52 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-30  2:47 [PATCH v2 0/4] cxl/mem: Fix memdev device setup Dan Williams
2021-03-30  2:47 ` [PATCH v2 1/4] cxl/mem: Use sysfs_emit() for attribute show routines Dan Williams
2021-03-30  2:47 ` [PATCH v2 2/4] cxl/mem: Fix synchronization mechanism for device removal vs ioctl operations Dan Williams
2021-03-30 11:16   ` Jason Gunthorpe
2021-03-30 15:37     ` Dan Williams
2021-03-30 15:47       ` Jason Gunthorpe
2021-03-30 16:05         ` Dan Williams
2021-03-30 17:02           ` Jason Gunthorpe
2021-03-30 17:31             ` Dan Williams
2021-03-30 17:54               ` Jason Gunthorpe
2021-03-30 19:00                 ` Dan Williams
2021-03-30 19:26                   ` Jason Gunthorpe
2021-03-30 19:43                     ` Dan Williams
2021-03-30 19:51                       ` Jason Gunthorpe [this message]
2021-03-30 21:00                         ` Dan Williams
2021-03-30 22:09                           ` Jason Gunthorpe
2021-03-30  2:47 ` [PATCH v2 3/4] cxl/mem: Do not rely on device_add() side effects for dev_set_name() failures Dan Williams
2021-03-30  2:48 ` [PATCH v2 4/4] cxl/mem: Disable cxl device power management 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=20210330195143.GB1430856@nvidia.com \
    --to=jgg@nvidia.com \
    --cc=alison.schofield@intel.com \
    --cc=dan.j.williams@intel.com \
    --cc=ira.weiny@intel.com \
    --cc=linux-cxl@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=vishal.l.verma@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).