2011-12-12 20:58:24

by Tigran Mkrtchyan

[permalink] [raw]
Subject: [PATH v4 0/3] nfsd41: current state id processing

From: Tigran Mkrtchyan <[email protected]>


Yet another update (v4) of current stateid handling

difference with v3:

Patches re-organized to only add functionality, e.g. there
are no patches fixing other patches. Operation flags are
not used any more. Existence of a corresponding function
pointer is enough.


Let me know if you want to see updates less often.

Tigran.

currently tested to work in a single compound:

OPEN+CLOSE
LOCK+WRITE+LOCKU+CLOSE

On the way (not tested yet):
OPEN+LAYOUTGET

TODO:
dispose current stateid on operations which provide CFH, but do not provide stateid.

Internals:
1. struct nfsd4_compound_state contains pointer to current stateid
2. during compound processing before operation execution current stateid copied into
operations stateid if it's equal to corresponding value ( 0, 1).
3. after operation execution current stateid changed to:
a) point to stateid of last operation
or
b) point to NULL, if operation is marked to do so.

Probably all patches have to be squashed into a single one before merged
as none of the changes makes sense without others.

Tigran.


Tigran Mkrtchyan (3):
nfsd41: handle current stateid in open and close
nfsd41: handle current stateid on lock and locku
nfsd41: consume current stateid on read and write

fs/nfsd/current_stateid.h | 22 +++++++++++++++
fs/nfsd/nfs4proc.c | 39 +++++++++++++++++++++++---
fs/nfsd/nfs4state.c | 66 +++++++++++++++++++++++++++++++++++++++++++++
fs/nfsd/xdr4.h | 1 +
4 files changed, 123 insertions(+), 5 deletions(-)
create mode 100644 fs/nfsd/current_stateid.h

--
1.7.7.4



2011-12-12 20:58:27

by Tigran Mkrtchyan

[permalink] [raw]
Subject: [PATH v4 3/3] nfsd41: consume current stateid on read and write

From: Tigran Mkrtchyan <[email protected]>


Signed-off-by: Tigran Mkrtchyan <[email protected]>
---
fs/nfsd/current_stateid.h | 2 ++
fs/nfsd/nfs4proc.c | 2 ++
fs/nfsd/nfs4state.c | 12 ++++++++++++
3 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/fs/nfsd/current_stateid.h b/fs/nfsd/current_stateid.h
index 21550b6..6e54d19 100644
--- a/fs/nfsd/current_stateid.h
+++ b/fs/nfsd/current_stateid.h
@@ -16,5 +16,7 @@ extern void nfsd4_set_closestateid(struct nfsd4_compound_state *, struct nfsd4_c
*/
extern void nfsd4_get_closestateid(struct nfsd4_compound_state *, struct nfsd4_close *);
extern void nfsd4_get_lockustateid(struct nfsd4_compound_state *, struct nfsd4_locku *);
+extern void nfsd4_get_readstateid(struct nfsd4_compound_state *, struct nfsd4_read *);
+extern void nfsd4_get_writestateid(struct nfsd4_compound_state *, struct nfsd4_write *);

#endif /* _NFSD4_CURRENT_STATE_H */
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 90c532b..1d2c8d5 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1550,6 +1550,7 @@ static struct nfsd4_operation nfsd4_ops[] = {
.op_flags = OP_MODIFIES_SOMETHING,
.op_name = "OP_READ",
.op_rsize_bop = (nfsd4op_rsize)nfsd4_read_rsize,
+ .op_get_currentstateid = (stateid_getter)nfsd4_get_readstateid,
},
[OP_READDIR] = {
.op_func = (nfsd4op_func)nfsd4_readdir,
@@ -1628,6 +1629,7 @@ static struct nfsd4_operation nfsd4_ops[] = {
.op_flags = OP_MODIFIES_SOMETHING | OP_CACHEME,
.op_name = "OP_WRITE",
.op_rsize_bop = (nfsd4op_rsize)nfsd4_write_rsize,
+ .op_get_currentstateid = (stateid_getter)nfsd4_get_writestateid,
},
[OP_RELEASE_LOCKOWNER] = {
.op_func = (nfsd4op_func)nfsd4_release_lockowner,
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 58410b2..50d7ad0 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4599,3 +4599,15 @@ nfsd4_get_lockustateid(struct nfsd4_compound_state *cstate, struct nfsd4_locku *
{
get_stateid(cstate, &locku->lu_stateid);
}
+
+void
+nfsd4_get_readstateid(struct nfsd4_compound_state *cstate, struct nfsd4_read *read)
+{
+ get_stateid(cstate, &read->rd_stateid);
+}
+
+void
+nfsd4_get_writestateid(struct nfsd4_compound_state *cstate, struct nfsd4_write *write)
+{
+ get_stateid(cstate, &write->wr_stateid);
+}
--
1.7.7.4


2011-12-13 08:14:45

by Benny Halevy

[permalink] [raw]
Subject: Re: [PATH v4 2/3] nfsd41: handle current stateid on lock and locku

On 2011-12-12 23:00, Tigran Mkrtchyan wrote:
> From: Tigran Mkrtchyan <[email protected]>
>
> plus minor fixes

hopefully that's not the case anymore :)

>
> Conflicts:
>
> fs/nfsd/nfs4proc.c

ditto

>
> Signed-off-by: Tigran Mkrtchyan <[email protected]>
> ---
> fs/nfsd/current_stateid.h | 11 ++++++++++-
> fs/nfsd/nfs4proc.c | 2 ++
> fs/nfsd/nfs4state.c | 22 ++++++++++++++++++++--
> 3 files changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/fs/nfsd/current_stateid.h b/fs/nfsd/current_stateid.h
> index a83dd50..21550b6 100644
> --- a/fs/nfsd/current_stateid.h
> +++ b/fs/nfsd/current_stateid.h
> @@ -4,8 +4,17 @@
> #include "state.h"
> #include "xdr4.h"
>
> +/*
> + * functions to set current state id
> + */
> extern void nfsd4_set_openstateid(struct nfsd4_compound_state *, struct nfsd4_open *);
> -extern void nfsd4_get_closestateid(struct nfsd4_compound_state *, struct nfsd4_close *);
> +extern void nfsd4_set_lockstateid(struct nfsd4_compound_state *, struct nfsd4_lock *);
> extern void nfsd4_set_closestateid(struct nfsd4_compound_state *, struct nfsd4_close *);
>
> +/*
> + * functions to consume current state id
> + */
> +extern void nfsd4_get_closestateid(struct nfsd4_compound_state *, struct nfsd4_close *);
> +extern void nfsd4_get_lockustateid(struct nfsd4_compound_state *, struct nfsd4_locku *);
> +
> #endif /* _NFSD4_CURRENT_STATE_H */
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 3dfb235..90c532b 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -1478,6 +1478,7 @@ static struct nfsd4_operation nfsd4_ops[] = {
> .op_flags = OP_MODIFIES_SOMETHING,
> .op_name = "OP_LOCK",
> .op_rsize_bop = (nfsd4op_rsize)nfsd4_lock_rsize,
> + .op_set_currentstateid = (stateid_setter)nfsd4_set_lockstateid,
> },
> [OP_LOCKT] = {
> .op_func = (nfsd4op_func)nfsd4_lockt,
> @@ -1488,6 +1489,7 @@ static struct nfsd4_operation nfsd4_ops[] = {
> .op_flags = OP_MODIFIES_SOMETHING,
> .op_name = "OP_LOCKU",
> .op_rsize_bop = (nfsd4op_rsize)nfsd4_status_stateid_rsize,
> + .op_get_currentstateid = (stateid_getter)nfsd4_get_lockustateid,
> },
> [OP_LOOKUP] = {
> .op_func = (nfsd4op_func)nfsd4_lookup,
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 594b44e..58410b2 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -4564,6 +4564,9 @@ put_stateid(struct nfsd4_compound_state *cstate, stateid_t *stateid)
> cstate->current_stateid = stateid;
> }
>
> +/*
> + * functions to set current state id
> + */
> void
> nfsd4_set_openstateid(struct nfsd4_compound_state *cstate, struct nfsd4_open *open)
> {
> @@ -4571,13 +4574,28 @@ nfsd4_set_openstateid(struct nfsd4_compound_state *cstate, struct nfsd4_open *op
> }
>
> void
> +nfsd4_set_closestateid(struct nfsd4_compound_state *cstate, struct nfsd4_close *close)
> +{
> + put_stateid(cstate, &close->cl_stateid);
> +}
> +
> +void
> +nfsd4_set_lockstateid(struct nfsd4_compound_state *cstate, struct nfsd4_lock *lock)
> +{
> + put_stateid(cstate, &lock->lk_resp_stateid);
> +}
> +
> +/*
> + * functions to consume current state id
> + */
> +void
> nfsd4_get_closestateid(struct nfsd4_compound_state *cstate, struct nfsd4_close *close)
> {
> get_stateid(cstate, &close->cl_stateid);
> }
>
> void
> -nfsd4_set_closestateid(struct nfsd4_compound_state *cstate, struct nfsd4_close *close)
> +nfsd4_get_lockustateid(struct nfsd4_compound_state *cstate, struct nfsd4_locku *locku)
> {
> - get_stateid(cstate, &close->cl_stateid);
> + get_stateid(cstate, &locku->lu_stateid);

Reading Section 8.2.3. Special Stateids, for some reason (that I'm not sure of)
RFC5661 requires the following:

The stateid passed to the operation in place
of the special value has its "seqid" value set to zero, except
when the current stateid is used by the operation CLOSE or
OPEN_DOWNGRADE.

How about defining another get_stateid helper that will clear si_generation?

Benny

> }

2011-12-12 20:58:25

by Tigran Mkrtchyan

[permalink] [raw]
Subject: [PATH v4 1/3] nfsd41: handle current stateid in open and close

From: Tigran Mkrtchyan <[email protected]>


Signed-off-by: Tigran Mkrtchyan <[email protected]>
---
fs/nfsd/current_stateid.h | 11 +++++++++++
fs/nfsd/nfs4proc.c | 35 ++++++++++++++++++++++++++++++-----
fs/nfsd/nfs4state.c | 36 ++++++++++++++++++++++++++++++++++++
fs/nfsd/xdr4.h | 1 +
4 files changed, 78 insertions(+), 5 deletions(-)
create mode 100644 fs/nfsd/current_stateid.h

diff --git a/fs/nfsd/current_stateid.h b/fs/nfsd/current_stateid.h
new file mode 100644
index 0000000..a83dd50
--- /dev/null
+++ b/fs/nfsd/current_stateid.h
@@ -0,0 +1,11 @@
+#ifndef _NFSD4_CURRENT_STATE_H
+#define _NFSD4_CURRENT_STATE_H
+
+#include "state.h"
+#include "xdr4.h"
+
+extern void nfsd4_set_openstateid(struct nfsd4_compound_state *, struct nfsd4_open *);
+extern void nfsd4_get_closestateid(struct nfsd4_compound_state *, struct nfsd4_close *);
+extern void nfsd4_set_closestateid(struct nfsd4_compound_state *, struct nfsd4_close *);
+
+#endif /* _NFSD4_CURRENT_STATE_H */
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index fa38336..3dfb235 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -39,6 +39,7 @@
#include "cache.h"
#include "xdr4.h"
#include "vfs.h"
+#include "current_stateid.h"

#define NFSDDBG_FACILITY NFSDDBG_PROC

@@ -556,7 +557,6 @@ nfsd4_create(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
create->cr_name, create->cr_namelen,
&create->cr_iattr, S_IFSOCK, 0, &resfh);
break;
-
case NF4FIFO:
status = nfsd_create(rqstp, &cstate->current_fh,
create->cr_name, create->cr_namelen,
@@ -1001,6 +1001,9 @@ static inline void nfsd4_increment_op_stats(u32 opnum)
typedef __be32(*nfsd4op_func)(struct svc_rqst *, struct nfsd4_compound_state *,
void *);
typedef u32(*nfsd4op_rsize)(struct svc_rqst *, struct nfsd4_op *op);
+typedef void(*stateid_setter)(struct nfsd4_compound_state *, void *);
+typedef void(*stateid_getter)(struct nfsd4_compound_state *, void *);
+typedef void(*stateid_cleaner)(struct nfsd4_compound_state *);

enum nfsd4_op_flags {
ALLOWED_WITHOUT_FH = 1 << 0, /* No current filehandle required */
@@ -1034,6 +1037,9 @@ struct nfsd4_operation {
char *op_name;
/* Try to get response size before operation */
nfsd4op_rsize op_rsize_bop;
+ stateid_setter op_get_currentstateid;
+ stateid_getter op_set_currentstateid;
+ stateid_cleaner op_clear_currentstateid;
};

static struct nfsd4_operation nfsd4_ops[];
@@ -1116,6 +1122,11 @@ static bool need_wrongsec_check(struct svc_rqst *rqstp)
return !(nextd->op_flags & OP_HANDLES_WRONGSEC);
}

+static void
+nfsd4_clear_currentstateid(struct nfsd4_compound_state *cstate)
+{
+ cstate->current_stateid = NULL;
+}
/*
* COMPOUND call.
*/
@@ -1216,13 +1227,24 @@ nfsd4_proc_compound(struct svc_rqst *rqstp,
if (op->status)
goto encode_op;

- if (opdesc->op_func)
+ if (opdesc->op_func) {
+ if (opdesc->op_get_currentstateid)
+ opdesc->op_get_currentstateid(cstate, &op->u);
op->status = opdesc->op_func(rqstp, cstate, &op->u);
- else
+ } else
BUG_ON(op->status == nfs_ok);

- if (!op->status && need_wrongsec_check(rqstp))
- op->status = check_nfsd_access(cstate->current_fh.fh_export, rqstp);
+ if (!op->status) {
+ if (opdesc->op_set_currentstateid) {
+ opdesc->op_set_currentstateid(cstate, &op->u);
+ }
+
+ if (opdesc->op_clear_currentstateid)
+ opdesc->op_clear_currentstateid(cstate);
+
+ if (need_wrongsec_check(rqstp))
+ op->status = check_nfsd_access(cstate->current_fh.fh_export, rqstp);
+ }

encode_op:
/* Only from SEQUENCE */
@@ -1414,6 +1436,8 @@ static struct nfsd4_operation nfsd4_ops[] = {
.op_flags = OP_MODIFIES_SOMETHING,
.op_name = "OP_CLOSE",
.op_rsize_bop = (nfsd4op_rsize)nfsd4_status_stateid_rsize,
+ .op_get_currentstateid = (stateid_getter)nfsd4_get_closestateid,
+ .op_set_currentstateid = (stateid_setter)nfsd4_set_closestateid,
},
[OP_COMMIT] = {
.op_func = (nfsd4op_func)nfsd4_commit,
@@ -1484,6 +1508,7 @@ static struct nfsd4_operation nfsd4_ops[] = {
.op_flags = OP_HANDLES_WRONGSEC | OP_MODIFIES_SOMETHING,
.op_name = "OP_OPEN",
.op_rsize_bop = (nfsd4op_rsize)nfsd4_open_rsize,
+ .op_set_currentstateid = (stateid_setter)nfsd4_set_openstateid,
},
[OP_OPEN_CONFIRM] = {
.op_func = (nfsd4op_func)nfsd4_open_confirm,
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 47e94e3..594b44e 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -51,10 +51,12 @@ time_t nfsd4_grace = 90;
static time_t boot_time;
static stateid_t zerostateid; /* bits all 0 */
static stateid_t onestateid; /* bits all 1 */
+static stateid_t currentstateid; /* other all 0, seqid 1 */
static u64 current_sessionid = 1;

#define ZERO_STATEID(stateid) (!memcmp((stateid), &zerostateid, sizeof(stateid_t)))
#define ONE_STATEID(stateid) (!memcmp((stateid), &onestateid, sizeof(stateid_t)))
+#define CURRENT_STATEID(stateid) (!memcmp((stateid), &currentstateid, sizeof(stateid_t)))

/* forward declarations */
static int check_for_locks(struct nfs4_file *filp, struct nfs4_lockowner *lowner);
@@ -4423,6 +4425,8 @@ nfs4_state_init(void)
INIT_LIST_HEAD(&lock_ownerstr_hashtbl[i]);
}
memset(&onestateid, ~0, sizeof(stateid_t));
+ /* seqid 1, other all 0 */
+ currentstateid.si_generation = 1;
INIT_LIST_HEAD(&close_lru);
INIT_LIST_HEAD(&client_lru);
INIT_LIST_HEAD(&del_recall_lru);
@@ -4545,3 +4549,35 @@ nfs4_state_shutdown(void)
nfs4_unlock_state();
nfsd4_destroy_callback_queue();
}
+
+static void
+get_stateid(struct nfsd4_compound_state *cstate, stateid_t *stateid)
+{
+ if (cstate->current_stateid && CURRENT_STATEID(stateid))
+ memcpy(stateid, cstate->current_stateid, sizeof(stateid_t));
+}
+
+static void
+put_stateid(struct nfsd4_compound_state *cstate, stateid_t *stateid)
+{
+ if (cstate->minorversion)
+ cstate->current_stateid = stateid;
+}
+
+void
+nfsd4_set_openstateid(struct nfsd4_compound_state *cstate, struct nfsd4_open *open)
+{
+ put_stateid(cstate, &open->op_stateid);
+}
+
+void
+nfsd4_get_closestateid(struct nfsd4_compound_state *cstate, struct nfsd4_close *close)
+{
+ get_stateid(cstate, &close->cl_stateid);
+}
+
+void
+nfsd4_set_closestateid(struct nfsd4_compound_state *cstate, struct nfsd4_close *close)
+{
+ get_stateid(cstate, &close->cl_stateid);
+}
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index 2364747..c1fe8ba 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -54,6 +54,7 @@ struct nfsd4_compound_state {
size_t iovlen;
u32 minorversion;
u32 status;
+ const stateid_t *current_stateid;
};

static inline bool nfsd4_has_session(struct nfsd4_compound_state *cs)
--
1.7.7.4


2011-12-14 17:01:18

by Mkrtchyan, Tigran

[permalink] [raw]
Subject: Re: [PATH v4 1/3] nfsd41: handle current stateid in open and close

On Tue, Dec 13, 2011 at 5:44 PM, J. Bruce Fields <[email protected]> wrote:
> On Mon, Dec 12, 2011 at 10:00:25PM +0100, Tigran Mkrtchyan wrote:
>> From: Tigran Mkrtchyan <[email protected]>
>>
>>
>> Signed-off-by: Tigran Mkrtchyan <[email protected]>
>> ---
>>  fs/nfsd/current_stateid.h |   11 +++++++++++
>>  fs/nfsd/nfs4proc.c        |   35 ++++++++++++++++++++++++++++++-----
>>  fs/nfsd/nfs4state.c       |   36 ++++++++++++++++++++++++++++++++++++
>>  fs/nfsd/xdr4.h            |    1 +
>>  4 files changed, 78 insertions(+), 5 deletions(-)
>>  create mode 100644 fs/nfsd/current_stateid.h
>>
>> diff --git a/fs/nfsd/current_stateid.h b/fs/nfsd/current_stateid.h
>> new file mode 100644
>> index 0000000..a83dd50
>> --- /dev/null
>> +++ b/fs/nfsd/current_stateid.h
>> @@ -0,0 +1,11 @@
>> +#ifndef _NFSD4_CURRENT_STATE_H
>> +#define _NFSD4_CURRENT_STATE_H
>> +
>> +#include "state.h"
>> +#include "xdr4.h"
>> +
>> +extern void nfsd4_set_openstateid(struct nfsd4_compound_state *, struct nfsd4_open *);
>> +extern void nfsd4_get_closestateid(struct nfsd4_compound_state *, struct nfsd4_close *);
>> +extern void nfsd4_set_closestateid(struct nfsd4_compound_state *, struct nfsd4_close *);
>> +
>> +#endif   /* _NFSD4_CURRENT_STATE_H */
>> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
>> index fa38336..3dfb235 100644
>> --- a/fs/nfsd/nfs4proc.c
>> +++ b/fs/nfsd/nfs4proc.c
>> @@ -39,6 +39,7 @@
>>  #include "cache.h"
>>  #include "xdr4.h"
>>  #include "vfs.h"
>> +#include "current_stateid.h"
>>
>>  #define NFSDDBG_FACILITY             NFSDDBG_PROC
>>
>> @@ -556,7 +557,6 @@ nfsd4_create(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>>                                    create->cr_name, create->cr_namelen,
>>                                    &create->cr_iattr, S_IFSOCK, 0, &resfh);
>>               break;
>> -
>>       case NF4FIFO:
>>               status = nfsd_create(rqstp, &cstate->current_fh,
>>                                    create->cr_name, create->cr_namelen,
>> @@ -1001,6 +1001,9 @@ static inline void nfsd4_increment_op_stats(u32 opnum)
>>  typedef __be32(*nfsd4op_func)(struct svc_rqst *, struct nfsd4_compound_state *,
>>                             void *);
>>  typedef u32(*nfsd4op_rsize)(struct svc_rqst *, struct nfsd4_op *op);
>> +typedef void(*stateid_setter)(struct nfsd4_compound_state *, void *);
>> +typedef void(*stateid_getter)(struct nfsd4_compound_state *, void *);
>> +typedef void(*stateid_cleaner)(struct nfsd4_compound_state *);
>>
>>  enum nfsd4_op_flags {
>>       ALLOWED_WITHOUT_FH = 1 << 0,    /* No current filehandle required */
>> @@ -1034,6 +1037,9 @@ struct nfsd4_operation {
>>       char *op_name;
>>       /* Try to get response size before operation */
>>       nfsd4op_rsize op_rsize_bop;
>> +     stateid_setter op_get_currentstateid;
>> +     stateid_getter op_set_currentstateid;
>> +     stateid_cleaner op_clear_currentstateid;
>
> Is op_clear_currentstateid ever going to have more than one
> implementation?
>
> If not, let's just make that one a flag and do
>
>        if (clear_currentstateid flag set on this op)
>                cstate->current_stateid = NULL;
>

ack

Tigran.

> --b.
>
>>  };
>>
>>  static struct nfsd4_operation nfsd4_ops[];
>> @@ -1116,6 +1122,11 @@ static bool need_wrongsec_check(struct svc_rqst *rqstp)
>>       return !(nextd->op_flags & OP_HANDLES_WRONGSEC);
>>  }
>>
>> +static void
>> +nfsd4_clear_currentstateid(struct nfsd4_compound_state *cstate)
>> +{
>> +     cstate->current_stateid = NULL;
>> +}
>>  /*
>>   * COMPOUND call.
>>   */
>> @@ -1216,13 +1227,24 @@ nfsd4_proc_compound(struct svc_rqst *rqstp,
>>               if (op->status)
>>                       goto encode_op;
>>
>> -             if (opdesc->op_func)
>> +             if (opdesc->op_func) {
>> +                     if (opdesc->op_get_currentstateid)
>> +                             opdesc->op_get_currentstateid(cstate, &op->u);
>>                       op->status = opdesc->op_func(rqstp, cstate, &op->u);
>> -             else
>> +             } else
>>                       BUG_ON(op->status == nfs_ok);
>>
>> -             if (!op->status && need_wrongsec_check(rqstp))
>> -                     op->status = check_nfsd_access(cstate->current_fh.fh_export, rqstp);
>> +             if (!op->status) {
>> +                     if (opdesc->op_set_currentstateid) {
>> +                             opdesc->op_set_currentstateid(cstate, &op->u);
>> +                     }
>> +
>> +                     if (opdesc->op_clear_currentstateid)
>> +                             opdesc->op_clear_currentstateid(cstate);
>> +
>> +                     if (need_wrongsec_check(rqstp))
>> +                             op->status = check_nfsd_access(cstate->current_fh.fh_export, rqstp);
>> +             }
>>
>>  encode_op:
>>               /* Only from SEQUENCE */
>> @@ -1414,6 +1436,8 @@ static struct nfsd4_operation nfsd4_ops[] = {
>>               .op_flags = OP_MODIFIES_SOMETHING,
>>               .op_name = "OP_CLOSE",
>>               .op_rsize_bop = (nfsd4op_rsize)nfsd4_status_stateid_rsize,
>> +             .op_get_currentstateid = (stateid_getter)nfsd4_get_closestateid,
>> +             .op_set_currentstateid = (stateid_setter)nfsd4_set_closestateid,
>>       },
>>       [OP_COMMIT] = {
>>               .op_func = (nfsd4op_func)nfsd4_commit,
>> @@ -1484,6 +1508,7 @@ static struct nfsd4_operation nfsd4_ops[] = {
>>               .op_flags = OP_HANDLES_WRONGSEC | OP_MODIFIES_SOMETHING,
>>               .op_name = "OP_OPEN",
>>               .op_rsize_bop = (nfsd4op_rsize)nfsd4_open_rsize,
>> +             .op_set_currentstateid = (stateid_setter)nfsd4_set_openstateid,
>>       },
>>       [OP_OPEN_CONFIRM] = {
>>               .op_func = (nfsd4op_func)nfsd4_open_confirm,
>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>> index 47e94e3..594b44e 100644
>> --- a/fs/nfsd/nfs4state.c
>> +++ b/fs/nfsd/nfs4state.c
>> @@ -51,10 +51,12 @@ time_t nfsd4_grace = 90;
>>  static time_t boot_time;
>>  static stateid_t zerostateid;             /* bits all 0 */
>>  static stateid_t onestateid;              /* bits all 1 */
>> +static stateid_t currentstateid;         /* other all 0, seqid 1 */
>>  static u64 current_sessionid = 1;
>>
>>  #define ZERO_STATEID(stateid) (!memcmp((stateid), &zerostateid, sizeof(stateid_t)))
>>  #define ONE_STATEID(stateid)  (!memcmp((stateid), &onestateid, sizeof(stateid_t)))
>> +#define CURRENT_STATEID(stateid) (!memcmp((stateid), &currentstateid, sizeof(stateid_t)))
>>
>>  /* forward declarations */
>>  static int check_for_locks(struct nfs4_file *filp, struct nfs4_lockowner *lowner);
>> @@ -4423,6 +4425,8 @@ nfs4_state_init(void)
>>               INIT_LIST_HEAD(&lock_ownerstr_hashtbl[i]);
>>       }
>>       memset(&onestateid, ~0, sizeof(stateid_t));
>> +     /* seqid 1, other all 0 */
>> +     currentstateid.si_generation = 1;
>>       INIT_LIST_HEAD(&close_lru);
>>       INIT_LIST_HEAD(&client_lru);
>>       INIT_LIST_HEAD(&del_recall_lru);
>> @@ -4545,3 +4549,35 @@ nfs4_state_shutdown(void)
>>       nfs4_unlock_state();
>>       nfsd4_destroy_callback_queue();
>>  }
>> +
>> +static void
>> +get_stateid(struct nfsd4_compound_state *cstate, stateid_t *stateid)
>> +{
>> +     if (cstate->current_stateid && CURRENT_STATEID(stateid))
>> +             memcpy(stateid, cstate->current_stateid, sizeof(stateid_t));
>> +}
>> +
>> +static void
>> +put_stateid(struct nfsd4_compound_state *cstate, stateid_t *stateid)
>> +{
>> +     if (cstate->minorversion)
>> +             cstate->current_stateid = stateid;
>> +}
>> +
>> +void
>> +nfsd4_set_openstateid(struct nfsd4_compound_state *cstate, struct nfsd4_open *open)
>> +{
>> +     put_stateid(cstate, &open->op_stateid);
>> +}
>> +
>> +void
>> +nfsd4_get_closestateid(struct nfsd4_compound_state *cstate, struct nfsd4_close *close)
>> +{
>> +     get_stateid(cstate, &close->cl_stateid);
>> +}
>> +
>> +void
>> +nfsd4_set_closestateid(struct nfsd4_compound_state *cstate, struct nfsd4_close *close)
>> +{
>> +     get_stateid(cstate, &close->cl_stateid);
>> +}
>> diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
>> index 2364747..c1fe8ba 100644
>> --- a/fs/nfsd/xdr4.h
>> +++ b/fs/nfsd/xdr4.h
>> @@ -54,6 +54,7 @@ struct nfsd4_compound_state {
>>       size_t                  iovlen;
>>       u32                     minorversion;
>>       u32                     status;
>> +     const stateid_t *current_stateid;
>>  };
>>
>>  static inline bool nfsd4_has_session(struct nfsd4_compound_state *cs)
>> --
>> 1.7.7.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
>> the body of a message to [email protected]
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

2011-12-12 20:58:26

by Tigran Mkrtchyan

[permalink] [raw]
Subject: [PATH v4 2/3] nfsd41: handle current stateid on lock and locku

From: Tigran Mkrtchyan <[email protected]>

plus minor fixes

Conflicts:

fs/nfsd/nfs4proc.c

Signed-off-by: Tigran Mkrtchyan <[email protected]>
---
fs/nfsd/current_stateid.h | 11 ++++++++++-
fs/nfsd/nfs4proc.c | 2 ++
fs/nfsd/nfs4state.c | 22 ++++++++++++++++++++--
3 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/fs/nfsd/current_stateid.h b/fs/nfsd/current_stateid.h
index a83dd50..21550b6 100644
--- a/fs/nfsd/current_stateid.h
+++ b/fs/nfsd/current_stateid.h
@@ -4,8 +4,17 @@
#include "state.h"
#include "xdr4.h"

+/*
+ * functions to set current state id
+ */
extern void nfsd4_set_openstateid(struct nfsd4_compound_state *, struct nfsd4_open *);
-extern void nfsd4_get_closestateid(struct nfsd4_compound_state *, struct nfsd4_close *);
+extern void nfsd4_set_lockstateid(struct nfsd4_compound_state *, struct nfsd4_lock *);
extern void nfsd4_set_closestateid(struct nfsd4_compound_state *, struct nfsd4_close *);

+/*
+ * functions to consume current state id
+ */
+extern void nfsd4_get_closestateid(struct nfsd4_compound_state *, struct nfsd4_close *);
+extern void nfsd4_get_lockustateid(struct nfsd4_compound_state *, struct nfsd4_locku *);
+
#endif /* _NFSD4_CURRENT_STATE_H */
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 3dfb235..90c532b 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1478,6 +1478,7 @@ static struct nfsd4_operation nfsd4_ops[] = {
.op_flags = OP_MODIFIES_SOMETHING,
.op_name = "OP_LOCK",
.op_rsize_bop = (nfsd4op_rsize)nfsd4_lock_rsize,
+ .op_set_currentstateid = (stateid_setter)nfsd4_set_lockstateid,
},
[OP_LOCKT] = {
.op_func = (nfsd4op_func)nfsd4_lockt,
@@ -1488,6 +1489,7 @@ static struct nfsd4_operation nfsd4_ops[] = {
.op_flags = OP_MODIFIES_SOMETHING,
.op_name = "OP_LOCKU",
.op_rsize_bop = (nfsd4op_rsize)nfsd4_status_stateid_rsize,
+ .op_get_currentstateid = (stateid_getter)nfsd4_get_lockustateid,
},
[OP_LOOKUP] = {
.op_func = (nfsd4op_func)nfsd4_lookup,
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 594b44e..58410b2 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4564,6 +4564,9 @@ put_stateid(struct nfsd4_compound_state *cstate, stateid_t *stateid)
cstate->current_stateid = stateid;
}

+/*
+ * functions to set current state id
+ */
void
nfsd4_set_openstateid(struct nfsd4_compound_state *cstate, struct nfsd4_open *open)
{
@@ -4571,13 +4574,28 @@ nfsd4_set_openstateid(struct nfsd4_compound_state *cstate, struct nfsd4_open *op
}

void
+nfsd4_set_closestateid(struct nfsd4_compound_state *cstate, struct nfsd4_close *close)
+{
+ put_stateid(cstate, &close->cl_stateid);
+}
+
+void
+nfsd4_set_lockstateid(struct nfsd4_compound_state *cstate, struct nfsd4_lock *lock)
+{
+ put_stateid(cstate, &lock->lk_resp_stateid);
+}
+
+/*
+ * functions to consume current state id
+ */
+void
nfsd4_get_closestateid(struct nfsd4_compound_state *cstate, struct nfsd4_close *close)
{
get_stateid(cstate, &close->cl_stateid);
}

void
-nfsd4_set_closestateid(struct nfsd4_compound_state *cstate, struct nfsd4_close *close)
+nfsd4_get_lockustateid(struct nfsd4_compound_state *cstate, struct nfsd4_locku *locku)
{
- get_stateid(cstate, &close->cl_stateid);
+ get_stateid(cstate, &locku->lu_stateid);
}
--
1.7.7.4


2011-12-13 08:04:06

by Benny Halevy

[permalink] [raw]
Subject: Re: [PATH v4 1/3] nfsd41: handle current stateid in open and close

On 2011-12-12 23:00, Tigran Mkrtchyan wrote:
> From: Tigran Mkrtchyan <[email protected]>
>
>
> Signed-off-by: Tigran Mkrtchyan <[email protected]>
> ---
> fs/nfsd/current_stateid.h | 11 +++++++++++
> fs/nfsd/nfs4proc.c | 35 ++++++++++++++++++++++++++++++-----
> fs/nfsd/nfs4state.c | 36 ++++++++++++++++++++++++++++++++++++
> fs/nfsd/xdr4.h | 1 +
> 4 files changed, 78 insertions(+), 5 deletions(-)
> create mode 100644 fs/nfsd/current_stateid.h
>
> diff --git a/fs/nfsd/current_stateid.h b/fs/nfsd/current_stateid.h
> new file mode 100644
> index 0000000..a83dd50
> --- /dev/null
> +++ b/fs/nfsd/current_stateid.h
> @@ -0,0 +1,11 @@
> +#ifndef _NFSD4_CURRENT_STATE_H
> +#define _NFSD4_CURRENT_STATE_H
> +
> +#include "state.h"
> +#include "xdr4.h"
> +
> +extern void nfsd4_set_openstateid(struct nfsd4_compound_state *, struct nfsd4_open *);
> +extern void nfsd4_get_closestateid(struct nfsd4_compound_state *, struct nfsd4_close *);
> +extern void nfsd4_set_closestateid(struct nfsd4_compound_state *, struct nfsd4_close *);
> +
> +#endif /* _NFSD4_CURRENT_STATE_H */
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index fa38336..3dfb235 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -39,6 +39,7 @@
> #include "cache.h"
> #include "xdr4.h"
> #include "vfs.h"
> +#include "current_stateid.h"
>
> #define NFSDDBG_FACILITY NFSDDBG_PROC
>
> @@ -556,7 +557,6 @@ nfsd4_create(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> create->cr_name, create->cr_namelen,
> &create->cr_iattr, S_IFSOCK, 0, &resfh);
> break;
> -
> case NF4FIFO:
> status = nfsd_create(rqstp, &cstate->current_fh,
> create->cr_name, create->cr_namelen,
> @@ -1001,6 +1001,9 @@ static inline void nfsd4_increment_op_stats(u32 opnum)
> typedef __be32(*nfsd4op_func)(struct svc_rqst *, struct nfsd4_compound_state *,
> void *);
> typedef u32(*nfsd4op_rsize)(struct svc_rqst *, struct nfsd4_op *op);
> +typedef void(*stateid_setter)(struct nfsd4_compound_state *, void *);
> +typedef void(*stateid_getter)(struct nfsd4_compound_state *, void *);
> +typedef void(*stateid_cleaner)(struct nfsd4_compound_state *);

stateid_cleaner isn't used in this patch.
Mind moving it to the patch that uses it?

>
> enum nfsd4_op_flags {
> ALLOWED_WITHOUT_FH = 1 << 0, /* No current filehandle required */
> @@ -1034,6 +1037,9 @@ struct nfsd4_operation {
> char *op_name;
> /* Try to get response size before operation */
> nfsd4op_rsize op_rsize_bop;
> + stateid_setter op_get_currentstateid;
> + stateid_getter op_set_currentstateid;
> + stateid_cleaner op_clear_currentstateid;

ditto

> };
>
> static struct nfsd4_operation nfsd4_ops[];
> @@ -1116,6 +1122,11 @@ static bool need_wrongsec_check(struct svc_rqst *rqstp)
> return !(nextd->op_flags & OP_HANDLES_WRONGSEC);
> }
>
> +static void
> +nfsd4_clear_currentstateid(struct nfsd4_compound_state *cstate)
> +{
> + cstate->current_stateid = NULL;
> +}

ditto

> /*
> * COMPOUND call.
> */
> @@ -1216,13 +1227,24 @@ nfsd4_proc_compound(struct svc_rqst *rqstp,
> if (op->status)
> goto encode_op;
>
> - if (opdesc->op_func)
> + if (opdesc->op_func) {
> + if (opdesc->op_get_currentstateid)
> + opdesc->op_get_currentstateid(cstate, &op->u);
> op->status = opdesc->op_func(rqstp, cstate, &op->u);
> - else
> + } else
> BUG_ON(op->status == nfs_ok);
>
> - if (!op->status && need_wrongsec_check(rqstp))
> - op->status = check_nfsd_access(cstate->current_fh.fh_export, rqstp);
> + if (!op->status) {
> + if (opdesc->op_set_currentstateid) {
> + opdesc->op_set_currentstateid(cstate, &op->u);
> + }

not: no need for parenthesis here

> +
> + if (opdesc->op_clear_currentstateid)
> + opdesc->op_clear_currentstateid(cstate);
> +
> + if (need_wrongsec_check(rqstp))
> + op->status = check_nfsd_access(cstate->current_fh.fh_export, rqstp);
> + }
>
> encode_op:
> /* Only from SEQUENCE */
> @@ -1414,6 +1436,8 @@ static struct nfsd4_operation nfsd4_ops[] = {
> .op_flags = OP_MODIFIES_SOMETHING,
> .op_name = "OP_CLOSE",
> .op_rsize_bop = (nfsd4op_rsize)nfsd4_status_stateid_rsize,
> + .op_get_currentstateid = (stateid_getter)nfsd4_get_closestateid,
> + .op_set_currentstateid = (stateid_setter)nfsd4_set_closestateid,
> },
> [OP_COMMIT] = {
> .op_func = (nfsd4op_func)nfsd4_commit,
> @@ -1484,6 +1508,7 @@ static struct nfsd4_operation nfsd4_ops[] = {
> .op_flags = OP_HANDLES_WRONGSEC | OP_MODIFIES_SOMETHING,
> .op_name = "OP_OPEN",
> .op_rsize_bop = (nfsd4op_rsize)nfsd4_open_rsize,
> + .op_set_currentstateid = (stateid_setter)nfsd4_set_openstateid,
> },
> [OP_OPEN_CONFIRM] = {
> .op_func = (nfsd4op_func)nfsd4_open_confirm,
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 47e94e3..594b44e 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -51,10 +51,12 @@ time_t nfsd4_grace = 90;
> static time_t boot_time;
> static stateid_t zerostateid; /* bits all 0 */
> static stateid_t onestateid; /* bits all 1 */
> +static stateid_t currentstateid; /* other all 0, seqid 1 */

This can be statically initialized as per Bruce's patch:
http://marc.info/?l=linux-nfs&m=132372972624376&w=2

> static u64 current_sessionid = 1;
>
> #define ZERO_STATEID(stateid) (!memcmp((stateid), &zerostateid, sizeof(stateid_t)))
> #define ONE_STATEID(stateid) (!memcmp((stateid), &onestateid, sizeof(stateid_t)))
> +#define CURRENT_STATEID(stateid) (!memcmp((stateid), &currentstateid, sizeof(stateid_t)))
>
> /* forward declarations */
> static int check_for_locks(struct nfs4_file *filp, struct nfs4_lockowner *lowner);
> @@ -4423,6 +4425,8 @@ nfs4_state_init(void)
> INIT_LIST_HEAD(&lock_ownerstr_hashtbl[i]);
> }
> memset(&onestateid, ~0, sizeof(stateid_t));
> + /* seqid 1, other all 0 */
> + currentstateid.si_generation = 1;
> INIT_LIST_HEAD(&close_lru);
> INIT_LIST_HEAD(&client_lru);
> INIT_LIST_HEAD(&del_recall_lru);
> @@ -4545,3 +4549,35 @@ nfs4_state_shutdown(void)
> nfs4_unlock_state();
> nfsd4_destroy_callback_queue();
> }
> +
> +static void
> +get_stateid(struct nfsd4_compound_state *cstate, stateid_t *stateid)
> +{
> + if (cstate->current_stateid && CURRENT_STATEID(stateid))
> + memcpy(stateid, cstate->current_stateid, sizeof(stateid_t));

It'd be great if you could add a test case to use a the current_stateid
before it's established. E.g. send a compound with only CLOSE using
the current stateid. In this case the stateid will contain the current_stateid
value and the caller MUST get a NFS4ERR_BAD_STATEID error.

Thanks!

Benny

> +}
> +
> +static void
> +put_stateid(struct nfsd4_compound_state *cstate, stateid_t *stateid)
> +{
> + if (cstate->minorversion)
> + cstate->current_stateid = stateid;
> +}
> +
> +void
> +nfsd4_set_openstateid(struct nfsd4_compound_state *cstate, struct nfsd4_open *open)
> +{
> + put_stateid(cstate, &open->op_stateid);
> +}
> +
> +void
> +nfsd4_get_closestateid(struct nfsd4_compound_state *cstate, struct nfsd4_close *close)
> +{
> + get_stateid(cstate, &close->cl_stateid);
> +}
> +
> +void
> +nfsd4_set_closestateid(struct nfsd4_compound_state *cstate, struct nfsd4_close *close)
> +{
> + get_stateid(cstate, &close->cl_stateid);
> +}
> diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
> index 2364747..c1fe8ba 100644
> --- a/fs/nfsd/xdr4.h
> +++ b/fs/nfsd/xdr4.h
> @@ -54,6 +54,7 @@ struct nfsd4_compound_state {
> size_t iovlen;
> u32 minorversion;
> u32 status;
> + const stateid_t *current_stateid;
> };
>
> static inline bool nfsd4_has_session(struct nfsd4_compound_state *cs)

2011-12-13 16:44:07

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATH v4 1/3] nfsd41: handle current stateid in open and close

On Mon, Dec 12, 2011 at 10:00:25PM +0100, Tigran Mkrtchyan wrote:
> From: Tigran Mkrtchyan <[email protected]>
>
>
> Signed-off-by: Tigran Mkrtchyan <[email protected]>
> ---
> fs/nfsd/current_stateid.h | 11 +++++++++++
> fs/nfsd/nfs4proc.c | 35 ++++++++++++++++++++++++++++++-----
> fs/nfsd/nfs4state.c | 36 ++++++++++++++++++++++++++++++++++++
> fs/nfsd/xdr4.h | 1 +
> 4 files changed, 78 insertions(+), 5 deletions(-)
> create mode 100644 fs/nfsd/current_stateid.h
>
> diff --git a/fs/nfsd/current_stateid.h b/fs/nfsd/current_stateid.h
> new file mode 100644
> index 0000000..a83dd50
> --- /dev/null
> +++ b/fs/nfsd/current_stateid.h
> @@ -0,0 +1,11 @@
> +#ifndef _NFSD4_CURRENT_STATE_H
> +#define _NFSD4_CURRENT_STATE_H
> +
> +#include "state.h"
> +#include "xdr4.h"
> +
> +extern void nfsd4_set_openstateid(struct nfsd4_compound_state *, struct nfsd4_open *);
> +extern void nfsd4_get_closestateid(struct nfsd4_compound_state *, struct nfsd4_close *);
> +extern void nfsd4_set_closestateid(struct nfsd4_compound_state *, struct nfsd4_close *);
> +
> +#endif /* _NFSD4_CURRENT_STATE_H */
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index fa38336..3dfb235 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -39,6 +39,7 @@
> #include "cache.h"
> #include "xdr4.h"
> #include "vfs.h"
> +#include "current_stateid.h"
>
> #define NFSDDBG_FACILITY NFSDDBG_PROC
>
> @@ -556,7 +557,6 @@ nfsd4_create(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> create->cr_name, create->cr_namelen,
> &create->cr_iattr, S_IFSOCK, 0, &resfh);
> break;
> -
> case NF4FIFO:
> status = nfsd_create(rqstp, &cstate->current_fh,
> create->cr_name, create->cr_namelen,
> @@ -1001,6 +1001,9 @@ static inline void nfsd4_increment_op_stats(u32 opnum)
> typedef __be32(*nfsd4op_func)(struct svc_rqst *, struct nfsd4_compound_state *,
> void *);
> typedef u32(*nfsd4op_rsize)(struct svc_rqst *, struct nfsd4_op *op);
> +typedef void(*stateid_setter)(struct nfsd4_compound_state *, void *);
> +typedef void(*stateid_getter)(struct nfsd4_compound_state *, void *);
> +typedef void(*stateid_cleaner)(struct nfsd4_compound_state *);
>
> enum nfsd4_op_flags {
> ALLOWED_WITHOUT_FH = 1 << 0, /* No current filehandle required */
> @@ -1034,6 +1037,9 @@ struct nfsd4_operation {
> char *op_name;
> /* Try to get response size before operation */
> nfsd4op_rsize op_rsize_bop;
> + stateid_setter op_get_currentstateid;
> + stateid_getter op_set_currentstateid;
> + stateid_cleaner op_clear_currentstateid;

Is op_clear_currentstateid ever going to have more than one
implementation?

If not, let's just make that one a flag and do

if (clear_currentstateid flag set on this op)
cstate->current_stateid = NULL;

--b.

> };
>
> static struct nfsd4_operation nfsd4_ops[];
> @@ -1116,6 +1122,11 @@ static bool need_wrongsec_check(struct svc_rqst *rqstp)
> return !(nextd->op_flags & OP_HANDLES_WRONGSEC);
> }
>
> +static void
> +nfsd4_clear_currentstateid(struct nfsd4_compound_state *cstate)
> +{
> + cstate->current_stateid = NULL;
> +}
> /*
> * COMPOUND call.
> */
> @@ -1216,13 +1227,24 @@ nfsd4_proc_compound(struct svc_rqst *rqstp,
> if (op->status)
> goto encode_op;
>
> - if (opdesc->op_func)
> + if (opdesc->op_func) {
> + if (opdesc->op_get_currentstateid)
> + opdesc->op_get_currentstateid(cstate, &op->u);
> op->status = opdesc->op_func(rqstp, cstate, &op->u);
> - else
> + } else
> BUG_ON(op->status == nfs_ok);
>
> - if (!op->status && need_wrongsec_check(rqstp))
> - op->status = check_nfsd_access(cstate->current_fh.fh_export, rqstp);
> + if (!op->status) {
> + if (opdesc->op_set_currentstateid) {
> + opdesc->op_set_currentstateid(cstate, &op->u);
> + }
> +
> + if (opdesc->op_clear_currentstateid)
> + opdesc->op_clear_currentstateid(cstate);
> +
> + if (need_wrongsec_check(rqstp))
> + op->status = check_nfsd_access(cstate->current_fh.fh_export, rqstp);
> + }
>
> encode_op:
> /* Only from SEQUENCE */
> @@ -1414,6 +1436,8 @@ static struct nfsd4_operation nfsd4_ops[] = {
> .op_flags = OP_MODIFIES_SOMETHING,
> .op_name = "OP_CLOSE",
> .op_rsize_bop = (nfsd4op_rsize)nfsd4_status_stateid_rsize,
> + .op_get_currentstateid = (stateid_getter)nfsd4_get_closestateid,
> + .op_set_currentstateid = (stateid_setter)nfsd4_set_closestateid,
> },
> [OP_COMMIT] = {
> .op_func = (nfsd4op_func)nfsd4_commit,
> @@ -1484,6 +1508,7 @@ static struct nfsd4_operation nfsd4_ops[] = {
> .op_flags = OP_HANDLES_WRONGSEC | OP_MODIFIES_SOMETHING,
> .op_name = "OP_OPEN",
> .op_rsize_bop = (nfsd4op_rsize)nfsd4_open_rsize,
> + .op_set_currentstateid = (stateid_setter)nfsd4_set_openstateid,
> },
> [OP_OPEN_CONFIRM] = {
> .op_func = (nfsd4op_func)nfsd4_open_confirm,
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 47e94e3..594b44e 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -51,10 +51,12 @@ time_t nfsd4_grace = 90;
> static time_t boot_time;
> static stateid_t zerostateid; /* bits all 0 */
> static stateid_t onestateid; /* bits all 1 */
> +static stateid_t currentstateid; /* other all 0, seqid 1 */
> static u64 current_sessionid = 1;
>
> #define ZERO_STATEID(stateid) (!memcmp((stateid), &zerostateid, sizeof(stateid_t)))
> #define ONE_STATEID(stateid) (!memcmp((stateid), &onestateid, sizeof(stateid_t)))
> +#define CURRENT_STATEID(stateid) (!memcmp((stateid), &currentstateid, sizeof(stateid_t)))
>
> /* forward declarations */
> static int check_for_locks(struct nfs4_file *filp, struct nfs4_lockowner *lowner);
> @@ -4423,6 +4425,8 @@ nfs4_state_init(void)
> INIT_LIST_HEAD(&lock_ownerstr_hashtbl[i]);
> }
> memset(&onestateid, ~0, sizeof(stateid_t));
> + /* seqid 1, other all 0 */
> + currentstateid.si_generation = 1;
> INIT_LIST_HEAD(&close_lru);
> INIT_LIST_HEAD(&client_lru);
> INIT_LIST_HEAD(&del_recall_lru);
> @@ -4545,3 +4549,35 @@ nfs4_state_shutdown(void)
> nfs4_unlock_state();
> nfsd4_destroy_callback_queue();
> }
> +
> +static void
> +get_stateid(struct nfsd4_compound_state *cstate, stateid_t *stateid)
> +{
> + if (cstate->current_stateid && CURRENT_STATEID(stateid))
> + memcpy(stateid, cstate->current_stateid, sizeof(stateid_t));
> +}
> +
> +static void
> +put_stateid(struct nfsd4_compound_state *cstate, stateid_t *stateid)
> +{
> + if (cstate->minorversion)
> + cstate->current_stateid = stateid;
> +}
> +
> +void
> +nfsd4_set_openstateid(struct nfsd4_compound_state *cstate, struct nfsd4_open *open)
> +{
> + put_stateid(cstate, &open->op_stateid);
> +}
> +
> +void
> +nfsd4_get_closestateid(struct nfsd4_compound_state *cstate, struct nfsd4_close *close)
> +{
> + get_stateid(cstate, &close->cl_stateid);
> +}
> +
> +void
> +nfsd4_set_closestateid(struct nfsd4_compound_state *cstate, struct nfsd4_close *close)
> +{
> + get_stateid(cstate, &close->cl_stateid);
> +}
> diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
> index 2364747..c1fe8ba 100644
> --- a/fs/nfsd/xdr4.h
> +++ b/fs/nfsd/xdr4.h
> @@ -54,6 +54,7 @@ struct nfsd4_compound_state {
> size_t iovlen;
> u32 minorversion;
> u32 status;
> + const stateid_t *current_stateid;
> };
>
> static inline bool nfsd4_has_session(struct nfsd4_compound_state *cs)
> --
> 1.7.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2011-12-12 23:07:10

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATH v4 0/3] nfsd41: current state id processing

On Mon, Dec 12, 2011 at 10:00:24PM +0100, Tigran Mkrtchyan wrote:
> From: Tigran Mkrtchyan <[email protected]>
>
>
> Yet another update (v4) of current stateid handling
>
> difference with v3:
>
> Patches re-organized to only add functionality, e.g. there
> are no patches fixing other patches. Operation flags are
> not used any more. Existence of a corresponding function
> pointer is enough.
>
>
> Let me know if you want to see updates less often.
>
> Tigran.
>
> currently tested to work in a single compound:
>
> OPEN+CLOSE
> LOCK+WRITE+LOCKU+CLOSE
>
> On the way (not tested yet):
> OPEN+LAYOUTGET
>
> TODO:
> dispose current stateid on operations which provide CFH, but do not provide stateid.

We also need to go through the list of operations that might set or use
current stateid and make sure we've got them all. On a quick glance, in
addition to the above, maybe these?:

COMMIT
DELETRETURN
LOCKT
OPEN_DOWNGRADE
READ
SAFEFH/RESTOREFH
SETATTR
FREE_STATEID
TEST_STATEID
WANT_DELEGATION (?? does it set current_stateid in the case
where it returns one?)

>
> Internals:
> 1. struct nfsd4_compound_state contains pointer to current stateid
> 2. during compound processing before operation execution current stateid copied into
> operations stateid if it's equal to corresponding value ( 0, 1).
> 3. after operation execution current stateid changed to:
> a) point to stateid of last operation
> or
> b) point to NULL, if operation is marked to do so.
>
> Probably all patches have to be squashed into a single one before merged
> as none of the changes makes sense without others.

I think it's probably OK to do operations (or groups of operations) at a
time, as you have been. Any operations that remain unfixed will
continue returning EXPIRED, as they have before, so we won't be making
things any worse.

In any case, just make sure the layout stuff is kept separate, as we
want to sort out the current stateid before merging pnfs.

--b.

>
> Tigran.
>
>
> Tigran Mkrtchyan (3):
> nfsd41: handle current stateid in open and close
> nfsd41: handle current stateid on lock and locku
> nfsd41: consume current stateid on read and write
>
> fs/nfsd/current_stateid.h | 22 +++++++++++++++
> fs/nfsd/nfs4proc.c | 39 +++++++++++++++++++++++---
> fs/nfsd/nfs4state.c | 66 +++++++++++++++++++++++++++++++++++++++++++++
> fs/nfsd/xdr4.h | 1 +
> 4 files changed, 123 insertions(+), 5 deletions(-)
> create mode 100644 fs/nfsd/current_stateid.h
>
> --
> 1.7.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2011-12-13 08:11:17

by Benny Halevy

[permalink] [raw]
Subject: Re: [PATH v4 3/3] nfsd41: consume current stateid on read and write

I think that this patch can be squashed with lock/locku and
all the other operations that use a similar, trivial
get/set mechanism as the change is pretty mechanical.

Benny

On 2011-12-12 23:00, Tigran Mkrtchyan wrote:
> From: Tigran Mkrtchyan <[email protected]>
>
>
> Signed-off-by: Tigran Mkrtchyan <[email protected]>
> ---
> fs/nfsd/current_stateid.h | 2 ++
> fs/nfsd/nfs4proc.c | 2 ++
> fs/nfsd/nfs4state.c | 12 ++++++++++++
> 3 files changed, 16 insertions(+), 0 deletions(-)
>
> diff --git a/fs/nfsd/current_stateid.h b/fs/nfsd/current_stateid.h
> index 21550b6..6e54d19 100644
> --- a/fs/nfsd/current_stateid.h
> +++ b/fs/nfsd/current_stateid.h
> @@ -16,5 +16,7 @@ extern void nfsd4_set_closestateid(struct nfsd4_compound_state *, struct nfsd4_c
> */
> extern void nfsd4_get_closestateid(struct nfsd4_compound_state *, struct nfsd4_close *);
> extern void nfsd4_get_lockustateid(struct nfsd4_compound_state *, struct nfsd4_locku *);
> +extern void nfsd4_get_readstateid(struct nfsd4_compound_state *, struct nfsd4_read *);
> +extern void nfsd4_get_writestateid(struct nfsd4_compound_state *, struct nfsd4_write *);
>
> #endif /* _NFSD4_CURRENT_STATE_H */
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 90c532b..1d2c8d5 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -1550,6 +1550,7 @@ static struct nfsd4_operation nfsd4_ops[] = {
> .op_flags = OP_MODIFIES_SOMETHING,
> .op_name = "OP_READ",
> .op_rsize_bop = (nfsd4op_rsize)nfsd4_read_rsize,
> + .op_get_currentstateid = (stateid_getter)nfsd4_get_readstateid,
> },
> [OP_READDIR] = {
> .op_func = (nfsd4op_func)nfsd4_readdir,
> @@ -1628,6 +1629,7 @@ static struct nfsd4_operation nfsd4_ops[] = {
> .op_flags = OP_MODIFIES_SOMETHING | OP_CACHEME,
> .op_name = "OP_WRITE",
> .op_rsize_bop = (nfsd4op_rsize)nfsd4_write_rsize,
> + .op_get_currentstateid = (stateid_getter)nfsd4_get_writestateid,
> },
> [OP_RELEASE_LOCKOWNER] = {
> .op_func = (nfsd4op_func)nfsd4_release_lockowner,
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 58410b2..50d7ad0 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -4599,3 +4599,15 @@ nfsd4_get_lockustateid(struct nfsd4_compound_state *cstate, struct nfsd4_locku *
> {
> get_stateid(cstate, &locku->lu_stateid);
> }
> +
> +void
> +nfsd4_get_readstateid(struct nfsd4_compound_state *cstate, struct nfsd4_read *read)
> +{
> + get_stateid(cstate, &read->rd_stateid);
> +}
> +
> +void
> +nfsd4_get_writestateid(struct nfsd4_compound_state *cstate, struct nfsd4_write *write)
> +{
> + get_stateid(cstate, &write->wr_stateid);
> +}

2011-12-13 08:17:12

by Mkrtchyan, Tigran

[permalink] [raw]
Subject: Re: [PATH v4 3/3] nfsd41: consume current stateid on read and write

On Tue, Dec 13, 2011 at 9:11 AM, Benny Halevy <[email protected]> wrote:
> I think that this patch can be squashed with lock/locku and
> all the other operations that use a similar, trivial
> get/set mechanism as the change is pretty mechanical.
>

Most of the changes now will be adding corresponding setters and getters.
The main work now is to write good set of tests.

Tigran.

> Benny
>
> On 2011-12-12 23:00, Tigran Mkrtchyan wrote:
>> From: Tigran Mkrtchyan <[email protected]>
>>
>>
>> Signed-off-by: Tigran Mkrtchyan <[email protected]>
>> ---
>>  fs/nfsd/current_stateid.h |    2 ++
>>  fs/nfsd/nfs4proc.c        |    2 ++
>>  fs/nfsd/nfs4state.c       |   12 ++++++++++++
>>  3 files changed, 16 insertions(+), 0 deletions(-)
>>
>> diff --git a/fs/nfsd/current_stateid.h b/fs/nfsd/current_stateid.h
>> index 21550b6..6e54d19 100644
>> --- a/fs/nfsd/current_stateid.h
>> +++ b/fs/nfsd/current_stateid.h
>> @@ -16,5 +16,7 @@ extern void nfsd4_set_closestateid(struct nfsd4_compound_state *, struct nfsd4_c
>>   */
>>  extern void nfsd4_get_closestateid(struct nfsd4_compound_state *, struct nfsd4_close *);
>>  extern void nfsd4_get_lockustateid(struct nfsd4_compound_state *, struct nfsd4_locku *);
>> +extern void nfsd4_get_readstateid(struct nfsd4_compound_state *, struct nfsd4_read *);
>> +extern void nfsd4_get_writestateid(struct nfsd4_compound_state *, struct nfsd4_write *);
>>
>>  #endif   /* _NFSD4_CURRENT_STATE_H */
>> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
>> index 90c532b..1d2c8d5 100644
>> --- a/fs/nfsd/nfs4proc.c
>> +++ b/fs/nfsd/nfs4proc.c
>> @@ -1550,6 +1550,7 @@ static struct nfsd4_operation nfsd4_ops[] = {
>>               .op_flags = OP_MODIFIES_SOMETHING,
>>               .op_name = "OP_READ",
>>               .op_rsize_bop = (nfsd4op_rsize)nfsd4_read_rsize,
>> +             .op_get_currentstateid = (stateid_getter)nfsd4_get_readstateid,
>>       },
>>       [OP_READDIR] = {
>>               .op_func = (nfsd4op_func)nfsd4_readdir,
>> @@ -1628,6 +1629,7 @@ static struct nfsd4_operation nfsd4_ops[] = {
>>               .op_flags = OP_MODIFIES_SOMETHING | OP_CACHEME,
>>               .op_name = "OP_WRITE",
>>               .op_rsize_bop = (nfsd4op_rsize)nfsd4_write_rsize,
>> +             .op_get_currentstateid = (stateid_getter)nfsd4_get_writestateid,
>>       },
>>       [OP_RELEASE_LOCKOWNER] = {
>>               .op_func = (nfsd4op_func)nfsd4_release_lockowner,
>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>> index 58410b2..50d7ad0 100644
>> --- a/fs/nfsd/nfs4state.c
>> +++ b/fs/nfsd/nfs4state.c
>> @@ -4599,3 +4599,15 @@ nfsd4_get_lockustateid(struct nfsd4_compound_state *cstate, struct nfsd4_locku *
>>  {
>>       get_stateid(cstate, &locku->lu_stateid);
>>  }
>> +
>> +void
>> +nfsd4_get_readstateid(struct nfsd4_compound_state *cstate, struct nfsd4_read *read)
>> +{
>> +     get_stateid(cstate, &read->rd_stateid);
>> +}
>> +
>> +void
>> +nfsd4_get_writestateid(struct nfsd4_compound_state *cstate, struct nfsd4_write *write)
>> +{
>> +     get_stateid(cstate, &write->wr_stateid);
>> +}
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html