* [PATH v3 1/5] nfsd41: handle current stateid in open and close
2011-12-11 16:41 [PATH v3 0/5] nfsd41: current state id processing Tigran Mkrtchyan
@ 2011-12-11 16:41 ` Tigran Mkrtchyan
2011-12-12 15:21 ` Benny Halevy
2011-12-11 16:41 ` [PATH v3 2/5] nfsd41: handle current stateid on lock and locku Tigran Mkrtchyan
` (4 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Tigran Mkrtchyan @ 2011-12-11 16:41 UTC (permalink / raw)
To: linux-nfs; +Cc: Tigran Mkrtchyan
From: Tigran Mkrtchyan <kofemann@gmail.com>
Signed-off-by: Tigran Mkrtchyan <kofemann@gmail.com>
---
fs/nfsd/current_stateid.h | 11 ++++++++++
fs/nfsd/nfs4proc.c | 47 ++++++++++++++++++++++++++++++++++++++------
fs/nfsd/nfs4state.c | 36 ++++++++++++++++++++++++++++++++++
fs/nfsd/xdr4.h | 2 +
4 files changed, 89 insertions(+), 7 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..4b8d81a 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,8 @@ 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 *);
enum nfsd4_op_flags {
ALLOWED_WITHOUT_FH = 1 << 0, /* No current filehandle required */
@@ -1026,6 +1028,9 @@ enum nfsd4_op_flags {
* the v4.0 case).
*/
OP_CACHEME = 1 << 6,
+ CONSUMES_CURRENT_STATEID = 1 << 7,
+ PROVIDES_CURRENT_STATEID = 1 << 8,
+ CLEARS_CURRENT_STATEID = 1 << 9,
};
struct nfsd4_operation {
@@ -1034,6 +1039,8 @@ 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;
};
static struct nfsd4_operation nfsd4_ops[];
@@ -1116,6 +1123,14 @@ 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)
+{
+ if (cstate->has_stateid) {
+ memset(&cstate->current_stateid, 0, sizeof(stateid_t));
+ cstate->has_stateid = false;
+ }
+}
/*
* COMPOUND call.
*/
@@ -1196,6 +1211,9 @@ nfsd4_proc_compound(struct svc_rqst *rqstp,
opdesc = OPDESC(op);
+ BUG_ON(opdesc->op_flags & PROVIDES_CURRENT_STATEID &&
+ opdesc->op_flags & CLEARS_CURRENT_STATEID);
+
if (!cstate->current_fh.fh_dentry) {
if (!(opdesc->op_flags & ALLOWED_WITHOUT_FH)) {
op->status = nfserr_nofilehandle;
@@ -1216,13 +1234,25 @@ nfsd4_proc_compound(struct svc_rqst *rqstp,
if (op->status)
goto encode_op;
- if (opdesc->op_func)
+ if (opdesc->op_func) {
+ if (opdesc->op_flags & CONSUMES_CURRENT_STATEID)
+ 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_flags & PROVIDES_CURRENT_STATEID) {
+ opdesc->op_set_currentstateid(cstate, &op->u);
+ cstate->has_stateid = true;
+ }
+
+ if (opdesc->op_flags & CLEARS_CURRENT_STATEID)
+ nfsd4_clear_currentstateid(cstate);
+
+ if (need_wrongsec_check(rqstp))
+ op->status = check_nfsd_access(cstate->current_fh.fh_export, rqstp);
+ }
encode_op:
/* Only from SEQUENCE */
@@ -1411,9 +1441,11 @@ static struct nfsd4_operation nfsd4_ops[] = {
},
[OP_CLOSE] = {
.op_func = (nfsd4op_func)nfsd4_close,
- .op_flags = OP_MODIFIES_SOMETHING,
+ .op_flags = OP_MODIFIES_SOMETHING | CONSUMES_CURRENT_STATEID,
.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,
@@ -1481,9 +1513,10 @@ static struct nfsd4_operation nfsd4_ops[] = {
},
[OP_OPEN] = {
.op_func = (nfsd4op_func)nfsd4_open,
- .op_flags = OP_HANDLES_WRONGSEC | OP_MODIFIES_SOMETHING,
+ .op_flags = OP_HANDLES_WRONGSEC | OP_MODIFIES_SOMETHING | PROVIDES_CURRENT_STATEID,
.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..278e0af 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), ¤tstateid, 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->minorversion && 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)
+ memcpy(&cstate->current_stateid, stateid, sizeof(stateid_t));
+}
+
+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..b8435d20 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -54,6 +54,8 @@ struct nfsd4_compound_state {
size_t iovlen;
u32 minorversion;
u32 status;
+ stateid_t current_stateid;
+ bool has_stateid;
};
static inline bool nfsd4_has_session(struct nfsd4_compound_state *cs)
--
1.7.7.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATH v3 1/5] nfsd41: handle current stateid in open and close
2011-12-11 16:41 ` [PATH v3 1/5] nfsd41: handle current stateid in open and close Tigran Mkrtchyan
@ 2011-12-12 15:21 ` Benny Halevy
2011-12-12 16:04 ` J. Bruce Fields
2011-12-12 16:42 ` Tigran Mkrtchyan
0 siblings, 2 replies; 13+ messages in thread
From: Benny Halevy @ 2011-12-12 15:21 UTC (permalink / raw)
To: Tigran Mkrtchyan; +Cc: linux-nfs, Tigran Mkrtchyan
On 2011-12-11 18:41, Tigran Mkrtchyan wrote:
> From: Tigran Mkrtchyan <kofemann@gmail.com>
>
>
> Signed-off-by: Tigran Mkrtchyan <kofemann@gmail.com>
> ---
> fs/nfsd/current_stateid.h | 11 ++++++++++
> fs/nfsd/nfs4proc.c | 47 ++++++++++++++++++++++++++++++++++++++------
> fs/nfsd/nfs4state.c | 36 ++++++++++++++++++++++++++++++++++
> fs/nfsd/xdr4.h | 2 +
> 4 files changed, 89 insertions(+), 7 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..4b8d81a 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;
> -
nit: I assume this cosmetic hunk can be dropped :)
> case NF4FIFO:
> status = nfsd_create(rqstp, &cstate->current_fh,
> create->cr_name, create->cr_namelen,
> @@ -1001,6 +1001,8 @@ 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 *);
>
> enum nfsd4_op_flags {
> ALLOWED_WITHOUT_FH = 1 << 0, /* No current filehandle required */
> @@ -1026,6 +1028,9 @@ enum nfsd4_op_flags {
> * the v4.0 case).
> */
> OP_CACHEME = 1 << 6,
> + CONSUMES_CURRENT_STATEID = 1 << 7,
> + PROVIDES_CURRENT_STATEID = 1 << 8,
> + CLEARS_CURRENT_STATEID = 1 << 9,
> };
>
> struct nfsd4_operation {
> @@ -1034,6 +1039,8 @@ 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;
> };
>
> static struct nfsd4_operation nfsd4_ops[];
> @@ -1116,6 +1123,14 @@ 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)
> +{
> + if (cstate->has_stateid) {
> + memset(&cstate->current_stateid, 0, sizeof(stateid_t));
> + cstate->has_stateid = false;
> + }
I'm a bit confused. In PATCH 0/5 you wrote:
"struct nfsd4_compound_state contains pointer to current stateid"
maybe it changes later in another patch. I'll be patient and keep reading ;-)
Also, you use both a belt and suspenders.
Instead of has_stateid I think you can just use the value of the
invalid stateid and all zeroes is a valid, special stateid.
> +}
> /*
> * COMPOUND call.
> */
> @@ -1196,6 +1211,9 @@ nfsd4_proc_compound(struct svc_rqst *rqstp,
>
> opdesc = OPDESC(op);
>
> + BUG_ON(opdesc->op_flags & PROVIDES_CURRENT_STATEID &&
> + opdesc->op_flags & CLEARS_CURRENT_STATEID);
> +
If so, couldn't the op just use nfsd4_clear_currentstateid()
as its op_set_currentstateid?
> if (!cstate->current_fh.fh_dentry) {
> if (!(opdesc->op_flags & ALLOWED_WITHOUT_FH)) {
> op->status = nfserr_nofilehandle;
> @@ -1216,13 +1234,25 @@ nfsd4_proc_compound(struct svc_rqst *rqstp,
> if (op->status)
> goto encode_op;
>
> - if (opdesc->op_func)
> + if (opdesc->op_func) {
> + if (opdesc->op_flags & CONSUMES_CURRENT_STATEID)
> + 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_flags & PROVIDES_CURRENT_STATEID) {
> + opdesc->op_set_currentstateid(cstate, &op->u);
> + cstate->has_stateid = true;
> + }
> +
> + if (opdesc->op_flags & CLEARS_CURRENT_STATEID)
> + nfsd4_clear_currentstateid(cstate);
> +
> + if (need_wrongsec_check(rqstp))
> + op->status = check_nfsd_access(cstate->current_fh.fh_export, rqstp);
Hmmm, if check_nfsd_access returns with a failure there's no point
in setting the current_stateid, is it?
> + }
>
> encode_op:
> /* Only from SEQUENCE */
> @@ -1411,9 +1441,11 @@ static struct nfsd4_operation nfsd4_ops[] = {
> },
> [OP_CLOSE] = {
> .op_func = (nfsd4op_func)nfsd4_close,
> - .op_flags = OP_MODIFIES_SOMETHING,
> + .op_flags = OP_MODIFIES_SOMETHING | CONSUMES_CURRENT_STATEID,
Looks like this also requires PROVIDES_CURRENT_STATEID
(or just ditch the flag so there can't be disagreement between the two :)
> .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,
> @@ -1481,9 +1513,10 @@ static struct nfsd4_operation nfsd4_ops[] = {
> },
> [OP_OPEN] = {
> .op_func = (nfsd4op_func)nfsd4_open,
> - .op_flags = OP_HANDLES_WRONGSEC | OP_MODIFIES_SOMETHING,
> + .op_flags = OP_HANDLES_WRONGSEC | OP_MODIFIES_SOMETHING | PROVIDES_CURRENT_STATEID,
> .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..278e0af 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), ¤tstateid, 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->minorversion && 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)
> + memcpy(&cstate->current_stateid, stateid, sizeof(stateid_t));
you can also set has_stateid to true here...
(If you keep it. I'm not convinced yet it's really needed)
> +}
> +
> +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..b8435d20 100644
> --- a/fs/nfsd/xdr4.h
> +++ b/fs/nfsd/xdr4.h
> @@ -54,6 +54,8 @@ struct nfsd4_compound_state {
> size_t iovlen;
> u32 minorversion;
> u32 status;
> + stateid_t current_stateid;
> + bool has_stateid;
> };
>
> static inline bool nfsd4_has_session(struct nfsd4_compound_state *cs)
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATH v3 1/5] nfsd41: handle current stateid in open and close
2011-12-12 15:21 ` Benny Halevy
@ 2011-12-12 16:04 ` J. Bruce Fields
2011-12-12 16:42 ` Tigran Mkrtchyan
2011-12-12 16:42 ` Tigran Mkrtchyan
1 sibling, 1 reply; 13+ messages in thread
From: J. Bruce Fields @ 2011-12-12 16:04 UTC (permalink / raw)
To: Benny Halevy; +Cc: Tigran Mkrtchyan, linux-nfs, Tigran Mkrtchyan
On Mon, Dec 12, 2011 at 05:21:08PM +0200, Benny Halevy wrote:
> On 2011-12-11 18:41, Tigran Mkrtchyan wrote:
> > @@ -1411,9 +1441,11 @@ static struct nfsd4_operation nfsd4_ops[] = {
> > },
> > [OP_CLOSE] = {
> > .op_func = (nfsd4op_func)nfsd4_close,
> > - .op_flags = OP_MODIFIES_SOMETHING,
> > + .op_flags = OP_MODIFIES_SOMETHING | CONSUMES_CURRENT_STATEID,
>
> Looks like this also requires PROVIDES_CURRENT_STATEID
> (or just ditch the flag so there can't be disagreement between the two :)
Please, yes, let's just eliminate the redundant flags.
--b.
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATH v3 1/5] nfsd41: handle current stateid in open and close
2011-12-12 16:04 ` J. Bruce Fields
@ 2011-12-12 16:42 ` Tigran Mkrtchyan
0 siblings, 0 replies; 13+ messages in thread
From: Tigran Mkrtchyan @ 2011-12-12 16:42 UTC (permalink / raw)
To: J. Bruce Fields
Cc: Benny Halevy, Tigran Mkrtchyan, linux-nfs, Tigran Mkrtchyan
On Mon, Dec 12, 2011 at 5:04 PM, J. Bruce Fields <bfields@fieldses.org> wrote:
> On Mon, Dec 12, 2011 at 05:21:08PM +0200, Benny Halevy wrote:
>> On 2011-12-11 18:41, Tigran Mkrtchyan wrote:
>> > @@ -1411,9 +1441,11 @@ static struct nfsd4_operation nfsd4_ops[] = {
>> > },
>> > [OP_CLOSE] = {
>> > .op_func = (nfsd4op_func)nfsd4_close,
>> > - .op_flags = OP_MODIFIES_SOMETHING,
>> > + .op_flags = OP_MODIFIES_SOMETHING | CONSUMES_CURRENT_STATEID,
>>
>> Looks like this also requires PROVIDES_CURRENT_STATEID
>> (or just ditch the flag so there can't be disagreement between the two :)
>
> Please, yes, let's just eliminate the redundant flags.
Fine with me.
Tigran.
>
> --b.
> --
> 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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATH v3 1/5] nfsd41: handle current stateid in open and close
2011-12-12 15:21 ` Benny Halevy
2011-12-12 16:04 ` J. Bruce Fields
@ 2011-12-12 16:42 ` Tigran Mkrtchyan
1 sibling, 0 replies; 13+ messages in thread
From: Tigran Mkrtchyan @ 2011-12-12 16:42 UTC (permalink / raw)
To: Benny Halevy; +Cc: Tigran Mkrtchyan, linux-nfs, Tigran Mkrtchyan
On Mon, Dec 12, 2011 at 4:21 PM, Benny Halevy <bhalevy@tonian.com> wrote:
> On 2011-12-11 18:41, Tigran Mkrtchyan wrote:
>> From: Tigran Mkrtchyan <kofemann@gmail.com>
>>
>>
>> Signed-off-by: Tigran Mkrtchyan <kofemann@gmail.com>
>> ---
>> fs/nfsd/current_stateid.h | 11 ++++++++++
>> fs/nfsd/nfs4proc.c | 47 ++++++++++++++++++++++++++++++++++++++------
>> fs/nfsd/nfs4state.c | 36 ++++++++++++++++++++++++++++++++++
>> fs/nfsd/xdr4.h | 2 +
>> 4 files changed, 89 insertions(+), 7 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..4b8d81a 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;
>> -
>
> nit: I assume this cosmetic hunk can be dropped :)
>
>> case NF4FIFO:
>> status = nfsd_create(rqstp, &cstate->current_fh,
>> create->cr_name, create->cr_namelen,
>> @@ -1001,6 +1001,8 @@ 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 *);
>>
>> enum nfsd4_op_flags {
>> ALLOWED_WITHOUT_FH = 1 << 0, /* No current filehandle required */
>> @@ -1026,6 +1028,9 @@ enum nfsd4_op_flags {
>> * the v4.0 case).
>> */
>> OP_CACHEME = 1 << 6,
>> + CONSUMES_CURRENT_STATEID = 1 << 7,
>> + PROVIDES_CURRENT_STATEID = 1 << 8,
>> + CLEARS_CURRENT_STATEID = 1 << 9,
>> };
>>
>> struct nfsd4_operation {
>> @@ -1034,6 +1039,8 @@ 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;
>> };
>>
>> static struct nfsd4_operation nfsd4_ops[];
>> @@ -1116,6 +1123,14 @@ 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)
>> +{
>> + if (cstate->has_stateid) {
>> + memset(&cstate->current_stateid, 0, sizeof(stateid_t));
>> + cstate->has_stateid = false;
>> + }
>
> I'm a bit confused. In PATCH 0/5 you wrote:
> "struct nfsd4_compound_state contains pointer to current stateid"
> maybe it changes later in another patch. I'll be patient and keep reading ;-)
>
> Also, you use both a belt and suspenders.
> Instead of has_stateid I think you can just use the value of the
> invalid stateid and all zeroes is a valid, special stateid.
>
>> +}
>> /*
>> * COMPOUND call.
>> */
>> @@ -1196,6 +1211,9 @@ nfsd4_proc_compound(struct svc_rqst *rqstp,
>>
>> opdesc = OPDESC(op);
>>
>> + BUG_ON(opdesc->op_flags & PROVIDES_CURRENT_STATEID &&
>> + opdesc->op_flags & CLEARS_CURRENT_STATEID);
>> +
>
> If so, couldn't the op just use nfsd4_clear_currentstateid()
> as its op_set_currentstateid?
>
>> if (!cstate->current_fh.fh_dentry) {
>> if (!(opdesc->op_flags & ALLOWED_WITHOUT_FH)) {
>> op->status = nfserr_nofilehandle;
>> @@ -1216,13 +1234,25 @@ nfsd4_proc_compound(struct svc_rqst *rqstp,
>> if (op->status)
>> goto encode_op;
>>
>> - if (opdesc->op_func)
>> + if (opdesc->op_func) {
>> + if (opdesc->op_flags & CONSUMES_CURRENT_STATEID)
>> + 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_flags & PROVIDES_CURRENT_STATEID) {
>> + opdesc->op_set_currentstateid(cstate, &op->u);
>> + cstate->has_stateid = true;
>> + }
>> +
>> + if (opdesc->op_flags & CLEARS_CURRENT_STATEID)
>> + nfsd4_clear_currentstateid(cstate);
>> +
>> + if (need_wrongsec_check(rqstp))
>> + op->status = check_nfsd_access(cstate->current_fh.fh_export, rqstp);
>
> Hmmm, if check_nfsd_access returns with a failure there's no point
> in setting the current_stateid, is it?
>
>> + }
>>
>> encode_op:
>> /* Only from SEQUENCE */
>> @@ -1411,9 +1441,11 @@ static struct nfsd4_operation nfsd4_ops[] = {
>> },
>> [OP_CLOSE] = {
>> .op_func = (nfsd4op_func)nfsd4_close,
>> - .op_flags = OP_MODIFIES_SOMETHING,
>> + .op_flags = OP_MODIFIES_SOMETHING | CONSUMES_CURRENT_STATEID,
>
> Looks like this also requires PROVIDES_CURRENT_STATEID
> (or just ditch the flag so there can't be disagreement between the two :)
this is corrected in patch 2.
Tigran.
>
>> .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,
>> @@ -1481,9 +1513,10 @@ static struct nfsd4_operation nfsd4_ops[] = {
>> },
>> [OP_OPEN] = {
>> .op_func = (nfsd4op_func)nfsd4_open,
>> - .op_flags = OP_HANDLES_WRONGSEC | OP_MODIFIES_SOMETHING,
>> + .op_flags = OP_HANDLES_WRONGSEC | OP_MODIFIES_SOMETHING | PROVIDES_CURRENT_STATEID,
>> .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..278e0af 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), ¤tstateid, 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->minorversion && 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)
>> + memcpy(&cstate->current_stateid, stateid, sizeof(stateid_t));
>
> you can also set has_stateid to true here...
> (If you keep it. I'm not convinced yet it's really needed)
>
>> +}
>> +
>> +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..b8435d20 100644
>> --- a/fs/nfsd/xdr4.h
>> +++ b/fs/nfsd/xdr4.h
>> @@ -54,6 +54,8 @@ struct nfsd4_compound_state {
>> size_t iovlen;
>> u32 minorversion;
>> u32 status;
>> + stateid_t current_stateid;
>> + bool has_stateid;
>> };
>>
>> static inline bool nfsd4_has_session(struct nfsd4_compound_state *cs)
> --
> 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
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATH v3 2/5] nfsd41: handle current stateid on lock and locku
2011-12-11 16:41 [PATH v3 0/5] nfsd41: current state id processing Tigran Mkrtchyan
2011-12-11 16:41 ` [PATH v3 1/5] nfsd41: handle current stateid in open and close Tigran Mkrtchyan
@ 2011-12-11 16:41 ` Tigran Mkrtchyan
2011-12-11 16:41 ` [PATH v3 3/5] nfsd41: update operations's stateid iff current stateid is set Tigran Mkrtchyan
` (3 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Tigran Mkrtchyan @ 2011-12-11 16:41 UTC (permalink / raw)
To: linux-nfs; +Cc: Tigran Mkrtchyan
From: Tigran Mkrtchyan <kofemann@gmail.com>
plus minor fixes
Signed-off-by: Tigran Mkrtchyan <kofemann@gmail.com>
---
fs/nfsd/current_stateid.h | 11 ++++++++++-
fs/nfsd/nfs4proc.c | 8 +++++---
fs/nfsd/nfs4state.c | 22 ++++++++++++++++++++--
3 files changed, 35 insertions(+), 6 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 4b8d81a..70534a0 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1441,7 +1441,7 @@ static struct nfsd4_operation nfsd4_ops[] = {
},
[OP_CLOSE] = {
.op_func = (nfsd4op_func)nfsd4_close,
- .op_flags = OP_MODIFIES_SOMETHING | CONSUMES_CURRENT_STATEID,
+ .op_flags = OP_MODIFIES_SOMETHING | CONSUMES_CURRENT_STATEID | PROVIDES_CURRENT_STATEID,
.op_name = "OP_CLOSE",
.op_rsize_bop = (nfsd4op_rsize)nfsd4_status_stateid_rsize,
.op_get_currentstateid = (stateid_getter)nfsd4_get_closestateid,
@@ -1483,9 +1483,10 @@ static struct nfsd4_operation nfsd4_ops[] = {
},
[OP_LOCK] = {
.op_func = (nfsd4op_func)nfsd4_lock,
- .op_flags = OP_MODIFIES_SOMETHING,
+ .op_flags = OP_MODIFIES_SOMETHING | PROVIDES_CURRENT_STATEID,
.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,
@@ -1493,9 +1494,10 @@ static struct nfsd4_operation nfsd4_ops[] = {
},
[OP_LOCKU] = {
.op_func = (nfsd4op_func)nfsd4_locku,
- .op_flags = OP_MODIFIES_SOMETHING,
+ .op_flags = OP_MODIFIES_SOMETHING | CONSUMES_CURRENT_STATEID,
.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 278e0af..fd10283 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)
memcpy(&cstate->current_stateid, stateid, sizeof(stateid_t));
}
+/*
+ * 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
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATH v3 3/5] nfsd41: update operations's stateid iff current stateid is set
2011-12-11 16:41 [PATH v3 0/5] nfsd41: current state id processing Tigran Mkrtchyan
2011-12-11 16:41 ` [PATH v3 1/5] nfsd41: handle current stateid in open and close Tigran Mkrtchyan
2011-12-11 16:41 ` [PATH v3 2/5] nfsd41: handle current stateid on lock and locku Tigran Mkrtchyan
@ 2011-12-11 16:41 ` Tigran Mkrtchyan
2011-12-11 16:41 ` [PATH v3 4/5] nfsd41: consume current stateid on read and write Tigran Mkrtchyan
` (2 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Tigran Mkrtchyan @ 2011-12-11 16:41 UTC (permalink / raw)
To: linux-nfs; +Cc: Tigran Mkrtchyan
From: Tigran Mkrtchyan <kofemann@gmail.com>
Signed-off-by: Tigran Mkrtchyan <kofemann@gmail.com>
---
fs/nfsd/nfs4state.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index fd10283..25ac9e8 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4553,15 +4553,17 @@ nfs4_state_shutdown(void)
static void
get_stateid(struct nfsd4_compound_state *cstate, stateid_t *stateid)
{
- if (cstate->minorversion && CURRENT_STATEID(stateid))
+ if (cstate->has_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)
+ if (cstate->minorversion) {
memcpy(&cstate->current_stateid, stateid, sizeof(stateid_t));
+ cstate->has_stateid = true;
+ }
}
/*
--
1.7.7.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATH v3 4/5] nfsd41: consume current stateid on read and write
2011-12-11 16:41 [PATH v3 0/5] nfsd41: current state id processing Tigran Mkrtchyan
` (2 preceding siblings ...)
2011-12-11 16:41 ` [PATH v3 3/5] nfsd41: update operations's stateid iff current stateid is set Tigran Mkrtchyan
@ 2011-12-11 16:41 ` Tigran Mkrtchyan
2011-12-11 16:41 ` [PATH v3 5/5] nfsd41: use pinter to current stateid to avoid extra copy Tigran Mkrtchyan
2011-12-12 19:44 ` [PATH v3 0/5] nfsd41: current state id processing Benny Halevy
5 siblings, 0 replies; 13+ messages in thread
From: Tigran Mkrtchyan @ 2011-12-11 16:41 UTC (permalink / raw)
To: linux-nfs; +Cc: Tigran Mkrtchyan
From: Tigran Mkrtchyan <kofemann@gmail.com>
Signed-off-by: Tigran Mkrtchyan <kofemann@gmail.com>
---
fs/nfsd/current_stateid.h | 2 ++
fs/nfsd/nfs4proc.c | 6 ++++--
fs/nfsd/nfs4state.c | 12 ++++++++++++
3 files changed, 18 insertions(+), 2 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 70534a0..f2f7dfa 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1555,9 +1555,10 @@ static struct nfsd4_operation nfsd4_ops[] = {
},
[OP_READ] = {
.op_func = (nfsd4op_func)nfsd4_read,
- .op_flags = OP_MODIFIES_SOMETHING,
+ .op_flags = OP_MODIFIES_SOMETHING | CONSUMES_CURRENT_STATEID,
.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,
@@ -1633,9 +1634,10 @@ static struct nfsd4_operation nfsd4_ops[] = {
},
[OP_WRITE] = {
.op_func = (nfsd4op_func)nfsd4_write,
- .op_flags = OP_MODIFIES_SOMETHING | OP_CACHEME,
+ .op_flags = OP_MODIFIES_SOMETHING | OP_CACHEME | CONSUMES_CURRENT_STATEID,
.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 25ac9e8..d7b8f25 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4601,3 +4601,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
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATH v3 5/5] nfsd41: use pinter to current stateid to avoid extra copy
2011-12-11 16:41 [PATH v3 0/5] nfsd41: current state id processing Tigran Mkrtchyan
` (3 preceding siblings ...)
2011-12-11 16:41 ` [PATH v3 4/5] nfsd41: consume current stateid on read and write Tigran Mkrtchyan
@ 2011-12-11 16:41 ` Tigran Mkrtchyan
2011-12-12 15:55 ` Benny Halevy
2011-12-12 19:44 ` [PATH v3 0/5] nfsd41: current state id processing Benny Halevy
5 siblings, 1 reply; 13+ messages in thread
From: Tigran Mkrtchyan @ 2011-12-11 16:41 UTC (permalink / raw)
To: linux-nfs; +Cc: Tigran Mkrtchyan
From: Tigran Mkrtchyan <kofemann@gmail.com>
Signed-off-by: Tigran Mkrtchyan <kofemann@gmail.com>
---
fs/nfsd/nfs4proc.c | 6 +-----
fs/nfsd/nfs4state.c | 7 +++----
fs/nfsd/xdr4.h | 3 +--
3 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index f2f7dfa..b7d1a7b 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1126,10 +1126,7 @@ static bool need_wrongsec_check(struct svc_rqst *rqstp)
static void
nfsd4_clear_currentstateid(struct nfsd4_compound_state *cstate)
{
- if (cstate->has_stateid) {
- memset(&cstate->current_stateid, 0, sizeof(stateid_t));
- cstate->has_stateid = false;
- }
+ cstate->current_stateid = NULL;
}
/*
* COMPOUND call.
@@ -1244,7 +1241,6 @@ nfsd4_proc_compound(struct svc_rqst *rqstp,
if (!op->status) {
if (opdesc->op_flags & PROVIDES_CURRENT_STATEID) {
opdesc->op_set_currentstateid(cstate, &op->u);
- cstate->has_stateid = true;
}
if (opdesc->op_flags & CLEARS_CURRENT_STATEID)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index d7b8f25..be77cd3 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4553,16 +4553,15 @@ nfs4_state_shutdown(void)
static void
get_stateid(struct nfsd4_compound_state *cstate, stateid_t *stateid)
{
- if (cstate->has_stateid && CURRENT_STATEID(stateid))
- memcpy(stateid, &cstate->current_stateid, sizeof(stateid_t));
+ 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) {
- memcpy(&cstate->current_stateid, stateid, sizeof(stateid_t));
- cstate->has_stateid = true;
+ cstate->current_stateid = stateid;
}
}
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index b8435d20..0ad0846 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -54,8 +54,7 @@ struct nfsd4_compound_state {
size_t iovlen;
u32 minorversion;
u32 status;
- stateid_t current_stateid;
- bool has_stateid;
+ stateid_t *current_stateid;
};
static inline bool nfsd4_has_session(struct nfsd4_compound_state *cs)
--
1.7.7.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATH v3 5/5] nfsd41: use pinter to current stateid to avoid extra copy
2011-12-11 16:41 ` [PATH v3 5/5] nfsd41: use pinter to current stateid to avoid extra copy Tigran Mkrtchyan
@ 2011-12-12 15:55 ` Benny Halevy
2011-12-12 20:59 ` Tiramisu Mokka
0 siblings, 1 reply; 13+ messages in thread
From: Benny Halevy @ 2011-12-12 15:55 UTC (permalink / raw)
To: Tigran Mkrtchyan; +Cc: linux-nfs, Tigran Mkrtchyan
On 2011-12-11 18:41, Tigran Mkrtchyan wrote:
> From: Tigran Mkrtchyan <kofemann@gmail.com>
>
>
> Signed-off-by: Tigran Mkrtchyan <kofemann@gmail.com>
> ---
> fs/nfsd/nfs4proc.c | 6 +-----
> fs/nfsd/nfs4state.c | 7 +++----
> fs/nfsd/xdr4.h | 3 +--
> 3 files changed, 5 insertions(+), 11 deletions(-)
>
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index f2f7dfa..b7d1a7b 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -1126,10 +1126,7 @@ static bool need_wrongsec_check(struct svc_rqst *rqstp)
> static void
> nfsd4_clear_currentstateid(struct nfsd4_compound_state *cstate)
> {
> - if (cstate->has_stateid) {
> - memset(&cstate->current_stateid, 0, sizeof(stateid_t));
> - cstate->has_stateid = false;
> - }
> + cstate->current_stateid = NULL;
> }
> /*
> * COMPOUND call.
> @@ -1244,7 +1241,6 @@ nfsd4_proc_compound(struct svc_rqst *rqstp,
> if (!op->status) {
> if (opdesc->op_flags & PROVIDES_CURRENT_STATEID) {
> opdesc->op_set_currentstateid(cstate, &op->u);
> - cstate->has_stateid = true;
> }
>
> if (opdesc->op_flags & CLEARS_CURRENT_STATEID)
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index d7b8f25..be77cd3 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -4553,16 +4553,15 @@ nfs4_state_shutdown(void)
> static void
> get_stateid(struct nfsd4_compound_state *cstate, stateid_t *stateid)
> {
> - if (cstate->has_stateid && CURRENT_STATEID(stateid))
> - memcpy(stateid, &cstate->current_stateid, sizeof(stateid_t));
> + 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) {
> - memcpy(&cstate->current_stateid, stateid, sizeof(stateid_t));
> - cstate->has_stateid = true;
> + cstate->current_stateid = stateid;
> }
> }
>
> diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
> index b8435d20..0ad0846 100644
> --- a/fs/nfsd/xdr4.h
> +++ b/fs/nfsd/xdr4.h
> @@ -54,8 +54,7 @@ struct nfsd4_compound_state {
> size_t iovlen;
> u32 minorversion;
> u32 status;
> - stateid_t current_stateid;
> - bool has_stateid;
> + stateid_t *current_stateid;
Certainly looks neat. I'm all for squashing this part of the patchset,
or just all of it...
Better be defined as const stateid_t * so its contents would be read only
for its consumers.
Benny
> };
>
> static inline bool nfsd4_has_session(struct nfsd4_compound_state *cs)
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATH v3 5/5] nfsd41: use pinter to current stateid to avoid extra copy
2011-12-12 15:55 ` Benny Halevy
@ 2011-12-12 20:59 ` Tiramisu Mokka
0 siblings, 0 replies; 13+ messages in thread
From: Tiramisu Mokka @ 2011-12-12 20:59 UTC (permalink / raw)
To: Benny Halevy; +Cc: linux-nfs, Tigran Mkrtchyan
On Mon, Dec 12, 2011 at 16:55, Benny Halevy <bhalevy@tonian.com> wrote:
> On 2011-12-11 18:41, Tigran Mkrtchyan wrote:
>> From: Tigran Mkrtchyan <kofemann@gmail.com>
>>
>>
>> Signed-off-by: Tigran Mkrtchyan <kofemann@gmail.com>
>> ---
>> fs/nfsd/nfs4proc.c | 6 +-----
>> fs/nfsd/nfs4state.c | 7 +++----
>> fs/nfsd/xdr4.h | 3 +--
>> 3 files changed, 5 insertions(+), 11 deletions(-)
>>
>> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
>> index f2f7dfa..b7d1a7b 100644
>> --- a/fs/nfsd/nfs4proc.c
>> +++ b/fs/nfsd/nfs4proc.c
>> @@ -1126,10 +1126,7 @@ static bool need_wrongsec_check(struct svc_rqst *rqstp)
>> static void
>> nfsd4_clear_currentstateid(struct nfsd4_compound_state *cstate)
>> {
>> - if (cstate->has_stateid) {
>> - memset(&cstate->current_stateid, 0, sizeof(stateid_t));
>> - cstate->has_stateid = false;
>> - }
>> + cstate->current_stateid = NULL;
>> }
>> /*
>> * COMPOUND call.
>> @@ -1244,7 +1241,6 @@ nfsd4_proc_compound(struct svc_rqst *rqstp,
>> if (!op->status) {
>> if (opdesc->op_flags & PROVIDES_CURRENT_STATEID) {
>> opdesc->op_set_currentstateid(cstate, &op->u);
>> - cstate->has_stateid = true;
>> }
>>
>> if (opdesc->op_flags & CLEARS_CURRENT_STATEID)
>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>> index d7b8f25..be77cd3 100644
>> --- a/fs/nfsd/nfs4state.c
>> +++ b/fs/nfsd/nfs4state.c
>> @@ -4553,16 +4553,15 @@ nfs4_state_shutdown(void)
>> static void
>> get_stateid(struct nfsd4_compound_state *cstate, stateid_t *stateid)
>> {
>> - if (cstate->has_stateid && CURRENT_STATEID(stateid))
>> - memcpy(stateid, &cstate->current_stateid, sizeof(stateid_t));
>> + 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) {
>> - memcpy(&cstate->current_stateid, stateid, sizeof(stateid_t));
>> - cstate->has_stateid = true;
>> + cstate->current_stateid = stateid;
>> }
>> }
>>
>> diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
>> index b8435d20..0ad0846 100644
>> --- a/fs/nfsd/xdr4.h
>> +++ b/fs/nfsd/xdr4.h
>> @@ -54,8 +54,7 @@ struct nfsd4_compound_state {
>> size_t iovlen;
>> u32 minorversion;
>> u32 status;
>> - stateid_t current_stateid;
>> - bool has_stateid;
>> + stateid_t *current_stateid;
>
> Certainly looks neat. I'm all for squashing this part of the patchset,
> or just all of it...
>
> Better be defined as const stateid_t * so its contents would be read only
> for its consumers.
sounds good. Thanks.
Tigran.
>
> Benny
>
>> };
>>
>> static inline bool nfsd4_has_session(struct nfsd4_compound_state *cs)
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATH v3 0/5] nfsd41: current state id processing
2011-12-11 16:41 [PATH v3 0/5] nfsd41: current state id processing Tigran Mkrtchyan
` (4 preceding siblings ...)
2011-12-11 16:41 ` [PATH v3 5/5] nfsd41: use pinter to current stateid to avoid extra copy Tigran Mkrtchyan
@ 2011-12-12 19:44 ` Benny Halevy
5 siblings, 0 replies; 13+ messages in thread
From: Benny Halevy @ 2011-12-12 19:44 UTC (permalink / raw)
To: Tigran Mkrtchyan; +Cc: linux-nfs
On 2011-12-11 18:41, Tigran Mkrtchyan wrote:
> From: Tigran Mkrtchyan <kofemann@gmail.com>
>
> This is yet another update of current stateid handling
Thanks! It looks must better every iteration!
>
> 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. operations marked to PROVIDE, CONSUME or CLEAR current stateid.
Does the flag add anything beyond simply implementing
the respective method?
Is CLEAR just a specific form of PROVIDE?
[more comments in reply to the actual patches...]
Thanks,
Benny
> 3. during compound processing before operation execution current stateid copied into
> operations stateid if it's equal to corresponding value ( 0, 1).
> 4. 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 (5):
> nfsd41: handle current stateid in open and close
> nfsd41: handle current stateid on lock and locku
> nfsd41: update operations's stateid iff current stateid is set
> nfsd41: consume current stateid on read and write
> nfsd41: use pinter to current stateid to avoid extra copy
>
> fs/nfsd/current_stateid.h | 22 +++++++++++++++
> fs/nfsd/nfs4proc.c | 55 +++++++++++++++++++++++++++++-------
> fs/nfsd/nfs4state.c | 67 +++++++++++++++++++++++++++++++++++++++++++++
> fs/nfsd/xdr4.h | 1 +
> 4 files changed, 134 insertions(+), 11 deletions(-)
> create mode 100644 fs/nfsd/current_stateid.h
>
^ permalink raw reply [flat|nested] 13+ messages in thread