From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1422880AbcBZRm5 (ORCPT <rfc822;w@1wt.eu>);
	Fri, 26 Feb 2016 12:42:57 -0500
Received: from zeniv.linux.org.uk ([195.92.253.2]:50396 "EHLO
	ZenIV.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1030296AbcBZRmw (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Fri, 26 Feb 2016 12:42:52 -0500
Date: Fri, 26 Feb 2016 17:42:50 +0000
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()?
Message-ID: <20160226174250.GA17997@ZenIV.linux.org.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
User-Agent: Mutt/1.5.21 (2010-09-15)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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.