All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: linux-fsdevel <linux-fsdevel@vger.kernel.org>
Subject: Re: [RFC] parent in ->d_compare() arguments
Date: Sun, 31 Jul 2016 00:30:44 +0100	[thread overview]
Message-ID: <20160730233044.GZ2356@ZenIV.linux.org.uk> (raw)
In-Reply-To: <CA+55aFy3zGCSQpQ594WmN6kn1hxQzN8=pyQ3yXGezv+mWeAeog@mail.gmail.com>

On Sat, Jul 30, 2016 at 01:44:48PM -0700, Linus Torvalds wrote:

> No. Let's not. "smp_rmb()" is completely free on x86 (ok, so it's a
> instruction scheduling barrer - close enough), so trying to optimize
> away rmb's and replacing them with double compares sounds entirely
> misdesigned.
> 
> Yes, yes, there are other architectures where rmb is much more
> expensive. But quite frankly, in most cases those architectures have
> broken synchronization to begin with ("synchronization is unusual, so
> let's not optimize it"). They'll fix it eventually.
> 
> Instead, what we should look at, is to make raw_seqcount_begin() use a
> smp_load_acquire() on architectures where that is cheaper than the
> rmb.
> 
> But again, I don't see the point of double-testing "parent" when a
> load-acquire or load+rmb _should_ be cheap (and absolutely is on x86).

Umm...  Even on x86, a lot of hash chain elements will have ->d_parent
mismatch.  Suppose rmb was a no-op; current code does
	fetch ->d_seq
	fetch ->d_parent
	compare with register
	branch taken to the end of body
while this would avoid the first fetch.  On the entries with the same
->d_parent we'd do
	fetch ->d_parent
	compare with register
	branch not taken
	fetch ->d_seq
	fetch ->d_parent
	compare with register
	branch (expectedly) not taken
which is the same as the mainline in terms of actual memory accesses and extra
3 insns.  I suspect that the win on the entries with ->d_parent mismatch can
outweight that, but that needs profiling to verify.

  reply	other threads:[~2016-07-30 23:30 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-30  1:07 [RFC] parent in ->d_compare() arguments Al Viro
2016-07-30 20:44 ` Linus Torvalds
2016-07-30 23:30   ` Al Viro [this message]
2016-07-30 23:36     ` Linus Torvalds
2016-07-30 23:52       ` Al Viro

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=20160730233044.GZ2356@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=torvalds@linux-foundation.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: 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.