From 322b2b9032f4beba6f1c4158852a5a5b9ab841d7 Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Fri, 11 Jan 2013 16:39:51 -0500 Subject: Revert "NFS: add nfs_sb_deactive_async to avoid deadlock" This reverts commit 324d003b0cd82151adbaecefef57b73f7959a469. The deadlock turned out to be caused by a workqueue limitation that has now been worked around in the RPC code (see comment in rpc_free_task). Signed-off-by: Trond Myklebust --- fs/nfs/unlink.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs/nfs/unlink.c') diff --git a/fs/nfs/unlink.c b/fs/nfs/unlink.c index 3f79c77153b8..13cea637eff8 100644 --- a/fs/nfs/unlink.c +++ b/fs/nfs/unlink.c @@ -95,7 +95,7 @@ static void nfs_async_unlink_release(void *calldata) nfs_dec_sillycount(data->dir); nfs_free_unlinkdata(data); - nfs_sb_deactive_async(sb); + nfs_sb_deactive(sb); } static void nfs_unlink_prepare(struct rpc_task *task, void *calldata) -- cgit v1.2.3 From 96aa1549afa6c79ae4a4f099de861efd218c38d8 Mon Sep 17 00:00:00 2001 From: Tim Gardner Date: Tue, 12 Feb 2013 13:03:42 -0700 Subject: nfs: remove kfree() redundant null checks smatch analysis: fs/nfs/getroot.c:130 nfs_get_root() info: redundant null check on name calling kfree() fs/nfs/unlink.c:272 nfs_async_unlink() info: redundant null check on devname_garbage calling kfree() Cc: Trond Myklebust Cc: linux-nfs@vger.kernel.org Signed-off-by: Tim Gardner Signed-off-by: Trond Myklebust --- fs/nfs/getroot.c | 3 +-- fs/nfs/unlink.c | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) (limited to 'fs/nfs/unlink.c') diff --git a/fs/nfs/getroot.c b/fs/nfs/getroot.c index 033803c36644..44efaa8c5f78 100644 --- a/fs/nfs/getroot.c +++ b/fs/nfs/getroot.c @@ -126,8 +126,7 @@ struct dentry *nfs_get_root(struct super_block *sb, struct nfs_fh *mntfh, } spin_unlock(&ret->d_lock); out: - if (name) - kfree(name); + kfree(name); nfs_free_fattr(fsinfo.fattr); return ret; } diff --git a/fs/nfs/unlink.c b/fs/nfs/unlink.c index 13cea637eff8..d26a32f5b53b 100644 --- a/fs/nfs/unlink.c +++ b/fs/nfs/unlink.c @@ -268,8 +268,7 @@ nfs_async_unlink(struct inode *dir, struct dentry *dentry) * point dentry is definitely not a root, so we won't need * that anymore. */ - if (devname_garbage) - kfree(devname_garbage); + kfree(devname_garbage); return 0; out_unlock: spin_unlock(&dentry->d_lock); -- cgit v1.2.3 From 5a7a613a47a715711b3f2d3322a0eac21d459166 Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Fri, 22 Feb 2013 12:53:43 -0500 Subject: NFS: Don't allow NFS silly-renamed files to be deleted, no signal Commit 73ca100 broke the code that prevents the client from deleting a silly renamed dentry. This affected "delete on last close" semantics as after that commit, nothing prevented removal of silly-renamed files. As a result, a process holding a file open could easily get an ESTALE on the file in a directory where some other process issued 'rm -rf some_dir_containing_the_file' twice. Before the commit, any attempt at unlinking silly renamed files would fail inside may_delete() with -EBUSY because of the DCACHE_NFSFS_RENAMED flag. The following testcase demonstrates the problem: tail -f /nfsmnt/dir/file & rm -rf /nfsmnt/dir rm -rf /nfsmnt/dir # second removal does not fail, 'tail' process receives ESTALE The problem with the above commit is that it unhashes the old and new dentries from the lookup path, even in the normal case when a signal is not encountered and it would have been safe to call d_move. Unfortunately the old dentry has the special DCACHE_NFSFS_RENAMED flag set on it. Unhashing has the side-effect that future lookups call d_alloc(), allocating a new dentry without the special flag for any silly-renamed files. As a result, subsequent calls to unlink silly renamed files do not fail but allow the removal to go through. This will result in ESTALE errors for any other process doing operations on the file. To fix this, go back to using d_move on success. For the signal case, it's unclear what we may safely do beyond d_drop. Reported-by: Dave Wysochanski Signed-off-by: Trond Myklebust Acked-by: Jeff Layton Cc: stable@vger.kernel.org --- fs/nfs/unlink.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) (limited to 'fs/nfs/unlink.c') diff --git a/fs/nfs/unlink.c b/fs/nfs/unlink.c index d26a32f5b53b..1f1f38f0c5d5 100644 --- a/fs/nfs/unlink.c +++ b/fs/nfs/unlink.c @@ -335,20 +335,14 @@ static void nfs_async_rename_done(struct rpc_task *task, void *calldata) struct inode *old_dir = data->old_dir; struct inode *new_dir = data->new_dir; struct dentry *old_dentry = data->old_dentry; - struct dentry *new_dentry = data->new_dentry; if (!NFS_PROTO(old_dir)->rename_done(task, old_dir, new_dir)) { rpc_restart_call_prepare(task); return; } - if (task->tk_status != 0) { + if (task->tk_status != 0) nfs_cancel_async_unlink(old_dentry); - return; - } - - d_drop(old_dentry); - d_drop(new_dentry); } /** @@ -549,6 +543,18 @@ nfs_sillyrename(struct inode *dir, struct dentry *dentry) error = rpc_wait_for_completion_task(task); if (error == 0) error = task->tk_status; + switch (error) { + case 0: + /* The rename succeeded */ + nfs_set_verifier(dentry, nfs_save_change_attribute(dir)); + d_move(dentry, sdentry); + break; + case -ERESTARTSYS: + /* The result of the rename is unknown. Play it safe by + * forcing a new lookup */ + d_drop(dentry); + d_drop(sdentry); + } rpc_put_task(task); out_dput: dput(sdentry); -- cgit v1.2.3