All of lore.kernel.org
 help / color / mirror / Atom feed
From: "bfields@fieldses.org" <bfields@fieldses.org>
To: Trond Myklebust <trondmy@hammerspace.com>
Cc: "linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH] SUNRPC: Don't allow compiler optimisation of svc_xprt_release_slot()
Date: Tue, 8 Jan 2019 10:01:07 -0500	[thread overview]
Message-ID: <20190108150107.GA15921@fieldses.org> (raw)
In-Reply-To: <e14e17167ef98c456e7648ad0536827b7870f0ff.camel@hammerspace.com>

On Mon, Jan 07, 2019 at 10:06:19PM +0000, Trond Myklebust wrote:
> On Mon, 2019-01-07 at 16:32 -0500, bfields@fieldses.org wrote:
> > So maybe we actually need
> > 
> >  static bool svc_xprt_has_something_to_do(struct svc_xprt *xprt)
> >  {
> > +	mb();
> 
> You would at best need a 'smp_rmb()'. There is nothing to gain from
> adding a write barrier here,

That's not my understanding.

What we have is basically:

	1			2
	----			----
	WRITE to A		WRITE to B

	READ from A and B	READ from A and B

and we want to guarantee that at least one of those two reads will see
both of the writes.

A read barrier only orders reads with respect to the barrier, it doesn't
do anything about writes, so doesn't guarantee anything here.

--b.



> and you don't even need a read barrier in
> the non-smp case.
> 
> >  	if (xprt->xpt_flags & ((1<<XPT_CONN)|(1<<XPT_CLOSE)))
> >  		return true;
> >  	if (xprt->xpt_flags & ((1<<XPT_DATA)|(1<<XPT_DEFERRED))) {
> > 
> > Then whichever memory barrier executes second guarantees that the
> > following check sees the result of both the XPT_DATA and xpt_nr_rqsts
> > changes.  I think....
> 
> 
> 
> -- 
> Trond Myklebust
> Linux NFS client maintainer, Hammerspace
> trond.myklebust@hammerspace.com
> 
> 

  reply	other threads:[~2019-01-08 15:01 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-03 14:17 [PATCH] SUNRPC: Don't allow compiler optimisation of svc_xprt_release_slot() Trond Myklebust
2019-01-03 22:45 ` J Bruce Fields
2019-01-03 23:40   ` Trond Myklebust
2019-01-04 17:39     ` bfields
2019-01-07 21:32       ` bfields
2019-01-07 22:06         ` Trond Myklebust
2019-01-08 15:01           ` bfields [this message]
2019-01-08 16:21             ` Trond Myklebust
2019-01-09 16:51               ` bfields
2019-01-09 17:41                 ` Trond Myklebust
2019-01-11 21:12                   ` bfields
2019-01-11 21:52                     ` Chuck Lever
2019-01-11 21:54                       ` Chuck Lever
2019-01-11 22:10                         ` Bruce Fields
2019-01-11 22:27                           ` Chuck Lever
2019-01-12  0:56                             ` Bruce Fields
2019-01-14 17:24                               ` Chuck Lever
2019-01-25 20:30                                 ` Bruce Fields
2019-01-25 21:32                                   ` Chuck Lever

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=20190108150107.GA15921@fieldses.org \
    --to=bfields@fieldses.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=trondmy@hammerspace.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.