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@vger.kernel.org
Subject: [RFC] parent in ->d_compare() arguments
Date: Sat, 30 Jul 2016 02:07:38 +0100	[thread overview]
Message-ID: <20160730010738.GY2356@ZenIV.linux.org.uk> (raw)

	We are passing to ->d_compare() instances parent, dentry itself,
and a consistent snapshot of its ->d_name.name and ->d_name.len.  In all
but one instance (ncpfs one) the only thing we need the parent for is
finding the superblock.  Which is available as dentry->d_sb.  ncpfs one
is weird, but it actually wants parent's ->d_inode, so it has to be
careful about the RCU case anyway.

	Do you have any objections to trimming the arguments list?  I want
to kill the 'parent' argument there and let ncpfs carefully walk
dentry->d_parent->d_inode.

	Another thing in the same area: __d_lookup_rcu() does
        hlist_bl_for_each_entry_rcu(dentry, node, b, d_hash) {
                unsigned seq;
                seq = raw_seqcount_begin(&dentry->d_seq);
                if (dentry->d_parent != parent)
                        continue;
and raw_seqcount_begin() contains smp_rmb().  Seeing that we hit ->d_parent
mismatch often enough and that we are fine with false negatives anyway,
let's turn that into
                if (dentry->d_parent != parent)
                        continue;
                seq = raw_seqcount_begin(&dentry->d_seq);
                if (unlikely(dentry->d_parent != parent))
                        continue;
and cut down on the number of smp_rmb() per __d_lookup_rcu().  We do need the
second check (to make sure that ->d_seq guarantees a consistent state of
everything), but it's going to trigger _very_ rarely - basically, only if
we step on dentry with the right parent just as it's being hit with
cross-directory rename.  That's a very slow case, since we are quite likely
to search the tail of the wrong hash chain and eventually bugger off with
NULL, switching to non-RCU codepath.  So in the cases of parent mismatch
we end up doing one fetch instead of two fetches + skip smp_rmb(), while in
case of parent match we do extra fetch from hot cacheline + branch not
taken.  AFAICS, it's going to be a win even on architectures with trivial
smp_rmb(); on something with costly smp_rmb() the win could be considerable.

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

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-30  1:07 Al Viro [this message]
2016-07-30 20:44 ` [RFC] parent in ->d_compare() arguments Linus Torvalds
2016-07-30 23:30   ` Al Viro
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=20160730010738.GY2356@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.