On Fri, Sep 06, 2013 at 10:03:39AM -0700, Christoph Hellwig wrote:
> On Fri, Sep 06, 2013 at 11:43:49AM -0400, J. Bruce Fields wrote:
> > From: "J. Bruce Fields" <bfields@xxxxxxxxxx>
> >
> > I can't for the life of me see any reason why anyone would care whether
> > a dentry that is never hooked into the dentry cache would need
> > DCACHE_DISCONNECTED set.
>
> __d_shrink did before your PATCH 1/3.
Well, d_alloc_pseudo dentries aren't even hashed, so even d_shrink
didn't actually reach that DCACHE_DISCONNECTED check.
Uh, maybe change that "would" above to a "should".
>
> With that fixed your patch looks fine.
>
> > +/*
> > + * For filesystems that do not actually use the dentry cache at all, and
> > + * only ever deal in IS_ROOT() dentries:
> > + */
>
> If you document the function it really should be a kerneldoc comment.
>
> Also the description, while technically correct, isn't all that useful.
>
> It need an explanation why the filesystem is fine with unhashed
> dentries, and the reason is that it never performs any lookups as it
> pins the dentries in memory.
Makes sense--result follows.
Thanks for the review.
--b.
commit 089d891965374b4262973e51839fe361e2ca3cf0
Author: J. Bruce Fields <bfields@xxxxxxxxxx>
Date: Fri Jun 29 16:20:47 2012 -0400
dcache: Don't set DISCONNECTED on "pseudo filesystem" dentries
I can't for the life of me see any reason why anyone should care whether
a dentry that is never hooked into the dentry cache would need
DCACHE_DISCONNECTED set.
This originates from 4b936885ab04dc6e0bb0ef35e0e23c1a7364d9e5 "fs:
improve scalability of pseudo filesystems", which probably just made the
false assumption the DCACHE_DISCONNECTED was meant to be set on anything
not connected to a parent somehow.
So this is just confusing. Ideally the only uses of DCACHE_DISCONNECTED
would be in the filehandle-lookup code, which needs it to ensure
dentries are connected into the dentry tree before use.
I left d_alloc_pseudo there even though it's now equivalent to
__d_alloc(), just on the theory the name is better documentation of its
intended use outside dcache.c.
Cc: Nick Piggin <npiggin@xxxxxxxxx>
Signed-off-by: J. Bruce Fields <bfields@xxxxxxxxxx>
diff --git a/fs/dcache.c b/fs/dcache.c
index 7208b38..2255359 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -1313,12 +1313,17 @@ struct dentry *d_alloc(struct dentry * parent, const struct qstr *name)
}
EXPORT_SYMBOL(d_alloc);
+/**
+ * d_alloc_pseudo - allocate a dentry (for lookup-less filesystems)
+ * @sb: the superblock
+ * @name: qstr of the name
+ *
+ * For a filesystem that just pins its dentries in memory and never
+ * performs lookups at all, return an unhashed IS_ROOT dentry.
+ */
struct dentry *d_alloc_pseudo(struct super_block *sb, const struct qstr *name)
{
- struct dentry *dentry = __d_alloc(sb, name);
- if (dentry)
- dentry->d_flags |= DCACHE_DISCONNECTED;
- return dentry;
+ return __d_alloc(sb, name);
}
EXPORT_SYMBOL(d_alloc_pseudo);
--
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
