From patchwork Fri Jul 27 20:11:28 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bruce Fields X-Patchwork-Id: 1250601 Return-Path: X-Original-To: patchwork-linux-nfs@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork2.kernel.org (Postfix) with ESMTP id 59CA0E00B4 for ; Fri, 27 Jul 2012 20:11:43 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752794Ab2G0ULf (ORCPT ); Fri, 27 Jul 2012 16:11:35 -0400 Received: from fieldses.org ([174.143.236.118]:44219 "EHLO fieldses.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752793Ab2G0ULd (ORCPT ); Fri, 27 Jul 2012 16:11:33 -0400 Received: from bfields by fieldses.org with local (Exim 4.72) (envelope-from ) id 1Suqsl-0001eU-FJ; Fri, 27 Jul 2012 16:11:31 -0400 From: "J. Bruce Fields" To: Al Viro Cc: linux-nfs@vger.kernel.org, linux-fsdevel@vger.kernel.org, "J. Bruce Fields" , Nick Piggin Subject: [PATCH 1/2] dcache: use IS_ROOT to decide where dentry is hashed Date: Fri, 27 Jul 2012 16:11:28 -0400 Message-Id: <1343419889-6320-1-git-send-email-bfields@redhat.com> X-Mailer: git-send-email 1.7.1 Sender: linux-nfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-nfs@vger.kernel.org From: "J. Bruce Fields" Every hashed dentry is either hashed in the dentry_hashtable, or a superblock's s_anon list. __d_shrink assumes it can determine which is the case by checking DCACHE_DISCONNECTED; this is not true. It is true that when DCACHE_DISCONNECTED is cleared, the dentry is not only hashed on dentry_hashtable, but is fully connected to its parents back to the root. But the converse is *not* true: fs/exportfs/expfs.c:reconnect_path() attempts to connect a directory (found by filehandle lookup) back to root by ascending to parents and performing lookups one at a time. It does not clear DCACHE_DISCONNECTED until its done, and that is not at all an atomic process. In particular, it is possible for DCACHE_DISCONNECTED to be set on a dentry which is hashed on the dentry_hashtable. Instead, use IS_ROOT() to check which hash chain a dentry is on. This *does* work: Dentries are hashed only by: - d_obtain_alias, which adds an IS_ROOT() dentry to sb_anon. - __d_rehash, called by _d_rehash: hashes to the dentry's parent, and all callers of _d_rehash appear to have d_parent set to a "real" parent. - __d_rehash, called by __d_move: rehashes the moved dentry to hash chain determined by target, and assigns target's d_parent to its d_parent, before dropping the dentry's d_lock. Therefore I believe it's safe for a holder of a dentry's d_lock to assume that it is hashed on sb_anon if and only if IS_ROOT(dentry) is true. I believe the incorrect assumption about DCACHE_DISCONNECTED was originally introduced by ceb5bdc2d246 "fs: dcache per-bucket dcache hash locking". Cc: Nick Piggin Reviewed-by: NeilBrown Signed-off-by: J. Bruce Fields --- fs/dcache.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/dcache.c b/fs/dcache.c index 4046904..740b4f5 100644 --- a/fs/dcache.c +++ b/fs/dcache.c @@ -410,7 +410,7 @@ static void __d_shrink(struct dentry *dentry) { if (!d_unhashed(dentry)) { struct hlist_bl_head *b; - if (unlikely(dentry->d_flags & DCACHE_DISCONNECTED)) + if (unlikely(IS_ROOT(dentry))) b = &dentry->d_sb->s_anon; else b = d_hash(dentry->d_parent, dentry->d_name.hash);