* DCACHE_DISCONNECTED patches
@ 2012-08-21 21:37 ` J. Bruce Fields
0 siblings, 0 replies; 7+ messages in thread
From: J. Bruce Fields @ 2012-08-21 21:37 UTC (permalink / raw)
To: Al Viro
Cc: linux-nfs-u79uwXL29TY76Z2rM5mHXA, linux-fsdevel-u79uwXL29TY76Z2rM5mHXA
Resending these--the second of these two is just cleanup, but the first
looks like a potentially ugly bug (not that I can trigger it).
--b.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 7+ messages in thread
* DCACHE_DISCONNECTED patches
@ 2012-08-21 21:37 ` J. Bruce Fields
0 siblings, 0 replies; 7+ messages in thread
From: J. Bruce Fields @ 2012-08-21 21:37 UTC (permalink / raw)
To: Al Viro; +Cc: linux-nfs, linux-fsdevel
Resending these--the second of these two is just cleanup, but the first
looks like a potentially ugly bug (not that I can trigger it).
--b.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] dcache: use IS_ROOT to decide where dentry is hashed
2012-08-21 21:37 ` J. Bruce Fields
@ 2012-08-21 21:37 ` J. Bruce Fields
-1 siblings, 0 replies; 7+ messages in thread
From: J. Bruce Fields @ 2012-08-21 21:37 UTC (permalink / raw)
To: Al Viro
Cc: linux-nfs-u79uwXL29TY76Z2rM5mHXA,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA, J. Bruce Fields,
Nick Piggin
From: "J. Bruce Fields" <bfields-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
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 <npiggin-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org>
Reviewed-by: NeilBrown <neilb-l3A5Bk7waGM@public.gmane.org>
Signed-off-by: J. Bruce Fields <bfields-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
fs/dcache.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/dcache.c b/fs/dcache.c
index 8086636..ea2fcfa 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);
--
1.7.11.4
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 1/2] dcache: use IS_ROOT to decide where dentry is hashed
@ 2012-08-21 21:37 ` J. Bruce Fields
0 siblings, 0 replies; 7+ messages in thread
From: J. Bruce Fields @ 2012-08-21 21:37 UTC (permalink / raw)
To: Al Viro; +Cc: linux-nfs, linux-fsdevel, J. Bruce Fields, Nick Piggin
From: "J. Bruce Fields" <bfields@redhat.com>
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 <npiggin@kernel.dk>
Reviewed-by: NeilBrown <neilb@suse.de>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
fs/dcache.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/dcache.c b/fs/dcache.c
index 8086636..ea2fcfa 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);
--
1.7.11.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] dcache: Don't set DISCONNECTED on "pseudo filesystem" dentries
2012-08-21 21:37 ` J. Bruce Fields
(?)
(?)
@ 2012-08-21 21:37 ` J. Bruce Fields
-1 siblings, 0 replies; 7+ messages in thread
From: J. Bruce Fields @ 2012-08-21 21:37 UTC (permalink / raw)
To: Al Viro; +Cc: linux-nfs, linux-fsdevel, J. Bruce Fields, Nick Piggin
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 use 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(-)
diff --git a/fs/dcache.c b/fs/dcache.c
index ea2fcfa..c4b9d4f2c 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -1352,12 +1352,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);
--
1.7.11.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* DCACHE_DISCONNECTED patches
@ 2012-05-09 21:18 ` J. Bruce Fields
0 siblings, 0 replies; 7+ messages in thread
From: J. Bruce Fields @ 2012-05-09 21:18 UTC (permalink / raw)
To: Al Viro
Cc: linux-fsdevel-u79uwXL29TY76Z2rM5mHXA, linux-nfs-u79uwXL29TY76Z2rM5mHXA
This is a resend of a couple DCACHE_DISCONNECTED patches that I think
could go into 3.5.
--b.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 7+ messages in thread
* DCACHE_DISCONNECTED patches
@ 2012-05-09 21:18 ` J. Bruce Fields
0 siblings, 0 replies; 7+ messages in thread
From: J. Bruce Fields @ 2012-05-09 21:18 UTC (permalink / raw)
To: Al Viro; +Cc: linux-fsdevel, linux-nfs
This is a resend of a couple DCACHE_DISCONNECTED patches that I think
could go into 3.5.
--b.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2012-08-21 21:37 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-21 21:37 DCACHE_DISCONNECTED patches J. Bruce Fields
2012-08-21 21:37 ` J. Bruce Fields
[not found] ` <1345585032-19119-1-git-send-email-bfields-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2012-08-21 21:37 ` [PATCH 1/2] dcache: use IS_ROOT to decide where dentry is hashed J. Bruce Fields
2012-08-21 21:37 ` J. Bruce Fields
2012-08-21 21:37 ` [PATCH 2/2] dcache: Don't set DISCONNECTED on "pseudo filesystem" dentries J. Bruce Fields
-- strict thread matches above, loose matches on Subject: below --
2012-05-09 21:18 DCACHE_DISCONNECTED patches J. Bruce Fields
2012-05-09 21:18 ` J. Bruce Fields
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.