[1/3] dcache: use IS_ROOT to decide where dentry is hashed

Commit Message

Bruce Fields Sept. 6, 2013, 7:03 p.m. UTC

On Fri, Sep 06, 2013 at 10:00:44AM -0700, Christoph Hellwig wrote:
> On Fri, Sep 06, 2013 at 11:43:48AM -0400, J. Bruce Fields wrote:
> > diff --git a/fs/dcache.c b/fs/dcache.c
> > index b949af8..934f02d 100644
> > --- a/fs/dcache.c
> > +++ b/fs/dcache.c
> > @@ -393,7 +393,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)))
> 
> I think this needs a comment about why the IS_ROOT check is fine,
> destilled from your commit log.

How about this?:

 	if (!d_unhashed(dentry)) {
 		struct hlist_bl_head *b;
+		/*
+		 * Hashed dentries are normally on the dentry hashtable,
+		 * with the exception of those newly allocated by
+		 * d_obtain_alias, which are always IS_ROOT:
+		 */
 		if (unlikely(IS_ROOT(dentry)))
 			b = &dentry->d_sb->s_anon;
 		else

> Also while reviewing this I noticed that one of the two callers of
> __d_shrink already has the d_unhashed check - it might make sense to
> move it to the other caller as well if you touch this area anyway.
> (as a separate patch).

Sure.  That'd look like the following.

--b.

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/fs/dcache.c b/fs/dcache.c
index 113c82f..81db000 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -391,23 +391,21 @@  static struct dentry *d_kill(struct dentry *dentry, struct dentry *parent)
  */
 static void __d_shrink(struct dentry *dentry)
 {
-	if (!d_unhashed(dentry)) {
-		struct hlist_bl_head *b;
-		/*
-		 * Hashed dentries are normally on the dentry hashtable,
-		 * with the exception of those newly allocated by
-		 * d_obtain_alias, which are always IS_ROOT:
-		 */
-		if (unlikely(IS_ROOT(dentry)))
-			b = &dentry->d_sb->s_anon;
-		else
-			b = d_hash(dentry->d_parent, dentry->d_name.hash);
+	struct hlist_bl_head *b;
+	/*
+	 * Hashed dentries are normally on the dentry hashtable,
+	 * with the exception of those newly allocated by
+	 * d_obtain_alias, which are always IS_ROOT:
+	 */
+	if (unlikely(IS_ROOT(dentry)))
+		b = &dentry->d_sb->s_anon;
+	else
+		b = d_hash(dentry->d_parent, dentry->d_name.hash);
 
-		hlist_bl_lock(b);
-		__hlist_bl_del(&dentry->d_hash);
-		dentry->d_hash.pprev = NULL;
-		hlist_bl_unlock(b);
-	}
+	hlist_bl_lock(b);
+	__hlist_bl_del(&dentry->d_hash);
+	dentry->d_hash.pprev = NULL;
+	hlist_bl_unlock(b);
 }
 
 /**
@@ -902,7 +900,8 @@  static void shrink_dcache_for_umount_subtree(struct dentry *dentry)
 				dentry->d_op->d_prune(dentry);
 
 			dentry_lru_del(dentry);
-			__d_shrink(dentry);
+			if (!d_unhashed(dentry))
+				__d_shrink(dentry);
 
 			if (dentry->d_lockref.count != 0) {
 				printk(KERN_ERR