From: Tigran Mkrtchyan <[email protected]>
This is ready-to-go patch set. Rebased to Bruce's nfsd-next.
Implemented for:
OPEN
OPENDOWNGRADE
CLOSE
LOCK
LOCKU
DELEGRETURN
FREE_STATEID
SETATTR
READ
WRITE
SAVEFH
RESTOREFH
PUTFH
PUTROOTFH
PUTPUBFH
LOOKUP
CREATE
PNFS released patches will be submitted as soon as patches will be merged with Benny's tree.
I will clean-up and submit corresponding pynfs test later today or tomorrow.
Comments and suggestions are welcome
Tigran.
Tigran Mkrtchyan (9):
nfsd4: initialize current stateid at compile time
nfsd41: handle current stateid in open and close
nfsd41: handle current stateid on lock and locku
nfsd41: consume current stateid on read and write
nfsd41: mark PUTFH, PUTPUBFH and PUTROOTFH to clear current stateid
nfsd41: save and restore current stateid with current fh
nfsd41: mark LOOKUP, LOOKUPP and CREATE to invalidate current stateid
nfsd41: handle current stateid in SETATTR and FREE_STATEID
nfsd41: consume current stateid on DELEGRETURN and OPENDOWNGRADE
fs/nfsd/current_stateid.h | 27 ++++++++++++
fs/nfsd/nfs4proc.c | 55 ++++++++++++++++++++-----
fs/nfsd/nfs4state.c | 99 ++++++++++++++++++++++++++++++++++++++++++++-
fs/nfsd/xdr4.h | 2 +
4 files changed, 172 insertions(+), 11 deletions(-)
create mode 100644 fs/nfsd/current_stateid.h
--
1.7.7.5
-- kofemann
/** caffeinated mutations of the core personality */
On Tue, Jan 17, 2012 at 21:45, J. Bruce Fields <[email protected]> wrote:
> On Tue, Jan 17, 2012 at 09:14:47PM +0100, Tigran Mkrtchyan wrote:
>> On Tue, Jan 17, 2012 at 8:40 PM, J. Bruce Fields <[email protected]> wrote:
>> > On Sun, Jan 15, 2012 at 05:20:03PM +0100, Tigran Mkrtchyan wrote:
>> >> +static void
>> >> +put_stateid(struct nfsd4_compound_state *cstate, stateid_t *stateid)
>> >> +{
>> >> + if (cstate->minorversion)
>> >> + cstate->current_stateid = stateid;
>> >> +}
>> > ...
>> >> @@ -54,6 +54,7 @@ struct nfsd4_compound_state {
>> >> size_t iovlen;
>> >> u32 minorversion;
>> >> u32 status;
>> >> + const stateid_t *current_stateid;
>> >> };
>> >
>> > I'd be more comfortable with passing stateid's by value rather than by
>> > reference.
>> >
>> > Presumably you're only ever pointing into one of the argument or result
>> > structures which are currently guaranteed to be around for as long as
>> > the compound is processed.
>> >
>> > But it'd seem safer not to have to worry about the lifetime of the
>> > pointed-to data at all, and just copy the stateid--it's only 16 bytes.
>>
>> Sure, it's only 16 bytes. Nevertheless, there are no allocation or
>> de-allocation of it. It just pointing to an existing stateid.
>
> Well, it's conceivable some day that we could for example take more of a
> one-op-at-a-time approach to compound processing and free the arguments
> and results from the previous op once we're done processing and xdr
> encoding it, in which case continuing to refer to a stateid from the
> previous op would be unsafe.
>
> Or maybe some day there will be an operation that sets the current
> stateid without also returning it in an argument, in which case we could
> be tempted to take a pointer to a field in an object without being
> assured how long that object will be around.
>
> Likely? Maybe not. I'd still feel safer not having to think about it.
>
>> Should be safe to use pointer. And yes, it's just 16 bytes, but if we can
>> avoid to
>> copy them why not?
>>
>> Of course I can change it if you want to.
>
> I'd prefer that.
>
Ok. No Problem!
> --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
From: Tigran Mkrtchyan <[email protected]>
Signed-off-by: Tigran Mkrtchyan <[email protected]>
---
fs/nfsd/current_stateid.h | 3 +++
fs/nfsd/nfs4proc.c | 3 +++
fs/nfsd/nfs4state.c | 18 ++++++++++++++++++
3 files changed, 24 insertions(+), 0 deletions(-)
diff --git a/fs/nfsd/current_stateid.h b/fs/nfsd/current_stateid.h
index e1ae950..d8c9992 100644
--- a/fs/nfsd/current_stateid.h
+++ b/fs/nfsd/current_stateid.h
@@ -7,6 +7,7 @@
/*
* functions to set current state id
*/
+extern void nfsd4_set_opendowngradestateid(struct nfsd4_compound_state *cstate, struct nfsd4_open_downgrade *);
extern void nfsd4_set_openstateid(struct nfsd4_compound_state *, struct nfsd4_open *);
extern void nfsd4_set_lockstateid(struct nfsd4_compound_state *, struct nfsd4_lock *);
extern void nfsd4_set_closestateid(struct nfsd4_compound_state *, struct nfsd4_close *);
@@ -14,6 +15,8 @@ extern void nfsd4_set_closestateid(struct nfsd4_compound_state *, struct nfsd4_c
/*
* functions to consume current state id
*/
+extern void nfsd4_get_opendowngradestateid(struct nfsd4_compound_state *cstate, struct nfsd4_open_downgrade *);
+extern void nfsd4_get_delegreturnstateid(struct nfsd4_compound_state *, struct nfsd4_delegreturn *);
extern void nfsd4_get_freestateid(struct nfsd4_compound_state *, struct nfsd4_free_stateid *);
extern void nfsd4_get_setattrstateid(struct nfsd4_compound_state *, struct nfsd4_setattr *);
extern void nfsd4_get_closestateid(struct nfsd4_compound_state *, struct nfsd4_close *);
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 124934a..dffc9bd 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1454,6 +1454,7 @@ static struct nfsd4_operation nfsd4_ops[] = {
.op_flags = OP_MODIFIES_SOMETHING,
.op_name = "OP_DELEGRETURN",
.op_rsize_bop = nfsd4_only_status_rsize,
+ .op_get_currentstateid = (stateid_getter)nfsd4_get_delegreturnstateid,
},
[OP_GETATTR] = {
.op_func = (nfsd4op_func)nfsd4_getattr,
@@ -1521,6 +1522,8 @@ static struct nfsd4_operation nfsd4_ops[] = {
.op_flags = OP_MODIFIES_SOMETHING,
.op_name = "OP_OPEN_DOWNGRADE",
.op_rsize_bop = (nfsd4op_rsize)nfsd4_status_stateid_rsize,
+ .op_get_currentstateid = (stateid_getter)nfsd4_get_opendowngradestateid,
+ .op_set_currentstateid = (stateid_setter)nfsd4_set_opendowngradestateid,
},
[OP_PUTFH] = {
.op_func = (nfsd4op_func)nfsd4_putfh,
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 8214c0f..9c5a239 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4714,6 +4714,12 @@ put_stateid(struct nfsd4_compound_state *cstate, stateid_t *stateid)
* functions to set current state id
*/
void
+nfsd4_set_opendowngradestateid(struct nfsd4_compound_state *cstate, struct nfsd4_open_downgrade *odp)
+{
+ put_stateid(cstate, &odp->od_stateid);
+}
+
+void
nfsd4_set_openstateid(struct nfsd4_compound_state *cstate, struct nfsd4_open *open)
{
put_stateid(cstate, &open->op_stateid);
@@ -4736,6 +4742,18 @@ nfsd4_set_lockstateid(struct nfsd4_compound_state *cstate, struct nfsd4_lock *lo
*/
void
+nfsd4_get_opendowngradestateid(struct nfsd4_compound_state *cstate, struct nfsd4_open_downgrade *odp)
+{
+ get_stateid(cstate, &odp->od_stateid);
+}
+
+void
+nfsd4_get_delegreturnstateid(struct nfsd4_compound_state *cstate, struct nfsd4_delegreturn *drp)
+{
+ get_stateid(cstate, &drp->dr_stateid);
+}
+
+void
nfsd4_get_freestateid(struct nfsd4_compound_state *cstate, struct nfsd4_free_stateid *fsp)
{
get_stateid(cstate, &fsp->fr_stateid);
--
1.7.7.5
On Tue, Jan 17, 2012 at 09:14:47PM +0100, Tigran Mkrtchyan wrote:
> On Tue, Jan 17, 2012 at 8:40 PM, J. Bruce Fields <[email protected]> wrote:
> > On Sun, Jan 15, 2012 at 05:20:03PM +0100, Tigran Mkrtchyan wrote:
> >> +static void
> >> +put_stateid(struct nfsd4_compound_state *cstate, stateid_t *stateid)
> >> +{
> >> + if (cstate->minorversion)
> >> + cstate->current_stateid = stateid;
> >> +}
> > ...
> >> @@ -54,6 +54,7 @@ struct nfsd4_compound_state {
> >> size_t iovlen;
> >> u32 minorversion;
> >> u32 status;
> >> + const stateid_t *current_stateid;
> >> };
> >
> > I'd be more comfortable with passing stateid's by value rather than by
> > reference.
> >
> > Presumably you're only ever pointing into one of the argument or result
> > structures which are currently guaranteed to be around for as long as
> > the compound is processed.
> >
> > But it'd seem safer not to have to worry about the lifetime of the
> > pointed-to data at all, and just copy the stateid--it's only 16 bytes.
>
> Sure, it's only 16 bytes. Nevertheless, there are no allocation or
> de-allocation of it. It just pointing to an existing stateid.
Well, it's conceivable some day that we could for example take more of a
one-op-at-a-time approach to compound processing and free the arguments
and results from the previous op once we're done processing and xdr
encoding it, in which case continuing to refer to a stateid from the
previous op would be unsafe.
Or maybe some day there will be an operation that sets the current
stateid without also returning it in an argument, in which case we could
be tempted to take a pointer to a field in an object without being
assured how long that object will be around.
Likely? Maybe not. I'd still feel safer not having to think about it.
> Should be safe to use pointer. And yes, it's just 16 bytes, but if we can
> avoid to
> copy them why not?
>
> Of course I can change it if you want to.
I'd prefer that.
--b.
From: Tigran Mkrtchyan <[email protected]>
Signed-off-by: Tigran Mkrtchyan <[email protected]>
---
fs/nfsd/nfs4proc.c | 2 ++
fs/nfsd/xdr4.h | 1 +
2 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 66bd005..59d9b4b 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -453,6 +453,7 @@ nfsd4_restorefh(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
return nfserr_restorefh;
fh_dup2(&cstate->current_fh, &cstate->save_fh);
+ cstate->current_stateid = cstate->save_stateid;
return nfs_ok;
}
@@ -464,6 +465,7 @@ nfsd4_savefh(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
return nfserr_nofilehandle;
fh_dup2(&cstate->save_fh, &cstate->current_fh);
+ cstate->save_stateid = cstate->current_stateid;
return nfs_ok;
}
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index c1fe8ba..2ae378e 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -55,6 +55,7 @@ struct nfsd4_compound_state {
u32 minorversion;
u32 status;
const stateid_t *current_stateid;
+ const stateid_t *save_stateid;
};
static inline bool nfsd4_has_session(struct nfsd4_compound_state *cs)
--
1.7.7.5
From: Tigran Mkrtchyan <[email protected]>
Signed-off-by: Tigran Mkrtchyan <[email protected]>
---
fs/nfsd/nfs4proc.c | 9 ++++++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 9a61bbf..66bd005 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1523,21 +1523,24 @@ static struct nfsd4_operation nfsd4_ops[] = {
[OP_PUTFH] = {
.op_func = (nfsd4op_func)nfsd4_putfh,
.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_ON_ABSENT_FS
- | OP_IS_PUTFH_LIKE | OP_MODIFIES_SOMETHING,
+ | OP_IS_PUTFH_LIKE | OP_MODIFIES_SOMETHING
+ | OP_CLEAR_STATEID,
.op_name = "OP_PUTFH",
.op_rsize_bop = (nfsd4op_rsize)nfsd4_only_status_rsize,
},
[OP_PUTPUBFH] = {
.op_func = (nfsd4op_func)nfsd4_putrootfh,
.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_ON_ABSENT_FS
- | OP_IS_PUTFH_LIKE | OP_MODIFIES_SOMETHING,
+ | OP_IS_PUTFH_LIKE | OP_MODIFIES_SOMETHING
+ | OP_CLEAR_STATEID,
.op_name = "OP_PUTPUBFH",
.op_rsize_bop = (nfsd4op_rsize)nfsd4_only_status_rsize,
},
[OP_PUTROOTFH] = {
.op_func = (nfsd4op_func)nfsd4_putrootfh,
.op_flags = ALLOWED_WITHOUT_FH | ALLOWED_ON_ABSENT_FS
- | OP_IS_PUTFH_LIKE | OP_MODIFIES_SOMETHING,
+ | OP_IS_PUTFH_LIKE | OP_MODIFIES_SOMETHING
+ | OP_CLEAR_STATEID,
.op_name = "OP_PUTROOTFH",
.op_rsize_bop = (nfsd4op_rsize)nfsd4_only_status_rsize,
},
--
1.7.7.5
From: Tigran Mkrtchyan <[email protected]>
Signed-off-by: Tigran Mkrtchyan <[email protected]>
---
fs/nfsd/current_stateid.h | 11 ++++++++++-
fs/nfsd/nfs4proc.c | 2 ++
fs/nfsd/nfs4state.c | 22 ++++++++++++++++++++--
3 files changed, 32 insertions(+), 3 deletions(-)
diff --git a/fs/nfsd/current_stateid.h b/fs/nfsd/current_stateid.h
index a83dd50..21550b6 100644
--- a/fs/nfsd/current_stateid.h
+++ b/fs/nfsd/current_stateid.h
@@ -4,8 +4,17 @@
#include "state.h"
#include "xdr4.h"
+/*
+ * functions to set current state id
+ */
extern void nfsd4_set_openstateid(struct nfsd4_compound_state *, struct nfsd4_open *);
-extern void nfsd4_get_closestateid(struct nfsd4_compound_state *, struct nfsd4_close *);
+extern void nfsd4_set_lockstateid(struct nfsd4_compound_state *, struct nfsd4_lock *);
extern void nfsd4_set_closestateid(struct nfsd4_compound_state *, struct nfsd4_close *);
+/*
+ * functions to consume current state id
+ */
+extern void nfsd4_get_closestateid(struct nfsd4_compound_state *, struct nfsd4_close *);
+extern void nfsd4_get_lockustateid(struct nfsd4_compound_state *, struct nfsd4_locku *);
+
#endif /* _NFSD4_CURRENT_STATE_H */
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index a457551..f66a8b2 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1474,6 +1474,7 @@ static struct nfsd4_operation nfsd4_ops[] = {
.op_flags = OP_MODIFIES_SOMETHING,
.op_name = "OP_LOCK",
.op_rsize_bop = (nfsd4op_rsize)nfsd4_lock_rsize,
+ .op_set_currentstateid = (stateid_setter)nfsd4_set_lockstateid,
},
[OP_LOCKT] = {
.op_func = (nfsd4op_func)nfsd4_lockt,
@@ -1484,6 +1485,7 @@ static struct nfsd4_operation nfsd4_ops[] = {
.op_flags = OP_MODIFIES_SOMETHING,
.op_name = "OP_LOCKU",
.op_rsize_bop = (nfsd4op_rsize)nfsd4_status_stateid_rsize,
+ .op_get_currentstateid = (stateid_getter)nfsd4_get_lockustateid,
},
[OP_LOOKUP] = {
.op_func = (nfsd4op_func)nfsd4_lookup,
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 6322a59..5665061 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4710,6 +4710,9 @@ put_stateid(struct nfsd4_compound_state *cstate, stateid_t *stateid)
cstate->current_stateid = stateid;
}
+/*
+ * functions to set current state id
+ */
void
nfsd4_set_openstateid(struct nfsd4_compound_state *cstate, struct nfsd4_open *open)
{
@@ -4717,13 +4720,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.5
On Sun, Jan 15, 2012 at 05:20:03PM +0100, Tigran Mkrtchyan wrote:
> +static void
> +put_stateid(struct nfsd4_compound_state *cstate, stateid_t *stateid)
> +{
> + if (cstate->minorversion)
> + cstate->current_stateid = stateid;
> +}
...
> @@ -54,6 +54,7 @@ struct nfsd4_compound_state {
> size_t iovlen;
> u32 minorversion;
> u32 status;
> + const stateid_t *current_stateid;
> };
I'd be more comfortable with passing stateid's by value rather than by
reference.
Presumably you're only ever pointing into one of the argument or result
structures which are currently guaranteed to be around for as long as
the compound is processed.
But it'd seem safer not to have to worry about the lifetime of the
pointed-to data at all, and just copy the stateid--it's only 16 bytes.
--b.
From: Tigran Mkrtchyan <[email protected]>
Signed-off-by: Tigran Mkrtchyan <[email protected]>
---
fs/nfsd/current_stateid.h | 2 ++
fs/nfsd/nfs4proc.c | 2 ++
fs/nfsd/nfs4state.c | 12 ++++++++++++
3 files changed, 16 insertions(+), 0 deletions(-)
diff --git a/fs/nfsd/current_stateid.h b/fs/nfsd/current_stateid.h
index 21550b6..6e54d19 100644
--- a/fs/nfsd/current_stateid.h
+++ b/fs/nfsd/current_stateid.h
@@ -16,5 +16,7 @@ extern void nfsd4_set_closestateid(struct nfsd4_compound_state *, struct nfsd4_c
*/
extern void nfsd4_get_closestateid(struct nfsd4_compound_state *, struct nfsd4_close *);
extern void nfsd4_get_lockustateid(struct nfsd4_compound_state *, struct nfsd4_locku *);
+extern void nfsd4_get_readstateid(struct nfsd4_compound_state *, struct nfsd4_read *);
+extern void nfsd4_get_writestateid(struct nfsd4_compound_state *, struct nfsd4_write *);
#endif /* _NFSD4_CURRENT_STATE_H */
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index f66a8b2..9a61bbf 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1546,6 +1546,7 @@ static struct nfsd4_operation nfsd4_ops[] = {
.op_flags = OP_MODIFIES_SOMETHING,
.op_name = "OP_READ",
.op_rsize_bop = (nfsd4op_rsize)nfsd4_read_rsize,
+ .op_get_currentstateid = (stateid_getter)nfsd4_get_readstateid,
},
[OP_READDIR] = {
.op_func = (nfsd4op_func)nfsd4_readdir,
@@ -1624,6 +1625,7 @@ static struct nfsd4_operation nfsd4_ops[] = {
.op_flags = OP_MODIFIES_SOMETHING | OP_CACHEME,
.op_name = "OP_WRITE",
.op_rsize_bop = (nfsd4op_rsize)nfsd4_write_rsize,
+ .op_get_currentstateid = (stateid_getter)nfsd4_get_writestateid,
},
[OP_RELEASE_LOCKOWNER] = {
.op_func = (nfsd4op_func)nfsd4_release_lockowner,
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 5665061..1c97912 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4745,3 +4745,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.5
On Tue, Jan 17, 2012 at 02:40:54PM -0500, bfields wrote:
> On Sun, Jan 15, 2012 at 05:20:03PM +0100, Tigran Mkrtchyan wrote:
> > +static void
> > +put_stateid(struct nfsd4_compound_state *cstate, stateid_t *stateid)
> > +{
> > + if (cstate->minorversion)
> > + cstate->current_stateid = stateid;
> > +}
> ...
> > @@ -54,6 +54,7 @@ struct nfsd4_compound_state {
> > size_t iovlen;
> > u32 minorversion;
> > u32 status;
> > + const stateid_t *current_stateid;
> > };
>
> I'd be more comfortable with passing stateid's by value rather than by
> reference.
>
> Presumably you're only ever pointing into one of the argument or result
> structures which are currently guaranteed to be around for as long as
> the compound is processed.
>
> But it'd seem safer not to have to worry about the lifetime of the
> pointed-to data at all, and just copy the stateid--it's only 16 bytes.
Other than that, the patches look fine to me.
--b.
From: Tigran Mkrtchyan <[email protected]>
Signed-off-by: Tigran Mkrtchyan <[email protected]>
---
fs/nfsd/nfs4proc.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 59d9b4b..620abcc 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1445,7 +1445,7 @@ static struct nfsd4_operation nfsd4_ops[] = {
},
[OP_CREATE] = {
.op_func = (nfsd4op_func)nfsd4_create,
- .op_flags = OP_MODIFIES_SOMETHING | OP_CACHEME,
+ .op_flags = OP_MODIFIES_SOMETHING | OP_CACHEME | OP_CLEAR_STATEID,
.op_name = "OP_CREATE",
.op_rsize_bop = (nfsd4op_rsize)nfsd4_create_rsize,
},
@@ -1491,12 +1491,12 @@ static struct nfsd4_operation nfsd4_ops[] = {
},
[OP_LOOKUP] = {
.op_func = (nfsd4op_func)nfsd4_lookup,
- .op_flags = OP_HANDLES_WRONGSEC,
+ .op_flags = OP_HANDLES_WRONGSEC | OP_CLEAR_STATEID,
.op_name = "OP_LOOKUP",
},
[OP_LOOKUPP] = {
.op_func = (nfsd4op_func)nfsd4_lookupp,
- .op_flags = OP_HANDLES_WRONGSEC,
+ .op_flags = OP_HANDLES_WRONGSEC | OP_CLEAR_STATEID,
.op_name = "OP_LOOKUPP",
},
[OP_NVERIFY] = {
--
1.7.7.5
From: Tigran Mkrtchyan <[email protected]>
Signed-off-by: Tigran Mkrtchyan <[email protected]>
---
fs/nfsd/current_stateid.h | 2 ++
fs/nfsd/nfs4proc.c | 1 +
fs/nfsd/nfs4state.c | 13 +++++++++++++
3 files changed, 16 insertions(+), 0 deletions(-)
diff --git a/fs/nfsd/current_stateid.h b/fs/nfsd/current_stateid.h
index 6e54d19..e1ae950 100644
--- a/fs/nfsd/current_stateid.h
+++ b/fs/nfsd/current_stateid.h
@@ -14,6 +14,8 @@ extern void nfsd4_set_closestateid(struct nfsd4_compound_state *, struct nfsd4_c
/*
* functions to consume current state id
*/
+extern void nfsd4_get_freestateid(struct nfsd4_compound_state *, struct nfsd4_free_stateid *);
+extern void nfsd4_get_setattrstateid(struct nfsd4_compound_state *, struct nfsd4_setattr *);
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 *);
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 620abcc..124934a 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1606,6 +1606,7 @@ static struct nfsd4_operation nfsd4_ops[] = {
.op_name = "OP_SETATTR",
.op_flags = OP_MODIFIES_SOMETHING | OP_CACHEME,
.op_rsize_bop = (nfsd4op_rsize)nfsd4_setattr_rsize,
+ .op_get_currentstateid = (stateid_getter)nfsd4_get_setattrstateid,
},
[OP_SETCLIENTID] = {
.op_func = (nfsd4op_func)nfsd4_setclientid,
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 1c97912..8214c0f 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4734,6 +4734,19 @@ nfsd4_set_lockstateid(struct nfsd4_compound_state *cstate, struct nfsd4_lock *lo
/*
* functions to consume current state id
*/
+
+void
+nfsd4_get_freestateid(struct nfsd4_compound_state *cstate, struct nfsd4_free_stateid *fsp)
+{
+ get_stateid(cstate, &fsp->fr_stateid);
+}
+
+void
+nfsd4_get_setattrstateid(struct nfsd4_compound_state *cstate, struct nfsd4_setattr *setattr)
+{
+ get_stateid(cstate, &setattr->sa_stateid);
+}
+
void
nfsd4_get_closestateid(struct nfsd4_compound_state *cstate, struct nfsd4_close *close)
{
--
1.7.7.5
From: Tigran Mkrtchyan <[email protected]>
Signed-off-by: Tigran Mkrtchyan <[email protected]>
---
fs/nfsd/nfs4state.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 7355fe4..8ca5ed1 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -58,11 +58,15 @@ static const stateid_t one_stateid = {
static const stateid_t zero_stateid = {
/* all fields zero */
};
-
+static const stateid_t currentstateid = {
+ .si_generation = 1,
+};
+
static u64 current_sessionid = 1;
#define ZERO_STATEID(stateid) (!memcmp((stateid), &zero_stateid, sizeof(stateid_t)))
#define ONE_STATEID(stateid) (!memcmp((stateid), &one_stateid, 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);
--
1.7.7.5
From: Tigran Mkrtchyan <[email protected]>
Signed-off-by: Tigran Mkrtchyan <[email protected]>
---
fs/nfsd/current_stateid.h | 11 +++++++++++
fs/nfsd/nfs4proc.c | 30 ++++++++++++++++++++++++++----
fs/nfsd/nfs4state.c | 32 ++++++++++++++++++++++++++++++++
fs/nfsd/xdr4.h | 1 +
4 files changed, 70 insertions(+), 4 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 9415bc4..a457551 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
@@ -1000,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 */
@@ -1025,6 +1028,10 @@ enum nfsd4_op_flags {
* the v4.0 case).
*/
OP_CACHEME = 1 << 6,
+ /*
+ * These are ops which clear current state id.
+ */
+ OP_CLEAR_STATEID = 1 << 7,
};
struct nfsd4_operation {
@@ -1033,6 +1040,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[];
@@ -1215,13 +1224,23 @@ nfsd4_proc_compound(struct svc_rqst *rqstp,
if (op->status)
goto encode_op;
- if (opdesc->op_func)
+ if (opdesc->op_func) {
+ if (opdesc->op_get_currentstateid)
+ opdesc->op_get_currentstateid(cstate, &op->u);
op->status = opdesc->op_func(rqstp, cstate, &op->u);
- else
+ } else
BUG_ON(op->status == nfs_ok);
- if (!op->status && need_wrongsec_check(rqstp))
- op->status = check_nfsd_access(cstate->current_fh.fh_export, rqstp);
+ if (!op->status) {
+ if (opdesc->op_set_currentstateid)
+ opdesc->op_set_currentstateid(cstate, &op->u);
+
+ if (opdesc->op_flags & OP_CLEAR_STATEID)
+ cstate->current_stateid = NULL;
+
+ if (need_wrongsec_check(rqstp))
+ op->status = check_nfsd_access(cstate->current_fh.fh_export, rqstp);
+ }
encode_op:
/* Only from SEQUENCE */
@@ -1413,6 +1432,8 @@ static struct nfsd4_operation nfsd4_ops[] = {
.op_flags = OP_MODIFIES_SOMETHING,
.op_name = "OP_CLOSE",
.op_rsize_bop = (nfsd4op_rsize)nfsd4_status_stateid_rsize,
+ .op_get_currentstateid = (stateid_getter)nfsd4_get_closestateid,
+ .op_set_currentstateid = (stateid_setter)nfsd4_set_closestateid,
},
[OP_COMMIT] = {
.op_func = (nfsd4op_func)nfsd4_commit,
@@ -1483,6 +1504,7 @@ static struct nfsd4_operation nfsd4_ops[] = {
.op_flags = OP_HANDLES_WRONGSEC | OP_MODIFIES_SOMETHING,
.op_name = "OP_OPEN",
.op_rsize_bop = (nfsd4op_rsize)nfsd4_open_rsize,
+ .op_set_currentstateid = (stateid_setter)nfsd4_set_openstateid,
},
[OP_OPEN_CONFIRM] = {
.op_func = (nfsd4op_func)nfsd4_open_confirm,
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 8ca5ed1..6322a59 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4695,3 +4695,35 @@ nfs4_state_shutdown(void)
nfs4_unlock_state();
nfsd4_destroy_callback_queue();
}
+
+static void
+get_stateid(struct nfsd4_compound_state *cstate, stateid_t *stateid)
+{
+ if (cstate->current_stateid && CURRENT_STATEID(stateid))
+ memcpy(stateid, cstate->current_stateid, sizeof(stateid_t));
+}
+
+static void
+put_stateid(struct nfsd4_compound_state *cstate, stateid_t *stateid)
+{
+ if (cstate->minorversion)
+ cstate->current_stateid = stateid;
+}
+
+void
+nfsd4_set_openstateid(struct nfsd4_compound_state *cstate, struct nfsd4_open *open)
+{
+ put_stateid(cstate, &open->op_stateid);
+}
+
+void
+nfsd4_get_closestateid(struct nfsd4_compound_state *cstate, struct nfsd4_close *close)
+{
+ get_stateid(cstate, &close->cl_stateid);
+}
+
+void
+nfsd4_set_closestateid(struct nfsd4_compound_state *cstate, struct nfsd4_close *close)
+{
+ get_stateid(cstate, &close->cl_stateid);
+}
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index 2364747..c1fe8ba 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -54,6 +54,7 @@ struct nfsd4_compound_state {
size_t iovlen;
u32 minorversion;
u32 status;
+ const stateid_t *current_stateid;
};
static inline bool nfsd4_has_session(struct nfsd4_compound_state *cs)
--
1.7.7.5
On Tue, Jan 17, 2012 at 8:40 PM, J. Bruce Fields <[email protected]> wrote:
> On Sun, Jan 15, 2012 at 05:20:03PM +0100, Tigran Mkrtchyan wrote:
>> +static void
>> +put_stateid(struct nfsd4_compound_state *cstate, stateid_t *stateid)
>> +{
>> + if (cstate->minorversion)
>> + cstate->current_stateid = stateid;
>> +}
> ...
>> @@ -54,6 +54,7 @@ struct nfsd4_compound_state {
>> size_t iovlen;
>> u32 minorversion;
>> u32 status;
>> + const stateid_t *current_stateid;
>> };
>
> I'd be more comfortable with passing stateid's by value rather than by
> reference.
>
> Presumably you're only ever pointing into one of the argument or result
> structures which are currently guaranteed to be around for as long as
> the compound is processed.
>
> But it'd seem safer not to have to worry about the lifetime of the
> pointed-to data at all, and just copy the stateid--it's only 16 bytes.
Sure, it's only 16 bytes. Nevertheless, there are no allocation or
de-allocation of it. It just pointing to an existing stateid. Should be
safe to use pointer. And yes, it's just 16 bytes, but if we can avoid to
copy them why not?
Of course I can change it if you want to.
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