Summary of changes:
v2: remove -ERESTARTSYS error handling in nfs_rename
hardcode isdir value in fsnotify_nameremove call for sillyrenames
As Trond pointed out recently, it makes little sense to maintain two
separate sets of functions for handling RENAMEs.
This patchset converts nfs_rename to use the asynchronous RENAME
infrastructure that is already in place for sillyrenames. It also
does some cleanup to remove some minor layering violations, and
adds a patch to make the sillyrename code emit a fsnotify_nameremove
when a sillyrename succeeds.
I've lightly tested this set and it seems to do the right thing,
but it obviously could stand some time in linux-next.
Jeff Layton (5):
nfs: abstract out code needed to complete a sillyrename
nfs: make nfs_async_rename non-static
nfs: convert nfs_rename to use async_rename infrastructure
nfs: remove synchronous rename code
nfs: emit a fsnotify_nameremove call in sillyrename codepath
fs/nfs/dir.c | 13 +++++++++++--
fs/nfs/internal.h | 7 +++++++
fs/nfs/nfs3proc.c | 36 ------------------------------------
fs/nfs/nfs4proc.c | 44 --------------------------------------------
fs/nfs/proc.c | 25 -------------------------
fs/nfs/unlink.c | 35 ++++++++++++++++++++++++++++++-----
include/linux/nfs_fs.h | 1 -
include/linux/nfs_xdr.h | 3 +--
8 files changed, 49 insertions(+), 115 deletions(-)
--
1.8.5.3
Just to let you know, I tested these a bit this morning and I haven't had any problems.
Anna
On 03/17/2014 07:06 AM, Jeff Layton wrote:
> Summary of changes:
>
> v2: remove -ERESTARTSYS error handling in nfs_rename
> hardcode isdir value in fsnotify_nameremove call for sillyrenames
>
> As Trond pointed out recently, it makes little sense to maintain two
> separate sets of functions for handling RENAMEs.
>
> This patchset converts nfs_rename to use the asynchronous RENAME
> infrastructure that is already in place for sillyrenames. It also
> does some cleanup to remove some minor layering violations, and
> adds a patch to make the sillyrename code emit a fsnotify_nameremove
> when a sillyrename succeeds.
>
> I've lightly tested this set and it seems to do the right thing,
> but it obviously could stand some time in linux-next.
>
> Jeff Layton (5):
> nfs: abstract out code needed to complete a sillyrename
> nfs: make nfs_async_rename non-static
> nfs: convert nfs_rename to use async_rename infrastructure
> nfs: remove synchronous rename code
> nfs: emit a fsnotify_nameremove call in sillyrename codepath
>
> fs/nfs/dir.c | 13 +++++++++++--
> fs/nfs/internal.h | 7 +++++++
> fs/nfs/nfs3proc.c | 36 ------------------------------------
> fs/nfs/nfs4proc.c | 44 --------------------------------------------
> fs/nfs/proc.c | 25 -------------------------
> fs/nfs/unlink.c | 35 ++++++++++++++++++++++++++++++-----
> include/linux/nfs_fs.h | 1 -
> include/linux/nfs_xdr.h | 3 +--
> 8 files changed, 49 insertions(+), 115 deletions(-)
>
There isn't much sense in maintaining two separate versions of rename
code. Convert nfs_rename to use the asynchronous rename infrastructure
that nfs_sillyrename uses, and emulate synchronous behavior by having
the task just wait on the reply.
Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfs/dir.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 4a48fe4b84b6..e6f8e2c97653 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1911,6 +1911,7 @@ int nfs_rename(struct inode *old_dir, struct dentry *old_dentry,
struct inode *old_inode = old_dentry->d_inode;
struct inode *new_inode = new_dentry->d_inode;
struct dentry *dentry = NULL, *rehash = NULL;
+ struct rpc_task *task;
int error = -EBUSY;
dfprintk(VFS, "NFS: rename(%pd2 -> %pd2, ct=%d)\n",
@@ -1958,8 +1959,16 @@ int nfs_rename(struct inode *old_dir, struct dentry *old_dentry,
if (new_inode != NULL)
NFS_PROTO(new_inode)->return_delegation(new_inode);
- error = NFS_PROTO(old_dir)->rename(old_dir, &old_dentry->d_name,
- new_dir, &new_dentry->d_name);
+ task = nfs_async_rename(old_dir, new_dir, old_dentry, new_dentry, NULL);
+ if (IS_ERR(task)) {
+ error = PTR_ERR(task);
+ goto out;
+ }
+
+ error = rpc_wait_for_completion_task(task);
+ if (error == 0)
+ error = task->tk_status;
+ rpc_put_task(task);
nfs_mark_for_revalidate(old_inode);
out:
if (rehash)
--
1.8.5.3
If a file is sillyrenamed, then the generic vfs_unlink code will skip
emitting fsnotify events for it.
This patch has the sillyrename code do that instead.
In truth this is a little bit odd since we aren't actually removing the
dentry per-se, but renaming it. Still, this is probably the right thing
to do since it's what userland apps expect to see when an unlink()
occurs or some file is renamed on top of the dentry.
Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfs/unlink.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/fs/nfs/unlink.c b/fs/nfs/unlink.c
index 818ded7b7b74..de54129336c6 100644
--- a/fs/nfs/unlink.c
+++ b/fs/nfs/unlink.c
@@ -14,6 +14,7 @@
#include <linux/sched.h>
#include <linux/wait.h>
#include <linux/namei.h>
+#include <linux/fsnotify.h>
#include "internal.h"
#include "nfs4_fs.h"
@@ -465,8 +466,18 @@ nfs_async_rename(struct inode *old_dir, struct inode *new_dir,
static void
nfs_complete_sillyrename(struct rpc_task *task, struct nfs_renamedata *data)
{
- if (task->tk_status != 0)
- nfs_cancel_async_unlink(data->old_dentry);
+ struct dentry *dentry = data->old_dentry;
+
+ if (task->tk_status != 0) {
+ nfs_cancel_async_unlink(dentry);
+ return;
+ }
+
+ /*
+ * vfs_unlink and the like do not issue this when a file is
+ * sillyrenamed, so do it here.
+ */
+ fsnotify_nameremove(dentry, 0);
}
#define SILLYNAME_PREFIX ".nfs"
--
1.8.5.3
The async rename code is currently "polluted" with some parts that are
really just for sillyrenames. Add a new "complete" operation vector to
the nfs_renamedata to separate out the stuff that just needs to be done
for a sillyrename.
Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfs/unlink.c | 22 ++++++++++++++++++----
include/linux/nfs_xdr.h | 1 +
2 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/fs/nfs/unlink.c b/fs/nfs/unlink.c
index 11d78944de79..3e6798c9ba1f 100644
--- a/fs/nfs/unlink.c
+++ b/fs/nfs/unlink.c
@@ -353,8 +353,8 @@ static void nfs_async_rename_done(struct rpc_task *task, void *calldata)
return;
}
- if (task->tk_status != 0)
- nfs_cancel_async_unlink(old_dentry);
+ if (data->complete)
+ data->complete(task, data);
}
/**
@@ -401,7 +401,8 @@ static const struct rpc_call_ops nfs_rename_ops = {
*/
static struct rpc_task *
nfs_async_rename(struct inode *old_dir, struct inode *new_dir,
- struct dentry *old_dentry, struct dentry *new_dentry)
+ struct dentry *old_dentry, struct dentry *new_dentry,
+ void (*complete)(struct rpc_task *, struct nfs_renamedata *))
{
struct nfs_renamedata *data;
struct rpc_message msg = { };
@@ -438,6 +439,7 @@ nfs_async_rename(struct inode *old_dir, struct inode *new_dir,
data->new_dentry = dget(new_dentry);
nfs_fattr_init(&data->old_fattr);
nfs_fattr_init(&data->new_fattr);
+ data->complete = complete;
/* set up nfs_renameargs */
data->args.old_dir = NFS_FH(old_dir);
@@ -456,6 +458,17 @@ nfs_async_rename(struct inode *old_dir, struct inode *new_dir,
return rpc_run_task(&task_setup_data);
}
+/*
+ * Perform tasks needed when a sillyrename is done such as cancelling the
+ * queued async unlink if it failed.
+ */
+static void
+nfs_complete_sillyrename(struct rpc_task *task, struct nfs_renamedata *data)
+{
+ if (task->tk_status != 0)
+ nfs_cancel_async_unlink(data->old_dentry);
+}
+
#define SILLYNAME_PREFIX ".nfs"
#define SILLYNAME_PREFIX_LEN ((unsigned)sizeof(SILLYNAME_PREFIX) - 1)
#define SILLYNAME_FILEID_LEN ((unsigned)sizeof(u64) << 1)
@@ -548,7 +561,8 @@ nfs_sillyrename(struct inode *dir, struct dentry *dentry)
}
/* run the rename task, undo unlink if it fails */
- task = nfs_async_rename(dir, dir, dentry, sdentry);
+ task = nfs_async_rename(dir, dir, dentry, sdentry,
+ nfs_complete_sillyrename);
if (IS_ERR(task)) {
error = -EBUSY;
nfs_cancel_async_unlink(dentry);
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index 5624e4e2763c..440d7b42525a 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -1402,6 +1402,7 @@ struct nfs_renamedata {
struct inode *new_dir;
struct dentry *new_dentry;
struct nfs_fattr new_fattr;
+ void (*complete)(struct rpc_task *, struct nfs_renamedata *);
};
struct nfs_access_entry;
--
1.8.5.3
Now that nfs_rename uses the async infrastructure, we can remove this.
Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfs/nfs3proc.c | 36 ------------------------------------
fs/nfs/nfs4proc.c | 44 --------------------------------------------
fs/nfs/proc.c | 25 -------------------------
include/linux/nfs_xdr.h | 2 --
4 files changed, 107 deletions(-)
diff --git a/fs/nfs/nfs3proc.c b/fs/nfs/nfs3proc.c
index a462ef0fb5d6..db60149c4579 100644
--- a/fs/nfs/nfs3proc.c
+++ b/fs/nfs/nfs3proc.c
@@ -479,41 +479,6 @@ nfs3_proc_rename_done(struct rpc_task *task, struct inode *old_dir,
}
static int
-nfs3_proc_rename(struct inode *old_dir, struct qstr *old_name,
- struct inode *new_dir, struct qstr *new_name)
-{
- struct nfs_renameargs arg = {
- .old_dir = NFS_FH(old_dir),
- .old_name = old_name,
- .new_dir = NFS_FH(new_dir),
- .new_name = new_name,
- };
- struct nfs_renameres res;
- struct rpc_message msg = {
- .rpc_proc = &nfs3_procedures[NFS3PROC_RENAME],
- .rpc_argp = &arg,
- .rpc_resp = &res,
- };
- int status = -ENOMEM;
-
- dprintk("NFS call rename %s -> %s\n", old_name->name, new_name->name);
-
- res.old_fattr = nfs_alloc_fattr();
- res.new_fattr = nfs_alloc_fattr();
- if (res.old_fattr == NULL || res.new_fattr == NULL)
- goto out;
-
- status = rpc_call_sync(NFS_CLIENT(old_dir), &msg, 0);
- nfs_post_op_update_inode(old_dir, res.old_fattr);
- nfs_post_op_update_inode(new_dir, res.new_fattr);
-out:
- nfs_free_fattr(res.old_fattr);
- nfs_free_fattr(res.new_fattr);
- dprintk("NFS reply rename: %d\n", status);
- return status;
-}
-
-static int
nfs3_proc_link(struct inode *inode, struct inode *dir, struct qstr *name)
{
struct nfs3_linkargs arg = {
@@ -968,7 +933,6 @@ const struct nfs_rpc_ops nfs_v3_clientops = {
.unlink_setup = nfs3_proc_unlink_setup,
.unlink_rpc_prepare = nfs3_proc_unlink_rpc_prepare,
.unlink_done = nfs3_proc_unlink_done,
- .rename = nfs3_proc_rename,
.rename_setup = nfs3_proc_rename_setup,
.rename_rpc_prepare = nfs3_proc_rename_rpc_prepare,
.rename_done = nfs3_proc_rename_done,
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 450bfedbe2f4..a501a5a82a42 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -3507,49 +3507,6 @@ static int nfs4_proc_rename_done(struct rpc_task *task, struct inode *old_dir,
return 1;
}
-static int _nfs4_proc_rename(struct inode *old_dir, struct qstr *old_name,
- struct inode *new_dir, struct qstr *new_name)
-{
- struct nfs_server *server = NFS_SERVER(old_dir);
- struct nfs_renameargs arg = {
- .old_dir = NFS_FH(old_dir),
- .new_dir = NFS_FH(new_dir),
- .old_name = old_name,
- .new_name = new_name,
- };
- struct nfs_renameres res = {
- .server = server,
- };
- struct rpc_message msg = {
- .rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_RENAME],
- .rpc_argp = &arg,
- .rpc_resp = &res,
- };
- int status = -ENOMEM;
-
- status = nfs4_call_sync(server->client, server, &msg, &arg.seq_args, &res.seq_res, 1);
- if (!status) {
- update_changeattr(old_dir, &res.old_cinfo);
- update_changeattr(new_dir, &res.new_cinfo);
- }
- return status;
-}
-
-static int nfs4_proc_rename(struct inode *old_dir, struct qstr *old_name,
- struct inode *new_dir, struct qstr *new_name)
-{
- struct nfs4_exception exception = { };
- int err;
- do {
- err = _nfs4_proc_rename(old_dir, old_name,
- new_dir, new_name);
- trace_nfs4_rename(old_dir, old_name, new_dir, new_name, err);
- err = nfs4_handle_exception(NFS_SERVER(old_dir), err,
- &exception);
- } while (exception.retry);
- return err;
-}
-
static int _nfs4_proc_link(struct inode *inode, struct inode *dir, struct qstr *name)
{
struct nfs_server *server = NFS_SERVER(inode);
@@ -8408,7 +8365,6 @@ const struct nfs_rpc_ops nfs_v4_clientops = {
.unlink_setup = nfs4_proc_unlink_setup,
.unlink_rpc_prepare = nfs4_proc_unlink_rpc_prepare,
.unlink_done = nfs4_proc_unlink_done,
- .rename = nfs4_proc_rename,
.rename_setup = nfs4_proc_rename_setup,
.rename_rpc_prepare = nfs4_proc_rename_rpc_prepare,
.rename_done = nfs4_proc_rename_done,
diff --git a/fs/nfs/proc.c b/fs/nfs/proc.c
index fddbba2d9eff..e55ce9e8b034 100644
--- a/fs/nfs/proc.c
+++ b/fs/nfs/proc.c
@@ -357,30 +357,6 @@ nfs_proc_rename_done(struct rpc_task *task, struct inode *old_dir,
}
static int
-nfs_proc_rename(struct inode *old_dir, struct qstr *old_name,
- struct inode *new_dir, struct qstr *new_name)
-{
- struct nfs_renameargs arg = {
- .old_dir = NFS_FH(old_dir),
- .old_name = old_name,
- .new_dir = NFS_FH(new_dir),
- .new_name = new_name,
- };
- struct rpc_message msg = {
- .rpc_proc = &nfs_procedures[NFSPROC_RENAME],
- .rpc_argp = &arg,
- };
- int status;
-
- dprintk("NFS call rename %s -> %s\n", old_name->name, new_name->name);
- status = rpc_call_sync(NFS_CLIENT(old_dir), &msg, 0);
- nfs_mark_for_revalidate(old_dir);
- nfs_mark_for_revalidate(new_dir);
- dprintk("NFS reply rename: %d\n", status);
- return status;
-}
-
-static int
nfs_proc_link(struct inode *inode, struct inode *dir, struct qstr *name)
{
struct nfs_linkargs arg = {
@@ -745,7 +721,6 @@ const struct nfs_rpc_ops nfs_v2_clientops = {
.unlink_setup = nfs_proc_unlink_setup,
.unlink_rpc_prepare = nfs_proc_unlink_rpc_prepare,
.unlink_done = nfs_proc_unlink_done,
- .rename = nfs_proc_rename,
.rename_setup = nfs_proc_rename_setup,
.rename_rpc_prepare = nfs_proc_rename_rpc_prepare,
.rename_done = nfs_proc_rename_done,
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index 440d7b42525a..6fb5b2335b59 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -1445,8 +1445,6 @@ struct nfs_rpc_ops {
void (*unlink_setup) (struct rpc_message *, struct inode *dir);
void (*unlink_rpc_prepare) (struct rpc_task *, struct nfs_unlinkdata *);
int (*unlink_done) (struct rpc_task *, struct inode *);
- int (*rename) (struct inode *, struct qstr *,
- struct inode *, struct qstr *);
void (*rename_setup) (struct rpc_message *msg, struct inode *dir);
void (*rename_rpc_prepare)(struct rpc_task *task, struct nfs_renamedata *);
int (*rename_done) (struct rpc_task *task, struct inode *old_dir, struct inode *new_dir);
--
1.8.5.3
...and move the prototype for nfs_sillyrename to internal.h.
Signed-off-by: Jeff Layton <[email protected]>
---
fs/nfs/internal.h | 7 +++++++
fs/nfs/unlink.c | 2 +-
include/linux/nfs_fs.h | 1 -
3 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index b46cf5a67329..ae765a56b84f 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -474,6 +474,13 @@ extern int nfs_migrate_page(struct address_space *,
#define nfs_migrate_page NULL
#endif
+/* unlink.c */
+extern struct rpc_task *
+nfs_async_rename(struct inode *old_dir, struct inode *new_dir,
+ struct dentry *old_dentry, struct dentry *new_dentry,
+ void (*complete)(struct rpc_task *, struct nfs_renamedata *));
+extern int nfs_sillyrename(struct inode *dir, struct dentry *dentry);
+
/* direct.c */
void nfs_init_cinfo_from_dreq(struct nfs_commit_info *cinfo,
struct nfs_direct_req *dreq);
diff --git a/fs/nfs/unlink.c b/fs/nfs/unlink.c
index 3e6798c9ba1f..818ded7b7b74 100644
--- a/fs/nfs/unlink.c
+++ b/fs/nfs/unlink.c
@@ -399,7 +399,7 @@ static const struct rpc_call_ops nfs_rename_ops = {
*
* It's expected that valid references to the dentries and inodes are held
*/
-static struct rpc_task *
+struct rpc_task *
nfs_async_rename(struct inode *old_dir, struct inode *new_dir,
struct dentry *old_dentry, struct dentry *new_dentry,
void (*complete)(struct rpc_task *, struct nfs_renamedata *))
diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index 0ae5807480f4..85d31ccc00f8 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -510,7 +510,6 @@ extern void nfs_complete_unlink(struct dentry *dentry, struct inode *);
extern void nfs_wait_on_sillyrename(struct dentry *dentry);
extern void nfs_block_sillyrename(struct dentry *dentry);
extern void nfs_unblock_sillyrename(struct dentry *dentry);
-extern int nfs_sillyrename(struct inode *dir, struct dentry *dentry);
/*
* linux/fs/nfs/write.c
--
1.8.5.3