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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox