From: Tigran Mkrtchyan <[email protected]>
This is yet another update of current stateid handling
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.
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
--
1.7.7.4
From: Tigran Mkrtchyan <[email protected]>
plus minor fixes
Signed-off-by: Tigran Mkrtchyan <[email protected]>
---
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
On Mon, Dec 12, 2011 at 4:21 PM, Benny Halevy <[email protected]> wrote:
> On 2011-12-11 18:41, 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 | 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Tigran Mkrtchyan <[email protected]>
Signed-off-by: Tigran Mkrtchyan <[email protected]>
---
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
From: Tigran Mkrtchyan <[email protected]>
Signed-off-by: Tigran Mkrtchyan <[email protected]>
---
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
From: Tigran Mkrtchyan <[email protected]>
Signed-off-by: Tigran Mkrtchyan <[email protected]>
---
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
On Mon, Dec 12, 2011 at 16:55, Benny Halevy <[email protected]> wrote:
> On 2011-12-11 18:41, Tigran Mkrtchyan wrote:
>> From: Tigran Mkrtchyan <[email protected]>
>>
>>
>> Signed-off-by: Tigran Mkrtchyan <[email protected]>
>> ---
>> 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)
On Mon, Dec 12, 2011 at 5:04 PM, J. Bruce Fields <[email protected]> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2011-12-11 18:41, Tigran Mkrtchyan wrote:
> From: Tigran Mkrtchyan <[email protected]>
>
> 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
>
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.
From: Tigran Mkrtchyan <[email protected]>
Signed-off-by: Tigran Mkrtchyan <[email protected]>
---
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
On 2011-12-11 18:41, Tigran Mkrtchyan wrote:
> From: Tigran Mkrtchyan <[email protected]>
>
>
> Signed-off-by: Tigran Mkrtchyan <[email protected]>
> ---
> 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)
On 2011-12-11 18:41, 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 | 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)