diff mbox

[2/3] dcache: Don't set DISCONNECTED on "pseudo filesystem" dentries

Message ID 1378482230-16312-2-git-send-email-bfields@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Bruce Fields Sept. 6, 2013, 3:43 p.m. UTC 
From: "J. Bruce Fields" <bfields@redhat.com>

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.

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@kernel.dk>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
 fs/dcache.c |    9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

Comments

Christoph Hellwig Sept. 6, 2013, 5:03 p.m. UTC | #1 
On Fri, Sep 06, 2013 at 11:43:49AM -0400, J. Bruce Fields wrote:
> From: "J. Bruce Fields" <bfields@redhat.com>
> 
> 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.

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.

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

Patch

diff --git a/fs/dcache.c b/fs/dcache.c
index 934f02d..ec66780 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -1308,12 +1308,13 @@  struct dentry *d_alloc(struct dentry * parent, const struct qstr *name)
 }
 EXPORT_SYMBOL(d_alloc);
 
+/*
+ * For filesystems that do not actually use the dentry cache at all, and
+ * only ever deal in IS_ROOT() dentries:
+ */
 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);