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.
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
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
