by Myklebust, Trond

[permalink] [raw]

Subject: [PATCH 14/15] NFSv41: Cleanup for nfs4_alloc_session.

There is no reason to change the nfs_client state every time we allocate a
new session. Move that line into nfs4_init_client_minor_version.

Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/client.c | 7 +++++++
fs/nfs/nfs4proc.c | 7 -------
2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 123fcbd..544810f 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -1157,6 +1157,13 @@ static int nfs4_init_client_minor_version(struct nfs_client *clp)
return -ENOMEM;

clp->cl_session = session;
+ /*
+ * The create session reply races with the server back
+ * channel probe. Mark the client NFS_CS_SESSION_INITING
+ * so that the client back channel can find the
+ * nfs_client struct
+ */
+ clp->cl_cons_state = NFS_CS_SESSION_INITING;
}
#endif /* CONFIG_NFS_V4_1 */

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index e9913de..1245939 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -4795,13 +4795,6 @@ struct nfs4_session *nfs4_alloc_session(struct nfs_client *clp)
if (!session)
return NULL;

- /*
- * The create session reply races with the server back
- * channel probe. Mark the client NFS_CS_SESSION_INITING
- * so that the client back channel can find the
- * nfs_client struct
- */
- clp->cl_cons_state = NFS_CS_SESSION_INITING;
init_completion(&session->complete);

tbl = &session->fc_slot_table;
--
1.7.0.1

2010-06-14 23:14:00

by Benny Halevy

[permalink] [raw]

Subject: Re: [PATCH 13/15] NFSv41: Clean up exclusive create

On Jun. 14, 2010, 17:51 -0400, Trond Myklebust <[email protected]> wrote:
> Signed-off-by: Trond Myklebust <[email protected]>
> ---
> fs/nfs/nfs4proc.c | 17 ++++++-----------
> include/linux/nfs_xdr.h | 6 ++++--
> 2 files changed, 10 insertions(+), 13 deletions(-)
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index ef613ea..e9913de 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -744,19 +744,14 @@ static struct nfs4_opendata *nfs4_opendata_alloc(struct path *path,
> p->o_arg.server = server;
> p->o_arg.bitmask = server->attr_bitmask;
> p->o_arg.claim = NFS4_OPEN_CLAIM_NULL;
> - if (flags & O_EXCL) {
> - if (nfs4_has_persistent_session(server->nfs_client)) {
> - /* GUARDED */
> - p->o_arg.u.attrs = &p->attrs;
> - memcpy(&p->attrs, attrs, sizeof(p->attrs));

What about this case?
Aren't we losing the attrs?

Benny

> - } else { /* EXCLUSIVE4_1 */
> - u32 *s = (u32 *) p->o_arg.u.verifier.data;
> - s[0] = jiffies;
> - s[1] = current->pid;
> - }
> - } else if (flags & O_CREAT) {
> + if (flags & O_CREAT) {
> + u32 *s;
> +
> p->o_arg.u.attrs = &p->attrs;
> memcpy(&p->attrs, attrs, sizeof(p->attrs));
> + s = (u32 *) p->o_arg.u.verifier.data;
> + s[0] = jiffies;
> + s[1] = current->pid;
> }
> p->c_arg.fh = &p->o_res.fh;
> p->c_arg.stateid = &p->o_res.stateid;
> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> index 51914d7..a319cb9 100644
> --- a/include/linux/nfs_xdr.h
> +++ b/include/linux/nfs_xdr.h
> @@ -196,8 +196,10 @@ struct nfs_openargs {
> __u64 clientid;
> __u64 id;
> union {
> - struct iattr * attrs; /* UNCHECKED, GUARDED */
> - nfs4_verifier verifier; /* EXCLUSIVE */
> + struct {
> + struct iattr * attrs; /* UNCHECKED, GUARDED */
> + nfs4_verifier verifier; /* EXCLUSIVE */
> + };
> nfs4_stateid delegation; /* CLAIM_DELEGATE_CUR */
> fmode_t delegation_type; /* CLAIM_PREVIOUS */
> } u;