* DCACHE_DISCONNECTED use in try_to_ascend/d_kill
@ 2012-07-12 22:28 J. Bruce Fields
2012-07-12 22:40 ` J. Bruce Fields
0 siblings, 1 reply; 3+ messages in thread
From: J. Bruce Fields @ 2012-07-12 22:28 UTC (permalink / raw)
To: Trond Myklebust; +Cc: linux-nfs, linux-fsdevel
I've been trying to figure out if we can pare down DCACHE_DISCONNECTED
uses to only those required by exportfs, and ran across this one.
Is there another test that would work instead?
(E.g., checking list_empty(&dentry->d_u.d_child)?)
--b.
commit c83ce989cb5ff86575821992ea82c4df5c388ebc
Author: Trond Myklebust <Trond.Myklebust@netapp.com>
Date: Tue Mar 15 13:36:43 2011 -0400
VFS: Fix the nfs sillyrename regression in kernel 2.6.38
The new vfs locking scheme introduced in 2.6.38 breaks NFS sillyrename
because the latter relies on being able to determine the parent
directory of the dentry in the ->iput() callback in order to send the
appropriate unlink rpc call.
Looking at the code that cares about races with dput(), there doesn't
seem to be anything that specifically uses d_parent as a test for
whether or not there is a race:
- __d_lookup_rcu(), __d_lookup() all test for d_hashed() after d_parent
- shrink_dcache_for_umount() is safe since nothing else can rearrange
the dentries in that super block.
- have_submount(), select_parent() and d_genocide() can test for a
deletion if we set the DCACHE_DISCONNECTED flag when the dentry
is removed from the parent's d_subdirs list.
Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
Cc: stable@kernel.org (2.6.38, needs commit c826cb7dfce8 "dcache.c:
create helper function for duplicated functionality" )
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
diff --git a/fs/dcache.c b/fs/dcache.c
index 361882a..a39fe47 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -296,8 +296,12 @@ static struct dentry *d_kill(struct dentry *dentry, struct dentry *parent)
__releases(parent->d_lock)
__releases(dentry->d_inode->i_lock)
{
- dentry->d_parent = NULL;
list_del(&dentry->d_u.d_child);
+ /*
+ * Inform try_to_ascend() that we are no longer attached to the
+ * dentry tree
+ */
+ dentry->d_flags |= DCACHE_DISCONNECTED;
if (parent)
spin_unlock(&parent->d_lock);
dentry_iput(dentry);
@@ -1030,6 +1034,7 @@ static struct dentry *try_to_ascend(struct dentry *old, int locked, unsigned seq
* or deletion
*/
if (new != old->d_parent ||
+ (old->d_flags & DCACHE_DISCONNECTED) ||
(!locked && read_seqretry(&rename_lock, seq))) {
spin_unlock(&new->d_lock);
new = NULL;
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: DCACHE_DISCONNECTED use in try_to_ascend/d_kill
2012-07-12 22:28 DCACHE_DISCONNECTED use in try_to_ascend/d_kill J. Bruce Fields
@ 2012-07-12 22:40 ` J. Bruce Fields
2012-07-13 11:55 ` J. Bruce Fields
0 siblings, 1 reply; 3+ messages in thread
From: J. Bruce Fields @ 2012-07-12 22:40 UTC (permalink / raw)
To: Trond Myklebust; +Cc: linux-nfs, linux-fsdevel
On Thu, Jul 12, 2012 at 06:28:49PM -0400, bfields wrote:
> I've been trying to figure out if we can pare down DCACHE_DISCONNECTED
> uses to only those required by exportfs, and ran across this one.
There's also one DCACHE_DISCONNECTED use in fs/nfs/dir.c that I was
hoping could be eliminated. It was added by Al with
d9e80b7de91db05c1c4d2e5ebbfd70b3b3ba0e0f. It looks totally theoretical
to me (nfs isn't exportable, hence shouldn't see DCACHE_DISCONNECTED
dentries), but maybe I'm missing something.
At a minimum maybe we could pull a comment out of Al's commit?:
--b.
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index f430057..b96c687 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1194,6 +1194,12 @@ out_zap_parent:
/* If we have submounts, don't unhash ! */
if (have_submounts(dentry))
goto out_valid;
+ /*
+ * We can't d_drop the root of a disconnected tree:
+ * its d_hash is on the s_anon list and d_drop() would hide
+ * it from shrink_dcache_for_unmount(), leading to busy
+ * inodes on unmount and further oopses.
+ */
if (dentry->d_flags & DCACHE_DISCONNECTED)
goto out_valid;
shrink_dcache_parent(dentry);
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: DCACHE_DISCONNECTED use in try_to_ascend/d_kill
2012-07-12 22:40 ` J. Bruce Fields
@ 2012-07-13 11:55 ` J. Bruce Fields
0 siblings, 0 replies; 3+ messages in thread
From: J. Bruce Fields @ 2012-07-13 11:55 UTC (permalink / raw)
To: Trond Myklebust; +Cc: linux-nfs, linux-fsdevel
On Thu, Jul 12, 2012 at 06:40:08PM -0400, J. Bruce Fields wrote:
> On Thu, Jul 12, 2012 at 06:28:49PM -0400, bfields wrote:
> > I've been trying to figure out if we can pare down DCACHE_DISCONNECTED
> > uses to only those required by exportfs, and ran across this one.
>
> There's also one DCACHE_DISCONNECTED use in fs/nfs/dir.c that I was
> hoping could be eliminated. It was added by Al with
> d9e80b7de91db05c1c4d2e5ebbfd70b3b3ba0e0f. It looks totally theoretical
> to me (nfs isn't exportable, hence shouldn't see DCACHE_DISCONNECTED
> dentries), but maybe I'm missing something.
Ignore me, I'm confused: there's a different use of DCACHE_DISCONECTED
here (for v2/v3 root dentries that we expect may one day be hooked into
a parent we find someplace else?). It looks to me like it clashes with
nfsd/exportfs's use. I'm not sure what to do about that.
--b.
>
> At a minimum maybe we could pull a comment out of Al's commit?:
>
> --b.
>
> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> index f430057..b96c687 100644
> --- a/fs/nfs/dir.c
> +++ b/fs/nfs/dir.c
> @@ -1194,6 +1194,12 @@ out_zap_parent:
> /* If we have submounts, don't unhash ! */
> if (have_submounts(dentry))
> goto out_valid;
> + /*
> + * We can't d_drop the root of a disconnected tree:
> + * its d_hash is on the s_anon list and d_drop() would hide
> + * it from shrink_dcache_for_unmount(), leading to busy
> + * inodes on unmount and further oopses.
> + */
> if (dentry->d_flags & DCACHE_DISCONNECTED)
> goto out_valid;
> shrink_dcache_parent(dentry);
> --
> 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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-07-13 11:55 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-07-12 22:28 DCACHE_DISCONNECTED use in try_to_ascend/d_kill J. Bruce Fields
2012-07-12 22:40 ` J. Bruce Fields
2012-07-13 11:55 ` J. Bruce Fields
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).