> On Mar 1, 2016, at 22:50, Sage Weil <sweil@xxxxxxxxxx> wrote:
>
> Hi Al,
>
> On Fri, 26 Feb 2016, Al Viro wrote:
>> 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.
>
> Right.
>
>> 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...
>
> In this case it doesn't actually matter if it is hashed or not, since
> we will look at the lease state on the dentry before trusting it...
>
> This code dates back to when Ceph was originally upstreamed, so the
> history is murky, but I expect at that point I wanted to avoid hashing in
> the no-lease case. But I don't think it matters. We should just remove
> the prehash argument from splice_dentry entirely.
>
> Zheng, does that sound right?
Yes. I think we can remove the d_rehash(dn) call and rehash parameter.
Regards
Yan, Zheng --
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
