From: Al Viro <viro@ZenIV.linux.org.uk>
To: Sage Weil <sweil@redhat.com>
Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org
Subject: [ceph] what's going on with d_rehash() in splice_dentry()?
Date: Fri, 26 Feb 2016 17:42:50 +0000 [thread overview]
Message-ID: <20160226174250.GA17997@ZenIV.linux.org.uk> (raw)
You have, modulo printks and BUG_ON(),
{
struct dentry *realdn;
/* dn must be unhashed */
if (!d_unhashed(dn))
d_drop(dn);
realdn = d_splice_alias(in, dn);
if (IS_ERR(realdn)) {
if (prehash)
*prehash = false; /* don't rehash on error */
dn = realdn; /* note realdn contains the error */
goto out;
} else if (realdn) {
dput(dn);
dn = realdn;
}
if ((!prehash || *prehash) && d_unhashed(dn))
d_rehash(dn);
When d_splice_alias() returns NULL it has hashed the dentry you'd given it;
when it returns a different dentry, that dentry is also returned hashed.
IOW, d_rehash(dn) in there should never be called.
If you have a case when it _is_ called, you've found a bug somewhere and
I'd like to see details. AFAICS, the whole prehash thing appears to be
pointless - even the place where we modify *prehash, since in that case
we return ERR_PTR() and the only caller passing non-NULL prehash (&have_lease)
buggers off on such return value past all code that would look at have_lease
value.
One possible reading is that you want to prevent hashing in !have_lease
case of
dn = splice_dentry(dn, in, &have_lease);
If that's the case, you might have a problem, since it will be hashed no
matter what...
PS: the proof that d_splice_alias() always hashes is simple - if you exclude
the places where it returns ERR_PTR(), you are left with
d_rehash(dentry);
return NULL;
in the very end,
__d_move(new, dentry, false);
...
return new;
and
int err = __d_unalias(inode, dentry, new);
...
// err turned out to be zero
return new;
The first one is obvious - we return NULL after an explicit d_rehash() of
the argument. __d_move() is guaranteed to return with its first argument
hashed due to
__d_drop(dentry);
__d_rehash(dentry, d_hash(target->d_parent, target->d_name.hash));
(dentry here refers to the first argument of __d_move() - it's our 'new').
And zero-returning __d_unalias() ends up calling __d_move(), with the
third argument of __d_unalias() ending up as the first one of __d_move().
So in both remaining cases we return a dentry that has just been hashed.
next reply other threads:[~2016-02-26 17:42 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-26 17:42 Al Viro [this message]
2016-03-01 14:50 ` [ceph] what's going on with d_rehash() in splice_dentry()? Sage Weil
2016-03-02 3:00 ` Yan, Zheng
2016-03-07 2:16 ` Al Viro
2016-03-07 11:25 ` Sage Weil
2016-03-07 14:56 ` 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=20160226174250.GA17997@ZenIV.linux.org.uk \
--to=viro@zeniv.linux.org.uk \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=sweil@redhat.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.