* [PATCH 0/8] nfs: ensure that sillyrenames run to completion (try #2)
@ 2010-09-15 13:23 Jeff Layton
2010-09-15 13:23 ` [PATCH 1/8] nfs: eliminate nfs3_renameargs Jeff Layton
` (7 more replies)
0 siblings, 8 replies; 19+ messages in thread
From: Jeff Layton @ 2010-09-15 13:23 UTC (permalink / raw)
To: trond.myklebust; +Cc: linux-nfs
This patchset is pretty much identical to the one that I sent last week.
It has one minor fix -- I fixed it to use nfs_free_fattr instead of
kfree in nfs_async_rename_release. I also cleaned up the comments a bit.
We had a bug report recently where someone was complaining about
silly-renamed files being left around:
https://bugzilla.redhat.com/show_bug.cgi?id=511901
...they attached a reproducer to the bug which involves a
pthreads-based program killing a child thread that's unlinking and
closing a file.
The unlink triggers a sillyrename (since the file is still open). The
parent kills the child thread and if timed just right, the child thread
will be killed after the RENAME call is issued but before the reply is
processed. The file will end up renamed on the server, but the client
won't be aware of it and won't unlink it during dentry_iput.
This patchset is intended to prevent this from happening by having
nfs_sillyrename use an asynchronous rename call and simply wait for the
call to complete in TASK_KILLABLE sleep before proceeding. If the task
is killed while the rename RPC is on the wire, it'll still run to
completion even though the thread that initiated it was killed off.
To facilitate this, the set first changes all of the existing rename
calls to use standardized argument and response containers. The
nfs_sillyrename call is then moved to unlink.c (where the rest of the
sillyrename code lives), and then is converted to use an asynchronous
RPC call to handle the rename.
I've tested this by running the reproducer in the above bug report
in a loop for several hours and never ended up with a leftover .nfs*
file.
I'd like to see this in 2.6.37 if possible.
Jeff Layton (8):
nfs: eliminate nfs3_renameargs
nfs: convert nfs_renameargs to use qstr structs
nfs: eliminate nfs4_rename_arg
nfs: standardize the rename response container
nfs: move nfs_sillyrename to unlink.c
nfs: add a rename_setup nfs_rpc_op for async renames
nfs: add rename_done nfs_rpc_op
nfs: make sillyrename an async operation
fs/nfs/dir.c | 70 -------------
fs/nfs/nfs2xdr.c | 8 +-
fs/nfs/nfs3proc.c | 62 +++++++++---
fs/nfs/nfs3xdr.c | 16 ++--
fs/nfs/nfs4proc.c | 43 ++++++++-
fs/nfs/nfs4xdr.c | 4 +-
fs/nfs/proc.c | 34 +++++-
fs/nfs/unlink.c | 253 ++++++++++++++++++++++++++++++++++++++++++++++-
include/linux/nfs_fs.h | 2 +-
include/linux/nfs_xdr.h | 64 +++++--------
10 files changed, 406 insertions(+), 150 deletions(-)
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 1/8] nfs: eliminate nfs3_renameargs
2010-09-15 13:23 [PATCH 0/8] nfs: ensure that sillyrenames run to completion (try #2) Jeff Layton
@ 2010-09-15 13:23 ` Jeff Layton
2010-09-15 13:23 ` [PATCH 2/8] nfs: convert nfs_renameargs to use qstr structs Jeff Layton
` (6 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Jeff Layton @ 2010-09-15 13:23 UTC (permalink / raw)
To: trond.myklebust; +Cc: linux-nfs
It's exactly the same as nfs_renameargs, so move it to the area of the
header where protocol-neutral defintions live and eliminate the v3
specific variant.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
fs/nfs/nfs3proc.c | 2 +-
fs/nfs/nfs3xdr.c | 2 +-
include/linux/nfs_xdr.h | 30 ++++++++++++------------------
3 files changed, 14 insertions(+), 20 deletions(-)
diff --git a/fs/nfs/nfs3proc.c b/fs/nfs/nfs3proc.c
index fabb4f2..b53b1a9 100644
--- a/fs/nfs/nfs3proc.c
+++ b/fs/nfs/nfs3proc.c
@@ -442,7 +442,7 @@ static int
nfs3_proc_rename(struct inode *old_dir, struct qstr *old_name,
struct inode *new_dir, struct qstr *new_name)
{
- struct nfs3_renameargs arg = {
+ struct nfs_renameargs arg = {
.fromfh = NFS_FH(old_dir),
.fromname = old_name->name,
.fromlen = old_name->len,
diff --git a/fs/nfs/nfs3xdr.c b/fs/nfs/nfs3xdr.c
index 9769704..d6506f8 100644
--- a/fs/nfs/nfs3xdr.c
+++ b/fs/nfs/nfs3xdr.c
@@ -442,7 +442,7 @@ nfs3_xdr_mknodargs(struct rpc_rqst *req, __be32 *p, struct nfs3_mknodargs *args)
* Encode RENAME arguments
*/
static int
-nfs3_xdr_renameargs(struct rpc_rqst *req, __be32 *p, struct nfs3_renameargs *args)
+nfs3_xdr_renameargs(struct rpc_rqst *req, __be32 *p, struct nfs_renameargs *args)
{
p = xdr_encode_fhandle(p, args->fromfh);
p = xdr_encode_array(p, args->fromname, args->fromlen);
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index fc46192..c606f60 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -400,6 +400,18 @@ struct nfs_removeres {
};
/*
+ * Common arguments to the rename call
+ */
+struct nfs_renameargs {
+ struct nfs_fh * fromfh;
+ const char * fromname;
+ unsigned int fromlen;
+ struct nfs_fh * tofh;
+ const char * toname;
+ unsigned int tolen;
+};
+
+/*
* Argument struct for decode_entry function
*/
struct nfs_entry {
@@ -434,15 +446,6 @@ struct nfs_createargs {
struct iattr * sattr;
};
-struct nfs_renameargs {
- struct nfs_fh * fromfh;
- const char * fromname;
- unsigned int fromlen;
- struct nfs_fh * tofh;
- const char * toname;
- unsigned int tolen;
-};
-
struct nfs_setattrargs {
struct nfs_fh * fh;
nfs4_stateid stateid;
@@ -586,15 +589,6 @@ struct nfs3_mknodargs {
dev_t rdev;
};
-struct nfs3_renameargs {
- struct nfs_fh * fromfh;
- const char * fromname;
- unsigned int fromlen;
- struct nfs_fh * tofh;
- const char * toname;
- unsigned int tolen;
-};
-
struct nfs3_linkargs {
struct nfs_fh * fromfh;
struct nfs_fh * tofh;
--
1.7.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 2/8] nfs: convert nfs_renameargs to use qstr structs
2010-09-15 13:23 [PATCH 0/8] nfs: ensure that sillyrenames run to completion (try #2) Jeff Layton
2010-09-15 13:23 ` [PATCH 1/8] nfs: eliminate nfs3_renameargs Jeff Layton
@ 2010-09-15 13:23 ` Jeff Layton
2010-09-15 13:23 ` [PATCH 3/8] nfs: eliminate nfs4_rename_arg Jeff Layton
` (5 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Jeff Layton @ 2010-09-15 13:23 UTC (permalink / raw)
To: trond.myklebust; +Cc: linux-nfs
The v4 version already does this.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
fs/nfs/nfs2xdr.c | 8 ++++----
fs/nfs/nfs3proc.c | 10 ++++------
fs/nfs/nfs3xdr.c | 8 ++++----
fs/nfs/proc.c | 10 ++++------
include/linux/nfs_xdr.h | 10 ++++------
5 files changed, 20 insertions(+), 26 deletions(-)
diff --git a/fs/nfs/nfs2xdr.c b/fs/nfs/nfs2xdr.c
index db8846a..e15bc03 100644
--- a/fs/nfs/nfs2xdr.c
+++ b/fs/nfs/nfs2xdr.c
@@ -337,10 +337,10 @@ nfs_xdr_createargs(struct rpc_rqst *req, __be32 *p, struct nfs_createargs *args)
static int
nfs_xdr_renameargs(struct rpc_rqst *req, __be32 *p, struct nfs_renameargs *args)
{
- p = xdr_encode_fhandle(p, args->fromfh);
- p = xdr_encode_array(p, args->fromname, args->fromlen);
- p = xdr_encode_fhandle(p, args->tofh);
- p = xdr_encode_array(p, args->toname, args->tolen);
+ p = xdr_encode_fhandle(p, args->old_dir);
+ p = xdr_encode_array(p, args->old_name->name, args->old_name->len);
+ p = xdr_encode_fhandle(p, args->new_dir);
+ p = xdr_encode_array(p, args->new_name->name, args->new_name->len);
req->rq_slen = xdr_adjust_iovec(req->rq_svec, p);
return 0;
}
diff --git a/fs/nfs/nfs3proc.c b/fs/nfs/nfs3proc.c
index b53b1a9..d8557b3 100644
--- a/fs/nfs/nfs3proc.c
+++ b/fs/nfs/nfs3proc.c
@@ -443,12 +443,10 @@ nfs3_proc_rename(struct inode *old_dir, struct qstr *old_name,
struct inode *new_dir, struct qstr *new_name)
{
struct nfs_renameargs arg = {
- .fromfh = NFS_FH(old_dir),
- .fromname = old_name->name,
- .fromlen = old_name->len,
- .tofh = NFS_FH(new_dir),
- .toname = new_name->name,
- .tolen = new_name->len
+ .old_dir = NFS_FH(old_dir),
+ .old_name = old_name,
+ .new_dir = NFS_FH(new_dir),
+ .new_name = new_name,
};
struct nfs3_renameres res;
struct rpc_message msg = {
diff --git a/fs/nfs/nfs3xdr.c b/fs/nfs/nfs3xdr.c
index d6506f8..f385759 100644
--- a/fs/nfs/nfs3xdr.c
+++ b/fs/nfs/nfs3xdr.c
@@ -444,10 +444,10 @@ nfs3_xdr_mknodargs(struct rpc_rqst *req, __be32 *p, struct nfs3_mknodargs *args)
static int
nfs3_xdr_renameargs(struct rpc_rqst *req, __be32 *p, struct nfs_renameargs *args)
{
- p = xdr_encode_fhandle(p, args->fromfh);
- p = xdr_encode_array(p, args->fromname, args->fromlen);
- p = xdr_encode_fhandle(p, args->tofh);
- p = xdr_encode_array(p, args->toname, args->tolen);
+ p = xdr_encode_fhandle(p, args->old_dir);
+ p = xdr_encode_array(p, args->old_name->name, args->old_name->len);
+ p = xdr_encode_fhandle(p, args->new_dir);
+ p = xdr_encode_array(p, args->new_name->name, args->new_name->len);
req->rq_slen = xdr_adjust_iovec(req->rq_svec, p);
return 0;
}
diff --git a/fs/nfs/proc.c b/fs/nfs/proc.c
index 611bec2..5c70b58 100644
--- a/fs/nfs/proc.c
+++ b/fs/nfs/proc.c
@@ -370,12 +370,10 @@ nfs_proc_rename(struct inode *old_dir, struct qstr *old_name,
struct inode *new_dir, struct qstr *new_name)
{
struct nfs_renameargs arg = {
- .fromfh = NFS_FH(old_dir),
- .fromname = old_name->name,
- .fromlen = old_name->len,
- .tofh = NFS_FH(new_dir),
- .toname = new_name->name,
- .tolen = new_name->len
+ .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],
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index c606f60..5436680 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -403,12 +403,10 @@ struct nfs_removeres {
* Common arguments to the rename call
*/
struct nfs_renameargs {
- struct nfs_fh * fromfh;
- const char * fromname;
- unsigned int fromlen;
- struct nfs_fh * tofh;
- const char * toname;
- unsigned int tolen;
+ const struct nfs_fh * old_dir;
+ const struct nfs_fh * new_dir;
+ const struct qstr * old_name;
+ const struct qstr * new_name;
};
/*
--
1.7.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 3/8] nfs: eliminate nfs4_rename_arg
2010-09-15 13:23 [PATCH 0/8] nfs: ensure that sillyrenames run to completion (try #2) Jeff Layton
2010-09-15 13:23 ` [PATCH 1/8] nfs: eliminate nfs3_renameargs Jeff Layton
2010-09-15 13:23 ` [PATCH 2/8] nfs: convert nfs_renameargs to use qstr structs Jeff Layton
@ 2010-09-15 13:23 ` Jeff Layton
2010-09-15 15:26 ` Chuck Lever
2010-09-15 13:23 ` [PATCH 4/8] nfs: standardize the rename response container Jeff Layton
` (4 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Jeff Layton @ 2010-09-15 13:23 UTC (permalink / raw)
To: trond.myklebust; +Cc: linux-nfs
Add the missing fields to nfs_renameargs and drop nfs4_rename_arg
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
fs/nfs/nfs4proc.c | 2 +-
fs/nfs/nfs4xdr.c | 2 +-
include/linux/nfs_xdr.h | 19 ++++++-------------
3 files changed, 8 insertions(+), 15 deletions(-)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 089da5b..eb36784 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -2675,7 +2675,7 @@ 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 nfs4_rename_arg arg = {
+ struct nfs_renameargs arg = {
.old_dir = NFS_FH(old_dir),
.new_dir = NFS_FH(new_dir),
.old_name = old_name,
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 08ef912..7a098ae 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -1823,7 +1823,7 @@ static int nfs4_xdr_enc_remove(struct rpc_rqst *req, __be32 *p, const struct nfs
/*
* Encode RENAME request
*/
-static int nfs4_xdr_enc_rename(struct rpc_rqst *req, __be32 *p, const struct nfs4_rename_arg *args)
+static int nfs4_xdr_enc_rename(struct rpc_rqst *req, __be32 *p, const struct nfs_renameargs *args)
{
struct xdr_stream xdr;
struct compound_hdr hdr = {
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index 5436680..60fa509 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -403,10 +403,12 @@ struct nfs_removeres {
* Common arguments to the rename call
*/
struct nfs_renameargs {
- const struct nfs_fh * old_dir;
- const struct nfs_fh * new_dir;
- const struct qstr * old_name;
- const struct qstr * new_name;
+ const struct nfs_fh * old_dir;
+ const struct nfs_fh * new_dir;
+ const struct qstr * old_name;
+ const struct qstr * new_name;
+ const u32 * bitmask;
+ struct nfs4_sequence_args seq_args;
};
/*
@@ -793,15 +795,6 @@ struct nfs4_readlink_res {
struct nfs4_sequence_res seq_res;
};
-struct nfs4_rename_arg {
- const struct nfs_fh * old_dir;
- const struct nfs_fh * new_dir;
- const struct qstr * old_name;
- const struct qstr * new_name;
- const u32 * bitmask;
- struct nfs4_sequence_args seq_args;
-};
-
struct nfs4_rename_res {
const struct nfs_server * server;
struct nfs4_change_info old_cinfo;
--
1.7.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 4/8] nfs: standardize the rename response container
2010-09-15 13:23 [PATCH 0/8] nfs: ensure that sillyrenames run to completion (try #2) Jeff Layton
` (2 preceding siblings ...)
2010-09-15 13:23 ` [PATCH 3/8] nfs: eliminate nfs4_rename_arg Jeff Layton
@ 2010-09-15 13:23 ` Jeff Layton
2010-09-15 15:29 ` Chuck Lever
2010-09-15 13:23 ` [PATCH 5/8] nfs: move nfs_sillyrename to unlink.c Jeff Layton
` (3 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Jeff Layton @ 2010-09-15 13:23 UTC (permalink / raw)
To: trond.myklebust; +Cc: linux-nfs
Right now, v3 and v4 have their own variants and v2 doesn't have one at
all. Create a standard struct that will work for v3 and v4.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
fs/nfs/nfs3proc.c | 16 ++++++++--------
fs/nfs/nfs3xdr.c | 6 +++---
fs/nfs/nfs4proc.c | 2 +-
fs/nfs/nfs4xdr.c | 2 +-
include/linux/nfs_xdr.h | 23 +++++++++--------------
5 files changed, 22 insertions(+), 27 deletions(-)
diff --git a/fs/nfs/nfs3proc.c b/fs/nfs/nfs3proc.c
index d8557b3..c5cccf1 100644
--- a/fs/nfs/nfs3proc.c
+++ b/fs/nfs/nfs3proc.c
@@ -448,7 +448,7 @@ nfs3_proc_rename(struct inode *old_dir, struct qstr *old_name,
.new_dir = NFS_FH(new_dir),
.new_name = new_name,
};
- struct nfs3_renameres res;
+ struct nfs_renameres res;
struct rpc_message msg = {
.rpc_proc = &nfs3_procedures[NFS3PROC_RENAME],
.rpc_argp = &arg,
@@ -458,17 +458,17 @@ nfs3_proc_rename(struct inode *old_dir, struct qstr *old_name,
dprintk("NFS call rename %s -> %s\n", old_name->name, new_name->name);
- res.fromattr = nfs_alloc_fattr();
- res.toattr = nfs_alloc_fattr();
- if (res.fromattr == NULL || res.toattr == NULL)
+ 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.fromattr);
- nfs_post_op_update_inode(new_dir, res.toattr);
+ 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.toattr);
- nfs_free_fattr(res.fromattr);
+ nfs_free_fattr(res.old_fattr);
+ nfs_free_fattr(res.new_fattr);
dprintk("NFS reply rename: %d\n", status);
return status;
}
diff --git a/fs/nfs/nfs3xdr.c b/fs/nfs/nfs3xdr.c
index f385759..89c40f8 100644
--- a/fs/nfs/nfs3xdr.c
+++ b/fs/nfs/nfs3xdr.c
@@ -970,14 +970,14 @@ nfs3_xdr_createres(struct rpc_rqst *req, __be32 *p, struct nfs3_diropres *res)
* Decode RENAME reply
*/
static int
-nfs3_xdr_renameres(struct rpc_rqst *req, __be32 *p, struct nfs3_renameres *res)
+nfs3_xdr_renameres(struct rpc_rqst *req, __be32 *p, struct nfs_renameres *res)
{
int status;
if ((status = ntohl(*p++)) != 0)
status = nfs_stat_to_errno(status);
- p = xdr_decode_wcc_data(p, res->fromattr);
- p = xdr_decode_wcc_data(p, res->toattr);
+ p = xdr_decode_wcc_data(p, res->old_fattr);
+ p = xdr_decode_wcc_data(p, res->new_fattr);
return status;
}
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index eb36784..120a8a6 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -2682,7 +2682,7 @@ static int _nfs4_proc_rename(struct inode *old_dir, struct qstr *old_name,
.new_name = new_name,
.bitmask = server->attr_bitmask,
};
- struct nfs4_rename_res res = {
+ struct nfs_renameres res = {
.server = server,
};
struct rpc_message msg = {
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 7a098ae..b0bd7ef 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -4873,7 +4873,7 @@ out:
/*
* Decode RENAME response
*/
-static int nfs4_xdr_dec_rename(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_rename_res *res)
+static int nfs4_xdr_dec_rename(struct rpc_rqst *rqstp, __be32 *p, struct nfs_renameres *res)
{
struct xdr_stream xdr;
struct compound_hdr hdr;
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index 60fa509..6265fa8 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -411,6 +411,15 @@ struct nfs_renameargs {
struct nfs4_sequence_args seq_args;
};
+struct nfs_renameres {
+ const struct nfs_server * server;
+ struct nfs4_change_info old_cinfo;
+ struct nfs_fattr * old_fattr;
+ struct nfs4_change_info new_cinfo;
+ struct nfs_fattr * new_fattr;
+ struct nfs4_sequence_res seq_res;
+};
+
/*
* Argument struct for decode_entry function
*/
@@ -623,11 +632,6 @@ struct nfs3_readlinkargs {
struct page ** pages;
};
-struct nfs3_renameres {
- struct nfs_fattr * fromattr;
- struct nfs_fattr * toattr;
-};
-
struct nfs3_linkres {
struct nfs_fattr * dir_attr;
struct nfs_fattr * fattr;
@@ -795,15 +799,6 @@ struct nfs4_readlink_res {
struct nfs4_sequence_res seq_res;
};
-struct nfs4_rename_res {
- const struct nfs_server * server;
- struct nfs4_change_info old_cinfo;
- struct nfs_fattr * old_fattr;
- struct nfs4_change_info new_cinfo;
- struct nfs_fattr * new_fattr;
- struct nfs4_sequence_res seq_res;
-};
-
#define NFS4_SETCLIENTID_NAMELEN (127)
struct nfs4_setclientid {
const nfs4_verifier * sc_verifier;
--
1.7.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 5/8] nfs: move nfs_sillyrename to unlink.c
2010-09-15 13:23 [PATCH 0/8] nfs: ensure that sillyrenames run to completion (try #2) Jeff Layton
` (3 preceding siblings ...)
2010-09-15 13:23 ` [PATCH 4/8] nfs: standardize the rename response container Jeff Layton
@ 2010-09-15 13:23 ` Jeff Layton
2010-09-15 13:23 ` [PATCH 6/8] nfs: add a rename_setup nfs_rpc_op for async renames Jeff Layton
` (2 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Jeff Layton @ 2010-09-15 13:23 UTC (permalink / raw)
To: trond.myklebust; +Cc: linux-nfs
...since that's where most of the sillyrenaming code lives. Also, make
nfs_async_unlink static as nfs_sillyrename is the only caller.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
fs/nfs/dir.c | 70 ---------------------------------------
fs/nfs/unlink.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++-
include/linux/nfs_fs.h | 2 +-
3 files changed, 85 insertions(+), 72 deletions(-)
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index e257172..d15ebc9 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1351,76 +1351,6 @@ static int nfs_rmdir(struct inode *dir, struct dentry *dentry)
return error;
}
-static int nfs_sillyrename(struct inode *dir, struct dentry *dentry)
-{
- static unsigned int sillycounter;
- const int fileidsize = sizeof(NFS_FILEID(dentry->d_inode))*2;
- const int countersize = sizeof(sillycounter)*2;
- const int slen = sizeof(".nfs")+fileidsize+countersize-1;
- char silly[slen+1];
- struct qstr qsilly;
- struct dentry *sdentry;
- int error = -EIO;
-
- dfprintk(VFS, "NFS: silly-rename(%s/%s, ct=%d)\n",
- dentry->d_parent->d_name.name, dentry->d_name.name,
- atomic_read(&dentry->d_count));
- nfs_inc_stats(dir, NFSIOS_SILLYRENAME);
-
- /*
- * We don't allow a dentry to be silly-renamed twice.
- */
- error = -EBUSY;
- if (dentry->d_flags & DCACHE_NFSFS_RENAMED)
- goto out;
-
- sprintf(silly, ".nfs%*.*Lx",
- fileidsize, fileidsize,
- (unsigned long long)NFS_FILEID(dentry->d_inode));
-
- /* Return delegation in anticipation of the rename */
- nfs_inode_return_delegation(dentry->d_inode);
-
- sdentry = NULL;
- do {
- char *suffix = silly + slen - countersize;
-
- dput(sdentry);
- sillycounter++;
- sprintf(suffix, "%*.*x", countersize, countersize, sillycounter);
-
- dfprintk(VFS, "NFS: trying to rename %s to %s\n",
- dentry->d_name.name, silly);
-
- sdentry = lookup_one_len(silly, dentry->d_parent, slen);
- /*
- * N.B. Better to return EBUSY here ... it could be
- * dangerous to delete the file while it's in use.
- */
- if (IS_ERR(sdentry))
- goto out;
- } while(sdentry->d_inode != NULL); /* need negative lookup */
-
- qsilly.name = silly;
- qsilly.len = strlen(silly);
- if (dentry->d_inode) {
- error = NFS_PROTO(dir)->rename(dir, &dentry->d_name,
- dir, &qsilly);
- nfs_mark_for_revalidate(dentry->d_inode);
- } else
- error = NFS_PROTO(dir)->rename(dir, &dentry->d_name,
- dir, &qsilly);
- if (!error) {
- nfs_set_verifier(dentry, nfs_save_change_attribute(dir));
- d_move(dentry, sdentry);
- error = nfs_async_unlink(dir, dentry);
- /* If we return 0 we don't unlink */
- }
- dput(sdentry);
-out:
- return error;
-}
-
/*
* Remove a file after making sure there are no pending writes,
* and after checking that the file has only one user.
diff --git a/fs/nfs/unlink.c b/fs/nfs/unlink.c
index 2f84ada..42cadd1 100644
--- a/fs/nfs/unlink.c
+++ b/fs/nfs/unlink.c
@@ -13,9 +13,12 @@
#include <linux/nfs_fs.h>
#include <linux/sched.h>
#include <linux/wait.h>
+#include <linux/namei.h>
#include "internal.h"
#include "nfs4_fs.h"
+#include "iostat.h"
+#include "delegation.h"
struct nfs_unlinkdata {
struct hlist_node list;
@@ -244,7 +247,7 @@ void nfs_unblock_sillyrename(struct dentry *dentry)
* @dir: parent directory of dentry
* @dentry: dentry to unlink
*/
-int
+static int
nfs_async_unlink(struct inode *dir, struct dentry *dentry)
{
struct nfs_unlinkdata *data;
@@ -303,3 +306,83 @@ nfs_complete_unlink(struct dentry *dentry, struct inode *inode)
if (data != NULL && (NFS_STALE(inode) || !nfs_call_unlink(dentry, data)))
nfs_free_unlinkdata(data);
}
+
+/**
+ * nfs_sillyrename - Perform a silly-rename of a dentry
+ * @dir: inode of directory that contains dentry
+ * @dentry: dentry to be sillyrenamed
+ *
+ * NFSv2/3 is stateless, so the server doesn't know when the client is
+ * holding a file open. To prevent problems when a file is unlinked while
+ * it's still open, we perform a "silly-rename" -- that is, we rename the
+ * file to a hidden file in the same directory, and only actually perform
+ * the unlink once the last reference to it is put.
+ *
+ * The final cleanup is done via nfs_dentry_iput.
+ */
+int
+nfs_sillyrename(struct inode *dir, struct dentry *dentry)
+{
+ static unsigned int sillycounter;
+ const int fileidsize = sizeof(NFS_FILEID(dentry->d_inode))*2;
+ const int countersize = sizeof(sillycounter)*2;
+ const int slen = sizeof(".nfs")+fileidsize+countersize-1;
+ char silly[slen+1];
+ struct qstr qsilly;
+ struct dentry *sdentry;
+ int error = -EIO;
+
+ dfprintk(VFS, "NFS: silly-rename(%s/%s, ct=%d)\n",
+ dentry->d_parent->d_name.name, dentry->d_name.name,
+ atomic_read(&dentry->d_count));
+ nfs_inc_stats(dir, NFSIOS_SILLYRENAME);
+
+ /*
+ * We don't allow a dentry to be silly-renamed twice.
+ */
+ error = -EBUSY;
+ if (dentry->d_flags & DCACHE_NFSFS_RENAMED)
+ goto out;
+
+ sprintf(silly, ".nfs%*.*Lx",
+ fileidsize, fileidsize,
+ (unsigned long long)NFS_FILEID(dentry->d_inode));
+
+ /* Return delegation in anticipation of the rename */
+ nfs_inode_return_delegation(dentry->d_inode);
+
+ sdentry = NULL;
+ do {
+ char *suffix = silly + slen - countersize;
+
+ dput(sdentry);
+ sillycounter++;
+ sprintf(suffix, "%*.*x", countersize, countersize, sillycounter);
+
+ dfprintk(VFS, "NFS: trying to rename %s to %s\n",
+ dentry->d_name.name, silly);
+
+ sdentry = lookup_one_len(silly, dentry->d_parent, slen);
+ /*
+ * N.B. Better to return EBUSY here ... it could be
+ * dangerous to delete the file while it's in use.
+ */
+ if (IS_ERR(sdentry))
+ goto out;
+ } while(sdentry->d_inode != NULL); /* need negative lookup */
+
+ qsilly.name = silly;
+ qsilly.len = strlen(silly);
+ error = NFS_PROTO(dir)->rename(dir, &dentry->d_name, dir, &qsilly);
+ if (dentry->d_inode)
+ nfs_mark_for_revalidate(dentry->d_inode);
+ if (!error) {
+ nfs_set_verifier(dentry, nfs_save_change_attribute(dir));
+ d_move(dentry, sdentry);
+ error = nfs_async_unlink(dir, dentry);
+ /* If we return 0 we don't unlink */
+ }
+ dput(sdentry);
+out:
+ return error;
+}
diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index 508f8cf..6cdf0d6 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -479,10 +479,10 @@ extern void nfs_release_automount_timer(void);
/*
* linux/fs/nfs/unlink.c
*/
-extern int nfs_async_unlink(struct inode *dir, struct dentry *dentry);
extern void nfs_complete_unlink(struct dentry *dentry, struct inode *);
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.7.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 6/8] nfs: add a rename_setup nfs_rpc_op for async renames
2010-09-15 13:23 [PATCH 0/8] nfs: ensure that sillyrenames run to completion (try #2) Jeff Layton
` (4 preceding siblings ...)
2010-09-15 13:23 ` [PATCH 5/8] nfs: move nfs_sillyrename to unlink.c Jeff Layton
@ 2010-09-15 13:23 ` Jeff Layton
2010-09-15 15:39 ` Chuck Lever
2010-09-15 13:24 ` [PATCH 7/8] nfs: add rename_done nfs_rpc_op Jeff Layton
2010-09-15 13:24 ` [PATCH 8/8] nfs: make sillyrename an async operation Jeff Layton
7 siblings, 1 reply; 19+ messages in thread
From: Jeff Layton @ 2010-09-15 13:23 UTC (permalink / raw)
To: trond.myklebust; +Cc: linux-nfs
Add a call that can set up the arguments and the response container for
an asynchronous rename call.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
fs/nfs/nfs3proc.c | 18 ++++++++++++++++++
fs/nfs/nfs4proc.c | 21 +++++++++++++++++++++
fs/nfs/proc.c | 12 ++++++++++++
include/linux/nfs_xdr.h | 1 +
4 files changed, 52 insertions(+), 0 deletions(-)
diff --git a/fs/nfs/nfs3proc.c b/fs/nfs/nfs3proc.c
index c5cccf1..0654177 100644
--- a/fs/nfs/nfs3proc.c
+++ b/fs/nfs/nfs3proc.c
@@ -439,6 +439,23 @@ nfs3_proc_unlink_done(struct rpc_task *task, struct inode *dir)
}
static int
+nfs3_proc_rename_setup(struct rpc_message *msg, struct inode *dir)
+{
+ struct nfs_renameres *res = msg->rpc_resp;
+
+ msg->rpc_proc = &nfs3_procedures[NFS3PROC_RENAME];
+
+ res->old_fattr = nfs_alloc_fattr();
+ res->new_fattr = nfs_alloc_fattr();
+ if (res->old_fattr != NULL && res->new_fattr != NULL)
+ return 0;
+
+ nfs_free_fattr(res->old_fattr);
+ nfs_free_fattr(res->new_fattr);
+ return -ENOMEM;
+}
+
+static int
nfs3_proc_rename(struct inode *old_dir, struct qstr *old_name,
struct inode *new_dir, struct qstr *new_name)
{
@@ -842,6 +859,7 @@ const struct nfs_rpc_ops nfs_v3_clientops = {
.unlink_setup = nfs3_proc_unlink_setup,
.unlink_done = nfs3_proc_unlink_done,
.rename = nfs3_proc_rename,
+ .rename_setup = nfs3_proc_rename_setup,
.link = nfs3_proc_link,
.symlink = nfs3_proc_symlink,
.mkdir = nfs3_proc_mkdir,
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 120a8a6..011dc90 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -2671,6 +2671,26 @@ static int nfs4_proc_unlink_done(struct rpc_task *task, struct inode *dir)
return 1;
}
+static int nfs4_proc_rename_setup(struct rpc_message *msg, struct inode *dir)
+{
+ struct nfs_server *server = NFS_SERVER(dir);
+ struct nfs_renameargs *arg = msg->rpc_argp;
+ struct nfs_renameres *res = msg->rpc_resp;
+
+ msg->rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_RENAME];
+ arg->bitmask = server->attr_bitmask;
+ res->server = server;
+
+ res->old_fattr = nfs_alloc_fattr();
+ res->new_fattr = nfs_alloc_fattr();
+ if (res->old_fattr != NULL && res->new_fattr != NULL)
+ return 0;
+
+ nfs_free_fattr(res->old_fattr);
+ nfs_free_fattr(res->new_fattr);
+ return -ENOMEM;
+}
+
static int _nfs4_proc_rename(struct inode *old_dir, struct qstr *old_name,
struct inode *new_dir, struct qstr *new_name)
{
@@ -5443,6 +5463,7 @@ const struct nfs_rpc_ops nfs_v4_clientops = {
.unlink_setup = nfs4_proc_unlink_setup,
.unlink_done = nfs4_proc_unlink_done,
.rename = nfs4_proc_rename,
+ .rename_setup = nfs4_proc_rename_setup,
.link = nfs4_proc_link,
.symlink = nfs4_proc_symlink,
.mkdir = nfs4_proc_mkdir,
diff --git a/fs/nfs/proc.c b/fs/nfs/proc.c
index 5c70b58..1b02dee 100644
--- a/fs/nfs/proc.c
+++ b/fs/nfs/proc.c
@@ -366,6 +366,17 @@ static int nfs_proc_unlink_done(struct rpc_task *task, struct inode *dir)
}
static int
+nfs_proc_rename_setup(struct rpc_message *msg, struct inode *dir)
+{
+ struct nfs_renameres *res = msg->rpc_resp;
+
+ msg->rpc_proc = &nfs_procedures[NFSPROC_RENAME];
+ res->old_fattr = NULL;
+ res->new_fattr = NULL;
+ return 0;
+}
+
+static int
nfs_proc_rename(struct inode *old_dir, struct qstr *old_name,
struct inode *new_dir, struct qstr *new_name)
{
@@ -703,6 +714,7 @@ const struct nfs_rpc_ops nfs_v2_clientops = {
.unlink_setup = nfs_proc_unlink_setup,
.unlink_done = nfs_proc_unlink_done,
.rename = nfs_proc_rename,
+ .rename_setup = nfs_proc_rename_setup,
.link = nfs_proc_link,
.symlink = nfs_proc_symlink,
.mkdir = nfs_proc_mkdir,
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index 6265fa8..d470751 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -1018,6 +1018,7 @@ struct nfs_rpc_ops {
int (*unlink_done) (struct rpc_task *, struct inode *);
int (*rename) (struct inode *, struct qstr *,
struct inode *, struct qstr *);
+ int (*rename_setup) (struct rpc_message *msg, struct inode *dir);
int (*link) (struct inode *, struct inode *, struct qstr *);
int (*symlink) (struct inode *, struct dentry *, struct page *,
unsigned int, struct iattr *);
--
1.7.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 7/8] nfs: add rename_done nfs_rpc_op
2010-09-15 13:23 [PATCH 0/8] nfs: ensure that sillyrenames run to completion (try #2) Jeff Layton
` (5 preceding siblings ...)
2010-09-15 13:23 ` [PATCH 6/8] nfs: add a rename_setup nfs_rpc_op for async renames Jeff Layton
@ 2010-09-15 13:24 ` Jeff Layton
2010-09-15 13:24 ` [PATCH 8/8] nfs: make sillyrename an async operation Jeff Layton
7 siblings, 0 replies; 19+ messages in thread
From: Jeff Layton @ 2010-09-15 13:24 UTC (permalink / raw)
To: trond.myklebust; +Cc: linux-nfs
...for processing the rename reply.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
fs/nfs/nfs3proc.c | 16 ++++++++++++++++
fs/nfs/nfs4proc.c | 18 ++++++++++++++++++
fs/nfs/proc.c | 12 ++++++++++++
include/linux/nfs_xdr.h | 1 +
4 files changed, 47 insertions(+), 0 deletions(-)
diff --git a/fs/nfs/nfs3proc.c b/fs/nfs/nfs3proc.c
index 0654177..b072519 100644
--- a/fs/nfs/nfs3proc.c
+++ b/fs/nfs/nfs3proc.c
@@ -456,6 +456,21 @@ nfs3_proc_rename_setup(struct rpc_message *msg, struct inode *dir)
}
static int
+nfs3_proc_rename_done(struct rpc_task *task, struct inode *old_dir,
+ struct inode *new_dir)
+{
+ struct nfs_renameres *res;
+
+ if (nfs3_async_handle_jukebox(task, old_dir))
+ return 0;
+ res = task->tk_msg.rpc_resp;
+
+ nfs_post_op_update_inode(old_dir, res->old_fattr);
+ nfs_post_op_update_inode(new_dir, res->new_fattr);
+ return 1;
+}
+
+static int
nfs3_proc_rename(struct inode *old_dir, struct qstr *old_name,
struct inode *new_dir, struct qstr *new_name)
{
@@ -860,6 +875,7 @@ const struct nfs_rpc_ops nfs_v3_clientops = {
.unlink_done = nfs3_proc_unlink_done,
.rename = nfs3_proc_rename,
.rename_setup = nfs3_proc_rename_setup,
+ .rename_done = nfs3_proc_rename_done,
.link = nfs3_proc_link,
.symlink = nfs3_proc_symlink,
.mkdir = nfs3_proc_mkdir,
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 011dc90..9c4160c 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -2691,6 +2691,23 @@ static int nfs4_proc_rename_setup(struct rpc_message *msg, struct inode *dir)
return -ENOMEM;
}
+static int nfs4_proc_rename_done(struct rpc_task *task, struct inode *old_dir,
+ struct inode *new_dir)
+{
+ struct nfs_renameres *res = task->tk_msg.rpc_resp;
+
+ if (!nfs4_sequence_done(task, &res->seq_res))
+ return 0;
+ if (nfs4_async_handle_error(task, res->server, NULL) == -EAGAIN)
+ return 0;
+
+ update_changeattr(old_dir, &res->old_cinfo);
+ nfs_post_op_update_inode(old_dir, res->old_fattr);
+ update_changeattr(new_dir, &res->new_cinfo);
+ nfs_post_op_update_inode(new_dir, res->new_fattr);
+ return 1;
+}
+
static int _nfs4_proc_rename(struct inode *old_dir, struct qstr *old_name,
struct inode *new_dir, struct qstr *new_name)
{
@@ -5464,6 +5481,7 @@ const struct nfs_rpc_ops nfs_v4_clientops = {
.unlink_done = nfs4_proc_unlink_done,
.rename = nfs4_proc_rename,
.rename_setup = nfs4_proc_rename_setup,
+ .rename_done = nfs4_proc_rename_done,
.link = nfs4_proc_link,
.symlink = nfs4_proc_symlink,
.mkdir = nfs4_proc_mkdir,
diff --git a/fs/nfs/proc.c b/fs/nfs/proc.c
index 1b02dee..e2d5cae 100644
--- a/fs/nfs/proc.c
+++ b/fs/nfs/proc.c
@@ -377,6 +377,17 @@ nfs_proc_rename_setup(struct rpc_message *msg, struct inode *dir)
}
static int
+nfs_proc_rename_done(struct rpc_task *task, struct inode *old_dir,
+ struct inode *new_dir)
+{
+ if (nfs_async_handle_expired_key(task))
+ return 0;
+ nfs_mark_for_revalidate(old_dir);
+ nfs_mark_for_revalidate(new_dir);
+ return 1;
+}
+
+static int
nfs_proc_rename(struct inode *old_dir, struct qstr *old_name,
struct inode *new_dir, struct qstr *new_name)
{
@@ -715,6 +726,7 @@ const struct nfs_rpc_ops nfs_v2_clientops = {
.unlink_done = nfs_proc_unlink_done,
.rename = nfs_proc_rename,
.rename_setup = nfs_proc_rename_setup,
+ .rename_done = nfs_proc_rename_done,
.link = nfs_proc_link,
.symlink = nfs_proc_symlink,
.mkdir = nfs_proc_mkdir,
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index d470751..97f7eaa 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -1019,6 +1019,7 @@ struct nfs_rpc_ops {
int (*rename) (struct inode *, struct qstr *,
struct inode *, struct qstr *);
int (*rename_setup) (struct rpc_message *msg, struct inode *dir);
+ int (*rename_done) (struct rpc_task *task, struct inode *old_dir, struct inode *new_dir);
int (*link) (struct inode *, struct inode *, struct qstr *);
int (*symlink) (struct inode *, struct dentry *, struct page *,
unsigned int, struct iattr *);
--
1.7.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 8/8] nfs: make sillyrename an async operation
2010-09-15 13:23 [PATCH 0/8] nfs: ensure that sillyrenames run to completion (try #2) Jeff Layton
` (6 preceding siblings ...)
2010-09-15 13:24 ` [PATCH 7/8] nfs: add rename_done nfs_rpc_op Jeff Layton
@ 2010-09-15 13:24 ` Jeff Layton
2010-09-15 13:37 ` Jeff Layton
7 siblings, 1 reply; 19+ messages in thread
From: Jeff Layton @ 2010-09-15 13:24 UTC (permalink / raw)
To: trond.myklebust; +Cc: linux-nfs
A synchronous rename can be interrupted by a SIGKILL. If that happens
during a sillyrename operation, it's possible for the rename call to
be sent to the server, but the task exits before processing the
reply. If this happens, the sillyrenamed file won't get cleaned up
during nfs_dentry_iput and the server is left with a dangling .nfs* file
hanging around.
Fix this problem by turning sillyrename into an asynchronous operation
and have the task doing the sillyrename just wait on the reply. If the
task is killed before the sillyrename completes, it'll still proceed
to completion.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
fs/nfs/unlink.c | 198 ++++++++++++++++++++++++++++++++++++++++++++++++++----
1 files changed, 183 insertions(+), 15 deletions(-)
diff --git a/fs/nfs/unlink.c b/fs/nfs/unlink.c
index 42cadd1..87a65ac 100644
--- a/fs/nfs/unlink.c
+++ b/fs/nfs/unlink.c
@@ -307,6 +307,164 @@ nfs_complete_unlink(struct dentry *dentry, struct inode *inode)
nfs_free_unlinkdata(data);
}
+/* Cancel a queued async unlink. Called when a sillyrename run fails. */
+static void
+nfs_cancel_async_unlink(struct dentry *dentry)
+{
+ spin_lock(&dentry->d_lock);
+ if (dentry->d_flags & DCACHE_NFSFS_RENAMED) {
+ dentry->d_flags &= ~DCACHE_NFSFS_RENAMED;
+ nfs_free_unlinkdata(dentry->d_fsdata);
+ }
+ spin_unlock(&dentry->d_lock);
+}
+
+struct nfs_renamedata {
+ struct nfs_renameargs args;
+ struct nfs_renameres res;
+ struct rpc_cred * cred;
+ struct inode * old_dir;
+ struct inode * new_dir;
+ struct dentry * old_dentry;
+ struct dentry * new_dentry;
+};
+
+/**
+ * nfs_async_rename_done - Sillyrename post-processing
+ * @task: rpc_task of the sillyrename
+ * @calldata: nfs_renamedata for the sillyrename
+ *
+ * Do the directory attribute updates and the d_move
+ */
+static void nfs_async_rename_done(struct rpc_task *task, void *calldata)
+{
+ struct nfs_renamedata *data = calldata;
+ struct inode *old_dir = data->old_dir;
+ struct inode *new_dir = data->new_dir;
+
+ if (!NFS_PROTO(old_dir)->rename_done(task, old_dir, new_dir)) {
+ nfs_restart_rpc(task, NFS_SERVER(old_dir)->nfs_client);
+ return;
+ }
+
+ if (task->tk_status != 0)
+ return;
+
+ nfs_set_verifier(data->old_dentry, nfs_save_change_attribute(old_dir));
+ d_move(data->old_dentry, data->new_dentry);
+}
+
+/**
+ * nfs_async_rename_release - Release the sillyrename data.
+ * @calldata: the struct nfs_renamedata to be released
+ */
+static void nfs_async_rename_release(void *calldata)
+{
+ struct nfs_renamedata *data = calldata;
+ struct super_block *sb = data->old_dir->i_sb;
+
+ /* FIXME: is this OK here? Note that call_done will be done first... */
+ if (data->old_dentry->d_inode)
+ nfs_mark_for_revalidate(data->old_dentry->d_inode);
+
+ nfs_free_fattr(data->res.old_fattr);
+ nfs_free_fattr(data->res.new_fattr);
+ dput(data->old_dentry);
+ dput(data->new_dentry);
+ iput(data->old_dir);
+ iput(data->new_dir);
+ nfs_sb_deactive(sb);
+ put_rpccred(data->cred);
+ kfree(data);
+}
+
+#if defined(CONFIG_NFS_V4_1)
+static void nfs_rename_prepare(struct rpc_task *task, void *calldata)
+{
+ struct nfs_renamedata *data = calldata;
+ struct nfs_server *server = NFS_SERVER(data->old_dir);
+
+ if (nfs4_setup_sequence(server, &data->args.seq_args,
+ &data->res.seq_res, 1, task))
+ return;
+ rpc_call_start(task);
+}
+#endif /* CONFIG_NFS_V4_1 */
+
+static const struct rpc_call_ops nfs_rename_ops = {
+ .rpc_call_done = nfs_async_rename_done,
+ .rpc_release = nfs_async_rename_release,
+#if defined(CONFIG_NFS_V4_1)
+ .rpc_call_prepare = nfs_rename_prepare,
+#endif /* CONFIG_NFS_V4_1 */
+};
+
+/**
+ * nfs_async_rename - perform an asynchronous rename operation
+ * @old_dir: directory that currently holds the dentry to be renamed
+ * @new_dir: target directory for the rename
+ * @old_dentry: original dentry to be renamed
+ * @new_dentry: dentry to which the old_dentry should be renamed
+ */
+static struct rpc_task *
+nfs_async_rename(struct inode *old_dir, struct inode *new_dir,
+ struct dentry *old_dentry, struct dentry *new_dentry)
+{
+ int error;
+ struct nfs_renamedata *data;
+ struct rpc_message msg = { };
+ struct rpc_task_setup task_setup_data = {
+ .rpc_message = &msg,
+ .callback_ops = &nfs_rename_ops,
+ .workqueue = nfsiod_workqueue,
+ .rpc_client = NFS_CLIENT(old_dir),
+ .flags = RPC_TASK_ASYNC,
+ };
+ struct rpc_task *task;
+
+ data = kzalloc(sizeof(*data), GFP_KERNEL);
+ if (data == NULL)
+ return ERR_PTR(-ENOMEM);
+ task_setup_data.callback_data = data,
+
+ data->cred = rpc_lookup_cred();
+ if (IS_ERR(data->cred)) {
+ kfree(data);
+ return (struct rpc_task *)data->cred;
+ }
+ msg.rpc_argp = &data->args,
+ msg.rpc_resp = &data->res,
+ msg.rpc_cred = data->cred;
+
+ /* set up nfs_renamedata */
+ data->old_dir = old_dir;
+ atomic_inc(&old_dir->i_count);
+ data->new_dir = new_dir;
+ atomic_inc(&new_dir->i_count);
+ data->old_dentry = dget(old_dentry);
+ data->new_dentry = dget(new_dentry);
+
+ /* set up nfs_renameargs */
+ data->args.old_dir = NFS_FH(old_dir);
+ data->args.old_name = &old_dentry->d_name;
+ data->args.new_dir = NFS_FH(new_dir);
+ data->args.new_name = &new_dentry->d_name;
+
+ nfs_sb_active(old_dir->i_sb);
+
+ error = NFS_PROTO(data->old_dir)->rename_setup(&msg, old_dir);
+ if (error) {
+ nfs_async_rename_release(data);
+ return ERR_PTR(error);
+ }
+
+ task = rpc_run_task(&task_setup_data);
+ if (IS_ERR(task))
+ nfs_async_rename_release(data);
+
+ return task;
+}
+
/**
* nfs_sillyrename - Perform a silly-rename of a dentry
* @dir: inode of directory that contains dentry
@@ -314,11 +472,11 @@ nfs_complete_unlink(struct dentry *dentry, struct inode *inode)
*
* NFSv2/3 is stateless, so the server doesn't know when the client is
* holding a file open. To prevent problems when a file is unlinked while
- * it's still open, we perform a "silly-rename" -- that is, we rename the
- * file to a hidden file in the same directory, and only actually perform
- * the unlink once the last reference to it is put.
+ * it's still open, the client performs a "silly-rename" -- that is, it
+ * renames the file to a hidden file in the same directory, and only
+ * performs the unlink once the last reference to it is put.
*
- * The final cleanup is done via nfs_dentry_iput.
+ * The final cleanup is done during dentry_iput.
*/
int
nfs_sillyrename(struct inode *dir, struct dentry *dentry)
@@ -328,8 +486,8 @@ nfs_sillyrename(struct inode *dir, struct dentry *dentry)
const int countersize = sizeof(sillycounter)*2;
const int slen = sizeof(".nfs")+fileidsize+countersize-1;
char silly[slen+1];
- struct qstr qsilly;
struct dentry *sdentry;
+ struct rpc_task *task;
int error = -EIO;
dfprintk(VFS, "NFS: silly-rename(%s/%s, ct=%d)\n",
@@ -371,17 +529,27 @@ nfs_sillyrename(struct inode *dir, struct dentry *dentry)
goto out;
} while(sdentry->d_inode != NULL); /* need negative lookup */
- qsilly.name = silly;
- qsilly.len = strlen(silly);
- error = NFS_PROTO(dir)->rename(dir, &dentry->d_name, dir, &qsilly);
- if (dentry->d_inode)
- nfs_mark_for_revalidate(dentry->d_inode);
- if (!error) {
- nfs_set_verifier(dentry, nfs_save_change_attribute(dir));
- d_move(dentry, sdentry);
- error = nfs_async_unlink(dir, dentry);
- /* If we return 0 we don't unlink */
+ /* queue unlink first. Can't do this from rpc_release as it
+ * has to allocate memory
+ */
+ error = nfs_async_unlink(dir, dentry);
+ if (error)
+ goto out_dput;
+
+ /* run the rename task, undo unlink if it fails */
+ task = nfs_async_rename(dir, dir, dentry, sdentry);
+ if (IS_ERR(task)) {
+ error = -EBUSY;
+ nfs_cancel_async_unlink(dentry);
+ goto out_dput;
}
+
+ /* wait for the RPC task to complete, unless a SIGKILL intervenes */
+ error = rpc_wait_for_completion_task(task);
+ if (error == 0)
+ error = task->tk_status;
+ rpc_put_task(task);
+out_dput:
dput(sdentry);
out:
return error;
--
1.7.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 8/8] nfs: make sillyrename an async operation
2010-09-15 13:24 ` [PATCH 8/8] nfs: make sillyrename an async operation Jeff Layton
@ 2010-09-15 13:37 ` Jeff Layton
2010-09-15 15:04 ` Jeff Layton
0 siblings, 1 reply; 19+ messages in thread
From: Jeff Layton @ 2010-09-15 13:37 UTC (permalink / raw)
To: trond.myklebust; +Cc: linux-nfs
On Wed, 15 Sep 2010 09:24:01 -0400
Jeff Layton <jlayton@redhat.com> wrote:
> +/**
> + * nfs_async_rename - perform an asynchronous rename operation
> + * @old_dir: directory that currently holds the dentry to be renamed
> + * @new_dir: target directory for the rename
> + * @old_dentry: original dentry to be renamed
> + * @new_dentry: dentry to which the old_dentry should be renamed
> + */
> +static struct rpc_task *
> +nfs_async_rename(struct inode *old_dir, struct inode *new_dir,
> + struct dentry *old_dentry, struct dentry *new_dentry)
> +{
> + int error;
> + struct nfs_renamedata *data;
> + struct rpc_message msg = { };
> + struct rpc_task_setup task_setup_data = {
> + .rpc_message = &msg,
> + .callback_ops = &nfs_rename_ops,
> + .workqueue = nfsiod_workqueue,
> + .rpc_client = NFS_CLIENT(old_dir),
> + .flags = RPC_TASK_ASYNC,
> + };
> + struct rpc_task *task;
> +
> + data = kzalloc(sizeof(*data), GFP_KERNEL);
> + if (data == NULL)
> + return ERR_PTR(-ENOMEM);
> + task_setup_data.callback_data = data,
> +
> + data->cred = rpc_lookup_cred();
> + if (IS_ERR(data->cred)) {
> + kfree(data);
> + return (struct rpc_task *)data->cred;
> + }
> + msg.rpc_argp = &data->args,
> + msg.rpc_resp = &data->res,
^^^
Eek! Just noticed the commas at the EOL in this patch. Not quite sure
how this compiled, but I'll fix, retest and resend. Review of the rest
of the patch would be appreciated though.
--
Jeff Layton <jlayton@redhat.com>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 8/8] nfs: make sillyrename an async operation
2010-09-15 13:37 ` Jeff Layton
@ 2010-09-15 15:04 ` Jeff Layton
0 siblings, 0 replies; 19+ messages in thread
From: Jeff Layton @ 2010-09-15 15:04 UTC (permalink / raw)
To: trond.myklebust; +Cc: linux-nfs
On Wed, 15 Sep 2010 09:37:21 -0400
Jeff Layton <jlayton@redhat.com> wrote:
> On Wed, 15 Sep 2010 09:24:01 -0400
> Jeff Layton <jlayton@redhat.com> wrote:
>
> > +/**
> > + * nfs_async_rename - perform an asynchronous rename operation
> > + * @old_dir: directory that currently holds the dentry to be renamed
> > + * @new_dir: target directory for the rename
> > + * @old_dentry: original dentry to be renamed
> > + * @new_dentry: dentry to which the old_dentry should be renamed
> > + */
> > +static struct rpc_task *
> > +nfs_async_rename(struct inode *old_dir, struct inode *new_dir,
> > + struct dentry *old_dentry, struct dentry *new_dentry)
> > +{
> > + int error;
> > + struct nfs_renamedata *data;
> > + struct rpc_message msg = { };
> > + struct rpc_task_setup task_setup_data = {
> > + .rpc_message = &msg,
> > + .callback_ops = &nfs_rename_ops,
> > + .workqueue = nfsiod_workqueue,
> > + .rpc_client = NFS_CLIENT(old_dir),
> > + .flags = RPC_TASK_ASYNC,
> > + };
> > + struct rpc_task *task;
> > +
> > + data = kzalloc(sizeof(*data), GFP_KERNEL);
> > + if (data == NULL)
> > + return ERR_PTR(-ENOMEM);
> > + task_setup_data.callback_data = data,
> > +
> > + data->cred = rpc_lookup_cred();
> > + if (IS_ERR(data->cred)) {
> > + kfree(data);
> > + return (struct rpc_task *)data->cred;
> > + }
> > + msg.rpc_argp = &data->args,
> > + msg.rpc_resp = &data->res,
> ^^^
> Eek! Just noticed the commas at the EOL in this patch. Not quite sure
> how this compiled, but I'll fix, retest and resend. Review of the rest
> of the patch would be appreciated though.
>
Ahh, false alarm. It's legit if a little weird:
http://en.wikipedia.org/wiki/Comma_operator
Regardless, I've done some other cleanups too so I'll resend the set
after I retest it.
Sorry for the noise...
--
Jeff Layton <jlayton@redhat.com>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/8] nfs: eliminate nfs4_rename_arg
2010-09-15 13:23 ` [PATCH 3/8] nfs: eliminate nfs4_rename_arg Jeff Layton
@ 2010-09-15 15:26 ` Chuck Lever
2010-09-15 15:34 ` Jeff Layton
2010-09-15 16:45 ` Trond Myklebust
0 siblings, 2 replies; 19+ messages in thread
From: Chuck Lever @ 2010-09-15 15:26 UTC (permalink / raw)
To: Jeff Layton; +Cc: trond.myklebust, linux-nfs
On Sep 15, 2010, at 9:23 AM, Jeff Layton wrote:
> Add the missing fields to nfs_renameargs and drop nfs4_rename_arg
>
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
> fs/nfs/nfs4proc.c | 2 +-
> fs/nfs/nfs4xdr.c | 2 +-
> include/linux/nfs_xdr.h | 19 ++++++-------------
> 3 files changed, 8 insertions(+), 15 deletions(-)
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 089da5b..eb36784 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -2675,7 +2675,7 @@ 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 nfs4_rename_arg arg = {
> + struct nfs_renameargs arg = {
> .old_dir = NFS_FH(old_dir),
> .new_dir = NFS_FH(new_dir),
> .old_name = old_name,
> diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
> index 08ef912..7a098ae 100644
> --- a/fs/nfs/nfs4xdr.c
> +++ b/fs/nfs/nfs4xdr.c
> @@ -1823,7 +1823,7 @@ static int nfs4_xdr_enc_remove(struct rpc_rqst *req, __be32 *p, const struct nfs
> /*
> * Encode RENAME request
> */
> -static int nfs4_xdr_enc_rename(struct rpc_rqst *req, __be32 *p, const struct nfs4_rename_arg *args)
> +static int nfs4_xdr_enc_rename(struct rpc_rqst *req, __be32 *p, const struct nfs_renameargs *args)
> {
> struct xdr_stream xdr;
> struct compound_hdr hdr = {
> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> index 5436680..60fa509 100644
> --- a/include/linux/nfs_xdr.h
> +++ b/include/linux/nfs_xdr.h
> @@ -403,10 +403,12 @@ struct nfs_removeres {
> * Common arguments to the rename call
> */
> struct nfs_renameargs {
> - const struct nfs_fh * old_dir;
> - const struct nfs_fh * new_dir;
> - const struct qstr * old_name;
> - const struct qstr * new_name;
> + const struct nfs_fh * old_dir;
> + const struct nfs_fh * new_dir;
> + const struct qstr * old_name;
> + const struct qstr * new_name;
> + const u32 * bitmask;
> + struct nfs4_sequence_args seq_args;
Should these new fields be gated by CONFIG_NFS_V4 ?
> };
>
> /*
> @@ -793,15 +795,6 @@ struct nfs4_readlink_res {
> struct nfs4_sequence_res seq_res;
> };
>
> -struct nfs4_rename_arg {
> - const struct nfs_fh * old_dir;
> - const struct nfs_fh * new_dir;
> - const struct qstr * old_name;
> - const struct qstr * new_name;
> - const u32 * bitmask;
> - struct nfs4_sequence_args seq_args;
> -};
> -
> struct nfs4_rename_res {
> const struct nfs_server * server;
> struct nfs4_change_info old_cinfo;
> --
> 1.7.1
>
> --
> 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
--
chuck[dot]lever[at]oracle[dot]com
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 4/8] nfs: standardize the rename response container
2010-09-15 13:23 ` [PATCH 4/8] nfs: standardize the rename response container Jeff Layton
@ 2010-09-15 15:29 ` Chuck Lever
2010-09-15 15:38 ` Jeff Layton
0 siblings, 1 reply; 19+ messages in thread
From: Chuck Lever @ 2010-09-15 15:29 UTC (permalink / raw)
To: Jeff Layton; +Cc: trond.myklebust, linux-nfs
On Sep 15, 2010, at 9:23 AM, Jeff Layton wrote:
> Right now, v3 and v4 have their own variants and v2 doesn't have one at
> all. Create a standard struct that will work for v3 and v4.
>
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
> fs/nfs/nfs3proc.c | 16 ++++++++--------
> fs/nfs/nfs3xdr.c | 6 +++---
> fs/nfs/nfs4proc.c | 2 +-
> fs/nfs/nfs4xdr.c | 2 +-
> include/linux/nfs_xdr.h | 23 +++++++++--------------
> 5 files changed, 22 insertions(+), 27 deletions(-)
>
> diff --git a/fs/nfs/nfs3proc.c b/fs/nfs/nfs3proc.c
> index d8557b3..c5cccf1 100644
> --- a/fs/nfs/nfs3proc.c
> +++ b/fs/nfs/nfs3proc.c
> @@ -448,7 +448,7 @@ nfs3_proc_rename(struct inode *old_dir, struct qstr *old_name,
> .new_dir = NFS_FH(new_dir),
> .new_name = new_name,
> };
> - struct nfs3_renameres res;
> + struct nfs_renameres res;
> struct rpc_message msg = {
> .rpc_proc = &nfs3_procedures[NFS3PROC_RENAME],
> .rpc_argp = &arg,
> @@ -458,17 +458,17 @@ nfs3_proc_rename(struct inode *old_dir, struct qstr *old_name,
>
> dprintk("NFS call rename %s -> %s\n", old_name->name, new_name->name);
>
> - res.fromattr = nfs_alloc_fattr();
> - res.toattr = nfs_alloc_fattr();
> - if (res.fromattr == NULL || res.toattr == NULL)
> + 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.fromattr);
> - nfs_post_op_update_inode(new_dir, res.toattr);
> + 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.toattr);
> - nfs_free_fattr(res.fromattr);
> + nfs_free_fattr(res.old_fattr);
> + nfs_free_fattr(res.new_fattr);
> dprintk("NFS reply rename: %d\n", status);
> return status;
> }
> diff --git a/fs/nfs/nfs3xdr.c b/fs/nfs/nfs3xdr.c
> index f385759..89c40f8 100644
> --- a/fs/nfs/nfs3xdr.c
> +++ b/fs/nfs/nfs3xdr.c
> @@ -970,14 +970,14 @@ nfs3_xdr_createres(struct rpc_rqst *req, __be32 *p, struct nfs3_diropres *res)
> * Decode RENAME reply
> */
> static int
> -nfs3_xdr_renameres(struct rpc_rqst *req, __be32 *p, struct nfs3_renameres *res)
> +nfs3_xdr_renameres(struct rpc_rqst *req, __be32 *p, struct nfs_renameres *res)
> {
> int status;
>
> if ((status = ntohl(*p++)) != 0)
> status = nfs_stat_to_errno(status);
> - p = xdr_decode_wcc_data(p, res->fromattr);
> - p = xdr_decode_wcc_data(p, res->toattr);
> + p = xdr_decode_wcc_data(p, res->old_fattr);
> + p = xdr_decode_wcc_data(p, res->new_fattr);
> return status;
> }
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index eb36784..120a8a6 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -2682,7 +2682,7 @@ static int _nfs4_proc_rename(struct inode *old_dir, struct qstr *old_name,
> .new_name = new_name,
> .bitmask = server->attr_bitmask,
> };
> - struct nfs4_rename_res res = {
> + struct nfs_renameres res = {
> .server = server,
> };
> struct rpc_message msg = {
> diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
> index 7a098ae..b0bd7ef 100644
> --- a/fs/nfs/nfs4xdr.c
> +++ b/fs/nfs/nfs4xdr.c
> @@ -4873,7 +4873,7 @@ out:
> /*
> * Decode RENAME response
> */
> -static int nfs4_xdr_dec_rename(struct rpc_rqst *rqstp, __be32 *p, struct nfs4_rename_res *res)
> +static int nfs4_xdr_dec_rename(struct rpc_rqst *rqstp, __be32 *p, struct nfs_renameres *res)
> {
> struct xdr_stream xdr;
> struct compound_hdr hdr;
> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> index 60fa509..6265fa8 100644
> --- a/include/linux/nfs_xdr.h
> +++ b/include/linux/nfs_xdr.h
> @@ -411,6 +411,15 @@ struct nfs_renameargs {
> struct nfs4_sequence_args seq_args;
> };
>
> +struct nfs_renameres {
> + const struct nfs_server * server;
> + struct nfs4_change_info old_cinfo;
> + struct nfs_fattr * old_fattr;
> + struct nfs4_change_info new_cinfo;
> + struct nfs_fattr * new_fattr;
> + struct nfs4_sequence_res seq_res;
> +};
Likewise. Is #ifdef CONFIG_NFS_V4 needed here? I guess the question is, have you tried building with CONFIG_NFS_V4 disabled?
> +
> /*
> * Argument struct for decode_entry function
> */
> @@ -623,11 +632,6 @@ struct nfs3_readlinkargs {
> struct page ** pages;
> };
>
> -struct nfs3_renameres {
> - struct nfs_fattr * fromattr;
> - struct nfs_fattr * toattr;
> -};
> -
> struct nfs3_linkres {
> struct nfs_fattr * dir_attr;
> struct nfs_fattr * fattr;
> @@ -795,15 +799,6 @@ struct nfs4_readlink_res {
> struct nfs4_sequence_res seq_res;
> };
>
> -struct nfs4_rename_res {
> - const struct nfs_server * server;
> - struct nfs4_change_info old_cinfo;
> - struct nfs_fattr * old_fattr;
> - struct nfs4_change_info new_cinfo;
> - struct nfs_fattr * new_fattr;
> - struct nfs4_sequence_res seq_res;
> -};
> -
> #define NFS4_SETCLIENTID_NAMELEN (127)
> struct nfs4_setclientid {
> const nfs4_verifier * sc_verifier;
> --
> 1.7.1
>
> --
> 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
--
chuck[dot]lever[at]oracle[dot]com
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/8] nfs: eliminate nfs4_rename_arg
2010-09-15 15:26 ` Chuck Lever
@ 2010-09-15 15:34 ` Jeff Layton
2010-09-15 16:45 ` Trond Myklebust
1 sibling, 0 replies; 19+ messages in thread
From: Jeff Layton @ 2010-09-15 15:34 UTC (permalink / raw)
To: Chuck Lever; +Cc: trond.myklebust, linux-nfs
On Wed, 15 Sep 2010 11:26:49 -0400
Chuck Lever <chuck.lever@oracle.com> wrote:
>
> On Sep 15, 2010, at 9:23 AM, Jeff Layton wrote:
>
> > Add the missing fields to nfs_renameargs and drop nfs4_rename_arg
> >
> > Signed-off-by: Jeff Layton <jlayton@redhat.com>
> > ---
> > fs/nfs/nfs4proc.c | 2 +-
> > fs/nfs/nfs4xdr.c | 2 +-
> > include/linux/nfs_xdr.h | 19 ++++++-------------
> > 3 files changed, 8 insertions(+), 15 deletions(-)
> >
> > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> > index 089da5b..eb36784 100644
> > --- a/fs/nfs/nfs4proc.c
> > +++ b/fs/nfs/nfs4proc.c
> > @@ -2675,7 +2675,7 @@ 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 nfs4_rename_arg arg = {
> > + struct nfs_renameargs arg = {
> > .old_dir = NFS_FH(old_dir),
> > .new_dir = NFS_FH(new_dir),
> > .old_name = old_name,
> > diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
> > index 08ef912..7a098ae 100644
> > --- a/fs/nfs/nfs4xdr.c
> > +++ b/fs/nfs/nfs4xdr.c
> > @@ -1823,7 +1823,7 @@ static int nfs4_xdr_enc_remove(struct rpc_rqst *req, __be32 *p, const struct nfs
> > /*
> > * Encode RENAME request
> > */
> > -static int nfs4_xdr_enc_rename(struct rpc_rqst *req, __be32 *p, const struct nfs4_rename_arg *args)
> > +static int nfs4_xdr_enc_rename(struct rpc_rqst *req, __be32 *p, const struct nfs_renameargs *args)
> > {
> > struct xdr_stream xdr;
> > struct compound_hdr hdr = {
> > diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> > index 5436680..60fa509 100644
> > --- a/include/linux/nfs_xdr.h
> > +++ b/include/linux/nfs_xdr.h
> > @@ -403,10 +403,12 @@ struct nfs_removeres {
> > * Common arguments to the rename call
> > */
> > struct nfs_renameargs {
> > - const struct nfs_fh * old_dir;
> > - const struct nfs_fh * new_dir;
> > - const struct qstr * old_name;
> > - const struct qstr * new_name;
> > + const struct nfs_fh * old_dir;
> > + const struct nfs_fh * new_dir;
> > + const struct qstr * old_name;
> > + const struct qstr * new_name;
> > + const u32 * bitmask;
> > + struct nfs4_sequence_args seq_args;
>
> Should these new fields be gated by CONFIG_NFS_V4 ?
>
I suppose they could be. nfs_removeargs has the same fields though and
is not gated like that.
> > };
> >
> > /*
> > @@ -793,15 +795,6 @@ struct nfs4_readlink_res {
> > struct nfs4_sequence_res seq_res;
> > };
> >
> > -struct nfs4_rename_arg {
> > - const struct nfs_fh * old_dir;
> > - const struct nfs_fh * new_dir;
> > - const struct qstr * old_name;
> > - const struct qstr * new_name;
> > - const u32 * bitmask;
> > - struct nfs4_sequence_args seq_args;
> > -};
> > -
> > struct nfs4_rename_res {
> > const struct nfs_server * server;
> > struct nfs4_change_info old_cinfo;
> > --
> > 1.7.1
> >
> > --
> > 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
>
--
Jeff Layton <jlayton@redhat.com>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 4/8] nfs: standardize the rename response container
2010-09-15 15:29 ` Chuck Lever
@ 2010-09-15 15:38 ` Jeff Layton
0 siblings, 0 replies; 19+ messages in thread
From: Jeff Layton @ 2010-09-15 15:38 UTC (permalink / raw)
To: Chuck Lever; +Cc: trond.myklebust, linux-nfs
On Wed, 15 Sep 2010 11:29:38 -0400
Chuck Lever <chuck.lever@oracle.com> wrote:
> > index 60fa509..6265fa8 100644
> > --- a/include/linux/nfs_xdr.h
> > +++ b/include/linux/nfs_xdr.h
> > @@ -411,6 +411,15 @@ struct nfs_renameargs {
> > struct nfs4_sequence_args seq_args;
> > };
> >
> > +struct nfs_renameres {
> > + const struct nfs_server * server;
> > + struct nfs4_change_info old_cinfo;
> > + struct nfs_fattr * old_fattr;
> > + struct nfs4_change_info new_cinfo;
> > + struct nfs_fattr * new_fattr;
> > + struct nfs4_sequence_res seq_res;
> > +};
>
> Likewise. Is #ifdef CONFIG_NFS_V4 needed here? I guess the question is, have you tried building with CONFIG_NFS_V4 disabled?
>
Yes, I have built this with v4 disabled and it worked just fine. The
#ifdef isn't really needed. I suppose we could add that in to save
a bit of memory but it's probably not worth it. Note that nfs_removeres
has the same sort of fields and they aren't wrapped in #ifdef's.
--
Jeff Layton <jlayton@redhat.com>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 6/8] nfs: add a rename_setup nfs_rpc_op for async renames
2010-09-15 13:23 ` [PATCH 6/8] nfs: add a rename_setup nfs_rpc_op for async renames Jeff Layton
@ 2010-09-15 15:39 ` Chuck Lever
2010-09-15 17:23 ` Jeff Layton
0 siblings, 1 reply; 19+ messages in thread
From: Chuck Lever @ 2010-09-15 15:39 UTC (permalink / raw)
To: Jeff Layton; +Cc: trond.myklebust, linux-nfs
On Sep 15, 2010, at 9:23 AM, Jeff Layton wrote:
> Add a call that can set up the arguments and the response container for
> an asynchronous rename call.
>
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
> fs/nfs/nfs3proc.c | 18 ++++++++++++++++++
> fs/nfs/nfs4proc.c | 21 +++++++++++++++++++++
> fs/nfs/proc.c | 12 ++++++++++++
> include/linux/nfs_xdr.h | 1 +
> 4 files changed, 52 insertions(+), 0 deletions(-)
>
> diff --git a/fs/nfs/nfs3proc.c b/fs/nfs/nfs3proc.c
> index c5cccf1..0654177 100644
> --- a/fs/nfs/nfs3proc.c
> +++ b/fs/nfs/nfs3proc.c
> @@ -439,6 +439,23 @@ nfs3_proc_unlink_done(struct rpc_task *task, struct inode *dir)
> }
>
> static int
> +nfs3_proc_rename_setup(struct rpc_message *msg, struct inode *dir)
> +{
> + struct nfs_renameres *res = msg->rpc_resp;
> +
> + msg->rpc_proc = &nfs3_procedures[NFS3PROC_RENAME];
> +
> + res->old_fattr = nfs_alloc_fattr();
> + res->new_fattr = nfs_alloc_fattr();
> + if (res->old_fattr != NULL && res->new_fattr != NULL)
> + return 0;
> +
> + nfs_free_fattr(res->old_fattr);
> + nfs_free_fattr(res->new_fattr);
> + return -ENOMEM;
> +}
This is just a style nit, but my understanding is that there is a stated preference for the error case to be handled in the statement block after the NULL checks, and the common case should exit through the tail of the function.
I suspect that these last three patches can be merged together for submission. I read somewhere recently that the powers that be do not like patches that introduce new code that isn't used by anything.
> +
> +static int
> nfs3_proc_rename(struct inode *old_dir, struct qstr *old_name,
> struct inode *new_dir, struct qstr *new_name)
> {
> @@ -842,6 +859,7 @@ const struct nfs_rpc_ops nfs_v3_clientops = {
> .unlink_setup = nfs3_proc_unlink_setup,
> .unlink_done = nfs3_proc_unlink_done,
> .rename = nfs3_proc_rename,
> + .rename_setup = nfs3_proc_rename_setup,
> .link = nfs3_proc_link,
> .symlink = nfs3_proc_symlink,
> .mkdir = nfs3_proc_mkdir,
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 120a8a6..011dc90 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -2671,6 +2671,26 @@ static int nfs4_proc_unlink_done(struct rpc_task *task, struct inode *dir)
> return 1;
> }
>
> +static int nfs4_proc_rename_setup(struct rpc_message *msg, struct inode *dir)
> +{
> + struct nfs_server *server = NFS_SERVER(dir);
> + struct nfs_renameargs *arg = msg->rpc_argp;
> + struct nfs_renameres *res = msg->rpc_resp;
> +
> + msg->rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_RENAME];
> + arg->bitmask = server->attr_bitmask;
> + res->server = server;
> +
> + res->old_fattr = nfs_alloc_fattr();
> + res->new_fattr = nfs_alloc_fattr();
> + if (res->old_fattr != NULL && res->new_fattr != NULL)
> + return 0;
> +
> + nfs_free_fattr(res->old_fattr);
> + nfs_free_fattr(res->new_fattr);
> + return -ENOMEM;
> +}
> +
> static int _nfs4_proc_rename(struct inode *old_dir, struct qstr *old_name,
> struct inode *new_dir, struct qstr *new_name)
> {
> @@ -5443,6 +5463,7 @@ const struct nfs_rpc_ops nfs_v4_clientops = {
> .unlink_setup = nfs4_proc_unlink_setup,
> .unlink_done = nfs4_proc_unlink_done,
> .rename = nfs4_proc_rename,
> + .rename_setup = nfs4_proc_rename_setup,
> .link = nfs4_proc_link,
> .symlink = nfs4_proc_symlink,
> .mkdir = nfs4_proc_mkdir,
> diff --git a/fs/nfs/proc.c b/fs/nfs/proc.c
> index 5c70b58..1b02dee 100644
> --- a/fs/nfs/proc.c
> +++ b/fs/nfs/proc.c
> @@ -366,6 +366,17 @@ static int nfs_proc_unlink_done(struct rpc_task *task, struct inode *dir)
> }
>
> static int
> +nfs_proc_rename_setup(struct rpc_message *msg, struct inode *dir)
> +{
> + struct nfs_renameres *res = msg->rpc_resp;
> +
> + msg->rpc_proc = &nfs_procedures[NFSPROC_RENAME];
> + res->old_fattr = NULL;
> + res->new_fattr = NULL;
> + return 0;
> +}
> +
> +static int
> nfs_proc_rename(struct inode *old_dir, struct qstr *old_name,
> struct inode *new_dir, struct qstr *new_name)
> {
> @@ -703,6 +714,7 @@ const struct nfs_rpc_ops nfs_v2_clientops = {
> .unlink_setup = nfs_proc_unlink_setup,
> .unlink_done = nfs_proc_unlink_done,
> .rename = nfs_proc_rename,
> + .rename_setup = nfs_proc_rename_setup,
> .link = nfs_proc_link,
> .symlink = nfs_proc_symlink,
> .mkdir = nfs_proc_mkdir,
> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> index 6265fa8..d470751 100644
> --- a/include/linux/nfs_xdr.h
> +++ b/include/linux/nfs_xdr.h
> @@ -1018,6 +1018,7 @@ struct nfs_rpc_ops {
> int (*unlink_done) (struct rpc_task *, struct inode *);
> int (*rename) (struct inode *, struct qstr *,
> struct inode *, struct qstr *);
> + int (*rename_setup) (struct rpc_message *msg, struct inode *dir);
> int (*link) (struct inode *, struct inode *, struct qstr *);
> int (*symlink) (struct inode *, struct dentry *, struct page *,
> unsigned int, struct iattr *);
> --
> 1.7.1
>
> --
> 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
--
chuck[dot]lever[at]oracle[dot]com
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/8] nfs: eliminate nfs4_rename_arg
2010-09-15 15:26 ` Chuck Lever
2010-09-15 15:34 ` Jeff Layton
@ 2010-09-15 16:45 ` Trond Myklebust
2010-09-15 16:50 ` Chuck Lever
1 sibling, 1 reply; 19+ messages in thread
From: Trond Myklebust @ 2010-09-15 16:45 UTC (permalink / raw)
To: Chuck Lever; +Cc: Jeff Layton, linux-nfs
On Wed, 2010-09-15 at 11:26 -0400, Chuck Lever wrote:
> On Sep 15, 2010, at 9:23 AM, Jeff Layton wrote:
>
> > Add the missing fields to nfs_renameargs and drop nfs4_rename_arg
> >
> > Signed-off-by: Jeff Layton <jlayton@redhat.com>
> > ---
> > fs/nfs/nfs4proc.c | 2 +-
> > fs/nfs/nfs4xdr.c | 2 +-
> > include/linux/nfs_xdr.h | 19 ++++++-------------
> > 3 files changed, 8 insertions(+), 15 deletions(-)
> >
> > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> > index 089da5b..eb36784 100644
> > --- a/fs/nfs/nfs4proc.c
> > +++ b/fs/nfs/nfs4proc.c
> > @@ -2675,7 +2675,7 @@ 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 nfs4_rename_arg arg = {
> > + struct nfs_renameargs arg = {
> > .old_dir = NFS_FH(old_dir),
> > .new_dir = NFS_FH(new_dir),
> > .old_name = old_name,
> > diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
> > index 08ef912..7a098ae 100644
> > --- a/fs/nfs/nfs4xdr.c
> > +++ b/fs/nfs/nfs4xdr.c
> > @@ -1823,7 +1823,7 @@ static int nfs4_xdr_enc_remove(struct rpc_rqst *req, __be32 *p, const struct nfs
> > /*
> > * Encode RENAME request
> > */
> > -static int nfs4_xdr_enc_rename(struct rpc_rqst *req, __be32 *p, const struct nfs4_rename_arg *args)
> > +static int nfs4_xdr_enc_rename(struct rpc_rqst *req, __be32 *p, const struct nfs_renameargs *args)
> > {
> > struct xdr_stream xdr;
> > struct compound_hdr hdr = {
> > diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> > index 5436680..60fa509 100644
> > --- a/include/linux/nfs_xdr.h
> > +++ b/include/linux/nfs_xdr.h
> > @@ -403,10 +403,12 @@ struct nfs_removeres {
> > * Common arguments to the rename call
> > */
> > struct nfs_renameargs {
> > - const struct nfs_fh * old_dir;
> > - const struct nfs_fh * new_dir;
> > - const struct qstr * old_name;
> > - const struct qstr * new_name;
> > + const struct nfs_fh * old_dir;
> > + const struct nfs_fh * new_dir;
> > + const struct qstr * old_name;
> > + const struct qstr * new_name;
> > + const u32 * bitmask;
> > + struct nfs4_sequence_args seq_args;
>
> Should these new fields be gated by CONFIG_NFS_V4 ?
No. Please don't add any more #ifdef CONFIG_NFS_V4 sections: that just
adds to the testing duties. The job now is to get rid of
CONFIG_NFS_V..., not to add more to it.
Trond
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/8] nfs: eliminate nfs4_rename_arg
2010-09-15 16:45 ` Trond Myklebust
@ 2010-09-15 16:50 ` Chuck Lever
0 siblings, 0 replies; 19+ messages in thread
From: Chuck Lever @ 2010-09-15 16:50 UTC (permalink / raw)
To: Trond Myklebust; +Cc: Jeff Layton, linux-nfs
On Sep 15, 2010, at 12:45 PM, Trond Myklebust wrote:
> On Wed, 2010-09-15 at 11:26 -0400, Chuck Lever wrote:
>> On Sep 15, 2010, at 9:23 AM, Jeff Layton wrote:
>>
>>> Add the missing fields to nfs_renameargs and drop nfs4_rename_arg
>>>
>>> Signed-off-by: Jeff Layton <jlayton@redhat.com>
>>> ---
>>> fs/nfs/nfs4proc.c | 2 +-
>>> fs/nfs/nfs4xdr.c | 2 +-
>>> include/linux/nfs_xdr.h | 19 ++++++-------------
>>> 3 files changed, 8 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>>> index 089da5b..eb36784 100644
>>> --- a/fs/nfs/nfs4proc.c
>>> +++ b/fs/nfs/nfs4proc.c
>>> @@ -2675,7 +2675,7 @@ 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 nfs4_rename_arg arg = {
>>> + struct nfs_renameargs arg = {
>>> .old_dir = NFS_FH(old_dir),
>>> .new_dir = NFS_FH(new_dir),
>>> .old_name = old_name,
>>> diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
>>> index 08ef912..7a098ae 100644
>>> --- a/fs/nfs/nfs4xdr.c
>>> +++ b/fs/nfs/nfs4xdr.c
>>> @@ -1823,7 +1823,7 @@ static int nfs4_xdr_enc_remove(struct rpc_rqst *req, __be32 *p, const struct nfs
>>> /*
>>> * Encode RENAME request
>>> */
>>> -static int nfs4_xdr_enc_rename(struct rpc_rqst *req, __be32 *p, const struct nfs4_rename_arg *args)
>>> +static int nfs4_xdr_enc_rename(struct rpc_rqst *req, __be32 *p, const struct nfs_renameargs *args)
>>> {
>>> struct xdr_stream xdr;
>>> struct compound_hdr hdr = {
>>> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
>>> index 5436680..60fa509 100644
>>> --- a/include/linux/nfs_xdr.h
>>> +++ b/include/linux/nfs_xdr.h
>>> @@ -403,10 +403,12 @@ struct nfs_removeres {
>>> * Common arguments to the rename call
>>> */
>>> struct nfs_renameargs {
>>> - const struct nfs_fh * old_dir;
>>> - const struct nfs_fh * new_dir;
>>> - const struct qstr * old_name;
>>> - const struct qstr * new_name;
>>> + const struct nfs_fh * old_dir;
>>> + const struct nfs_fh * new_dir;
>>> + const struct qstr * old_name;
>>> + const struct qstr * new_name;
>>> + const u32 * bitmask;
>>> + struct nfs4_sequence_args seq_args;
>>
>> Should these new fields be gated by CONFIG_NFS_V4 ?
>
> No. Please don't add any more #ifdef CONFIG_NFS_V4 sections: that just
> adds to the testing duties. The job now is to get rid of
> CONFIG_NFS_V..., not to add more to it.
The underlying question was whether this builds when CONFIG_NFS_V4 is disabled, and Jeff has confirmed that it does. Agreed that no change is needed here.
--
chuck[dot]lever[at]oracle[dot]com
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 6/8] nfs: add a rename_setup nfs_rpc_op for async renames
2010-09-15 15:39 ` Chuck Lever
@ 2010-09-15 17:23 ` Jeff Layton
0 siblings, 0 replies; 19+ messages in thread
From: Jeff Layton @ 2010-09-15 17:23 UTC (permalink / raw)
To: Chuck Lever; +Cc: trond.myklebust, linux-nfs
On Wed, 15 Sep 2010 11:39:38 -0400
Chuck Lever <chuck.lever@oracle.com> wrote:
>
> On Sep 15, 2010, at 9:23 AM, Jeff Layton wrote:
>
> > Add a call that can set up the arguments and the response container for
> > an asynchronous rename call.
> >
> > Signed-off-by: Jeff Layton <jlayton@redhat.com>
> > ---
> > fs/nfs/nfs3proc.c | 18 ++++++++++++++++++
> > fs/nfs/nfs4proc.c | 21 +++++++++++++++++++++
> > fs/nfs/proc.c | 12 ++++++++++++
> > include/linux/nfs_xdr.h | 1 +
> > 4 files changed, 52 insertions(+), 0 deletions(-)
> >
> > diff --git a/fs/nfs/nfs3proc.c b/fs/nfs/nfs3proc.c
> > index c5cccf1..0654177 100644
> > --- a/fs/nfs/nfs3proc.c
> > +++ b/fs/nfs/nfs3proc.c
> > @@ -439,6 +439,23 @@ nfs3_proc_unlink_done(struct rpc_task *task, struct inode *dir)
> > }
> >
> > static int
> > +nfs3_proc_rename_setup(struct rpc_message *msg, struct inode *dir)
> > +{
> > + struct nfs_renameres *res = msg->rpc_resp;
> > +
> > + msg->rpc_proc = &nfs3_procedures[NFS3PROC_RENAME];
> > +
> > + res->old_fattr = nfs_alloc_fattr();
> > + res->new_fattr = nfs_alloc_fattr();
> > + if (res->old_fattr != NULL && res->new_fattr != NULL)
> > + return 0;
> > +
> > + nfs_free_fattr(res->old_fattr);
> > + nfs_free_fattr(res->new_fattr);
> > + return -ENOMEM;
> > +}
>
> This is just a style nit, but my understanding is that there is a stated preference for the error case to be handled in the statement block after the NULL checks, and the common case should exit through the tail of the function.
>
Fair enough. I can change that.
> I suspect that these last three patches can be merged together for submission. I read somewhere recently that the powers that be do not like patches that introduce new code that isn't used by anything.
>
I can do that. I figured this would make it easier to review, but I'm
fine doing it that way too.
Thanks for the review.
--
Jeff Layton <jlayton@redhat.com>
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2010-09-15 17:24 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-09-15 13:23 [PATCH 0/8] nfs: ensure that sillyrenames run to completion (try #2) Jeff Layton
2010-09-15 13:23 ` [PATCH 1/8] nfs: eliminate nfs3_renameargs Jeff Layton
2010-09-15 13:23 ` [PATCH 2/8] nfs: convert nfs_renameargs to use qstr structs Jeff Layton
2010-09-15 13:23 ` [PATCH 3/8] nfs: eliminate nfs4_rename_arg Jeff Layton
2010-09-15 15:26 ` Chuck Lever
2010-09-15 15:34 ` Jeff Layton
2010-09-15 16:45 ` Trond Myklebust
2010-09-15 16:50 ` Chuck Lever
2010-09-15 13:23 ` [PATCH 4/8] nfs: standardize the rename response container Jeff Layton
2010-09-15 15:29 ` Chuck Lever
2010-09-15 15:38 ` Jeff Layton
2010-09-15 13:23 ` [PATCH 5/8] nfs: move nfs_sillyrename to unlink.c Jeff Layton
2010-09-15 13:23 ` [PATCH 6/8] nfs: add a rename_setup nfs_rpc_op for async renames Jeff Layton
2010-09-15 15:39 ` Chuck Lever
2010-09-15 17:23 ` Jeff Layton
2010-09-15 13:24 ` [PATCH 7/8] nfs: add rename_done nfs_rpc_op Jeff Layton
2010-09-15 13:24 ` [PATCH 8/8] nfs: make sillyrename an async operation Jeff Layton
2010-09-15 13:37 ` Jeff Layton
2010-09-15 15:04 ` Jeff Layton
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.