From: Tigran Mkrtchyan <[email protected]>
Signed-off-by: Tigran Mkrtchyan <[email protected]>
---
fs/nfsd/current_stateid.h | 1 +
fs/nfsd/nfs4proc.c | 12 +++++++++---
fs/nfsd/nfs4state.c | 19 +++++++++++++++----
fs/nfsd/xdr4.h | 6 ++++--
4 files changed, 29 insertions(+), 9 deletions(-)
diff --git a/fs/nfsd/current_stateid.h b/fs/nfsd/current_stateid.h
index d8c9992..4123551 100644
--- a/fs/nfsd/current_stateid.h
+++ b/fs/nfsd/current_stateid.h
@@ -4,6 +4,7 @@
#include "state.h"
#include "xdr4.h"
+extern void clear_current_stateid(struct nfsd4_compound_state *cstate);
/*
* functions to set current state id
*/
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index dffc9bd..56c1977 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -453,7 +453,10 @@ 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;
+ if (cstate->has_ssid) {
+ memcpy(&cstate->current_stateid, &cstate->save_stateid, sizeof(stateid_t));
+ cstate->has_csid = true;
+ }
return nfs_ok;
}
@@ -465,7 +468,10 @@ 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;
+ if (cstate->has_csid) {
+ memcpy(&cstate->save_stateid, &cstate->current_stateid, sizeof(stateid_t));
+ cstate->has_ssid = true;
+ }
return nfs_ok;
}
@@ -1238,7 +1244,7 @@ nfsd4_proc_compound(struct svc_rqst *rqstp,
opdesc->op_set_currentstateid(cstate, &op->u);
if (opdesc->op_flags & OP_CLEAR_STATEID)
- cstate->current_stateid = NULL;
+ clear_current_stateid(cstate);
if (need_wrongsec_check(rqstp))
op->status = check_nfsd_access(cstate->current_fh.fh_export, rqstp);
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 9c5a239..c6d2980 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4699,15 +4699,26 @@ nfs4_state_shutdown(void)
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));
+ if (cstate->has_csid && 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;
+ if (cstate->minorversion) {
+ memcpy(&cstate->current_stateid, stateid, sizeof(stateid_t));
+ cstate->has_csid = true;
+ }
+}
+
+void
+clear_current_stateid(struct nfsd4_compound_state *cstate)
+{
+ if (cstate->has_csid) {
+ memcpy(&cstate->current_stateid, &zero_stateid, sizeof(stateid_t));
+ cstate->has_csid = false;
+ }
}
/*
diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
index 2ae378e..3692ba8 100644
--- a/fs/nfsd/xdr4.h
+++ b/fs/nfsd/xdr4.h
@@ -54,8 +54,10 @@ struct nfsd4_compound_state {
size_t iovlen;
u32 minorversion;
u32 status;
- const stateid_t *current_stateid;
- const stateid_t *save_stateid;
+ stateid_t current_stateid;
+ bool has_csid; /* true if compound has current state id */
+ stateid_t save_stateid;
+ bool has_ssid; /* true if compound has saved state id */
};
static inline bool nfsd4_has_session(struct nfsd4_compound_state *cs)
--
1.7.7.5
On Tue, Jan 24, 2012 at 3:06 PM, Benny Halevy <[email protected]> wrote:
> On 2012-01-24 00:23, Tigran Mkrtchyan wrote:
>> From: Tigran Mkrtchyan <[email protected]>
>>
>>
>> Signed-off-by: Tigran Mkrtchyan <[email protected]>
>> ---
>> fs/nfsd/current_stateid.h | 1 +
>> fs/nfsd/nfs4proc.c | 12 +++++++++---
>> fs/nfsd/nfs4state.c | 19 +++++++++++++++----
>> fs/nfsd/xdr4.h | 6 ++++--
>> 4 files changed, 29 insertions(+), 9 deletions(-)
>>
>> diff --git a/fs/nfsd/current_stateid.h b/fs/nfsd/current_stateid.h
>> index d8c9992..4123551 100644
>> --- a/fs/nfsd/current_stateid.h
>> +++ b/fs/nfsd/current_stateid.h
>> @@ -4,6 +4,7 @@
>> #include "state.h"
>> #include "xdr4.h"
>>
>> +extern void clear_current_stateid(struct nfsd4_compound_state *cstate);
>> /*
>> * functions to set current state id
>> */
>> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
>> index dffc9bd..56c1977 100644
>> --- a/fs/nfsd/nfs4proc.c
>> +++ b/fs/nfsd/nfs4proc.c
>> @@ -453,7 +453,10 @@ 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;
>> + if (cstate->has_ssid) {
>> + memcpy(&cstate->current_stateid, &cstate->save_stateid, sizeof(stateid_t));
>> + cstate->has_csid = true;
>> + }
>> return nfs_ok;
>> }
>>
>> @@ -465,7 +468,10 @@ 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;
>> + if (cstate->has_csid) {
>> + memcpy(&cstate->save_stateid, &cstate->current_stateid, sizeof(stateid_t));
>> + cstate->has_ssid = true;
>> + }
>> return nfs_ok;
>> }
>>
>> @@ -1238,7 +1244,7 @@ nfsd4_proc_compound(struct svc_rqst *rqstp,
>> opdesc->op_set_currentstateid(cstate, &op->u);
>>
>> if (opdesc->op_flags & OP_CLEAR_STATEID)
>> - cstate->current_stateid = NULL;
>> + clear_current_stateid(cstate);
>>
>> if (need_wrongsec_check(rqstp))
>> op->status = check_nfsd_access(cstate->current_fh.fh_export, rqstp);
>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>> index 9c5a239..c6d2980 100644
>> --- a/fs/nfsd/nfs4state.c
>> +++ b/fs/nfsd/nfs4state.c
>> @@ -4699,15 +4699,26 @@ nfs4_state_shutdown(void)
>> 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));
>> + if (cstate->has_csid && 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;
>> + if (cstate->minorversion) {
>> + memcpy(&cstate->current_stateid, stateid, sizeof(stateid_t));
>> + cstate->has_csid = true;
>> + }
>> +}
>> +
>> +void
>> +clear_current_stateid(struct nfsd4_compound_state *cstate)
>> +{
>> + if (cstate->has_csid) {
>> + memcpy(&cstate->current_stateid, &zero_stateid, sizeof(stateid_t));
>
> This shouldn't be needed as long as has_csid is set to false, right?
correct. I can change it as following:
if (!cstate->has_csid)
return;
memcpy(&cstate->current_stateid, &zero_stateid, sizeof(stateid_t));
cstate->has_csid = false;
or move it into nfsd4proc.c
>
>> + cstate->has_csid = false;
>> + }
>> }
>>
>> /*
>> diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
>> index 2ae378e..3692ba8 100644
>> --- a/fs/nfsd/xdr4.h
>> +++ b/fs/nfsd/xdr4.h
>> @@ -54,8 +54,10 @@ struct nfsd4_compound_state {
>> size_t iovlen;
>> u32 minorversion;
>> u32 status;
>> - const stateid_t *current_stateid;
>> - const stateid_t *save_stateid;
>> + stateid_t current_stateid;
>> + bool has_csid; /* true if compound has current state id */
>> + stateid_t save_stateid;
>> + bool has_ssid; /* true if compound has saved state id */
>
> This is a pretty big overhead for the status bits.
> I'm not sure what Bruce's stand but using single-bit bit fields or
> flags would be more space efficient...
Fine with me.
>
> Benny
>
>> };
>>
>> 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
On 2012-01-24 00:23, Tigran Mkrtchyan wrote:
> From: Tigran Mkrtchyan <[email protected]>
>
>
> Signed-off-by: Tigran Mkrtchyan <[email protected]>
> ---
> fs/nfsd/current_stateid.h | 1 +
> fs/nfsd/nfs4proc.c | 12 +++++++++---
> fs/nfsd/nfs4state.c | 19 +++++++++++++++----
> fs/nfsd/xdr4.h | 6 ++++--
> 4 files changed, 29 insertions(+), 9 deletions(-)
>
> diff --git a/fs/nfsd/current_stateid.h b/fs/nfsd/current_stateid.h
> index d8c9992..4123551 100644
> --- a/fs/nfsd/current_stateid.h
> +++ b/fs/nfsd/current_stateid.h
> @@ -4,6 +4,7 @@
> #include "state.h"
> #include "xdr4.h"
>
> +extern void clear_current_stateid(struct nfsd4_compound_state *cstate);
> /*
> * functions to set current state id
> */
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index dffc9bd..56c1977 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -453,7 +453,10 @@ 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;
> + if (cstate->has_ssid) {
> + memcpy(&cstate->current_stateid, &cstate->save_stateid, sizeof(stateid_t));
> + cstate->has_csid = true;
> + }
> return nfs_ok;
> }
>
> @@ -465,7 +468,10 @@ 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;
> + if (cstate->has_csid) {
> + memcpy(&cstate->save_stateid, &cstate->current_stateid, sizeof(stateid_t));
> + cstate->has_ssid = true;
> + }
> return nfs_ok;
> }
>
> @@ -1238,7 +1244,7 @@ nfsd4_proc_compound(struct svc_rqst *rqstp,
> opdesc->op_set_currentstateid(cstate, &op->u);
>
> if (opdesc->op_flags & OP_CLEAR_STATEID)
> - cstate->current_stateid = NULL;
> + clear_current_stateid(cstate);
>
> if (need_wrongsec_check(rqstp))
> op->status = check_nfsd_access(cstate->current_fh.fh_export, rqstp);
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 9c5a239..c6d2980 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -4699,15 +4699,26 @@ nfs4_state_shutdown(void)
> 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));
> + if (cstate->has_csid && 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;
> + if (cstate->minorversion) {
> + memcpy(&cstate->current_stateid, stateid, sizeof(stateid_t));
> + cstate->has_csid = true;
> + }
> +}
> +
> +void
> +clear_current_stateid(struct nfsd4_compound_state *cstate)
> +{
> + if (cstate->has_csid) {
> + memcpy(&cstate->current_stateid, &zero_stateid, sizeof(stateid_t));
This shouldn't be needed as long as has_csid is set to false, right?
> + cstate->has_csid = false;
> + }
> }
>
> /*
> diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
> index 2ae378e..3692ba8 100644
> --- a/fs/nfsd/xdr4.h
> +++ b/fs/nfsd/xdr4.h
> @@ -54,8 +54,10 @@ struct nfsd4_compound_state {
> size_t iovlen;
> u32 minorversion;
> u32 status;
> - const stateid_t *current_stateid;
> - const stateid_t *save_stateid;
> + stateid_t current_stateid;
> + bool has_csid; /* true if compound has current state id */
> + stateid_t save_stateid;
> + bool has_ssid; /* true if compound has saved state id */
This is a pretty big overhead for the status bits.
I'm not sure what Bruce's stand but using single-bit bit fields or
flags would be more space efficient...
Benny
> };
>
> static inline bool nfsd4_has_session(struct nfsd4_compound_state *cs)
On 2012-01-24 16:23, Tigran Mkrtchyan wrote:
> On Tue, Jan 24, 2012 at 3:06 PM, Benny Halevy <[email protected]> wrote:
>> On 2012-01-24 00:23, Tigran Mkrtchyan wrote:
>>> From: Tigran Mkrtchyan <[email protected]>
>>>
>>>
>>> Signed-off-by: Tigran Mkrtchyan <[email protected]>
>>> ---
>>> fs/nfsd/current_stateid.h | 1 +
>>> fs/nfsd/nfs4proc.c | 12 +++++++++---
>>> fs/nfsd/nfs4state.c | 19 +++++++++++++++----
>>> fs/nfsd/xdr4.h | 6 ++++--
>>> 4 files changed, 29 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/fs/nfsd/current_stateid.h b/fs/nfsd/current_stateid.h
>>> index d8c9992..4123551 100644
>>> --- a/fs/nfsd/current_stateid.h
>>> +++ b/fs/nfsd/current_stateid.h
>>> @@ -4,6 +4,7 @@
>>> #include "state.h"
>>> #include "xdr4.h"
>>>
>>> +extern void clear_current_stateid(struct nfsd4_compound_state *cstate);
>>> /*
>>> * functions to set current state id
>>> */
>>> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
>>> index dffc9bd..56c1977 100644
>>> --- a/fs/nfsd/nfs4proc.c
>>> +++ b/fs/nfsd/nfs4proc.c
>>> @@ -453,7 +453,10 @@ 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;
>>> + if (cstate->has_ssid) {
>>> + memcpy(&cstate->current_stateid, &cstate->save_stateid, sizeof(stateid_t));
>>> + cstate->has_csid = true;
>>> + }
>>> return nfs_ok;
>>> }
>>>
>>> @@ -465,7 +468,10 @@ 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;
>>> + if (cstate->has_csid) {
>>> + memcpy(&cstate->save_stateid, &cstate->current_stateid, sizeof(stateid_t));
>>> + cstate->has_ssid = true;
>>> + }
>>> return nfs_ok;
>>> }
>>>
>>> @@ -1238,7 +1244,7 @@ nfsd4_proc_compound(struct svc_rqst *rqstp,
>>> opdesc->op_set_currentstateid(cstate, &op->u);
>>>
>>> if (opdesc->op_flags & OP_CLEAR_STATEID)
>>> - cstate->current_stateid = NULL;
>>> + clear_current_stateid(cstate);
>>>
>>> if (need_wrongsec_check(rqstp))
>>> op->status = check_nfsd_access(cstate->current_fh.fh_export, rqstp);
>>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>>> index 9c5a239..c6d2980 100644
>>> --- a/fs/nfsd/nfs4state.c
>>> +++ b/fs/nfsd/nfs4state.c
>>> @@ -4699,15 +4699,26 @@ nfs4_state_shutdown(void)
>>> 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));
>>> + if (cstate->has_csid && 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;
>>> + if (cstate->minorversion) {
>>> + memcpy(&cstate->current_stateid, stateid, sizeof(stateid_t));
>>> + cstate->has_csid = true;
>>> + }
>>> +}
>>> +
>>> +void
>>> +clear_current_stateid(struct nfsd4_compound_state *cstate)
>>> +{
>>> + if (cstate->has_csid) {
>>> + memcpy(&cstate->current_stateid, &zero_stateid, sizeof(stateid_t));
>>
>> This shouldn't be needed as long as has_csid is set to false, right?
>
> correct. I can change it as following:
> if (!cstate->has_csid)
> return;
> memcpy(&cstate->current_stateid, &zero_stateid, sizeof(stateid_t));
> cstate->has_csid = false;
What I meant is if has_csid is set to false why do the memcpy at all?
cstate->current_stateid's contents should never be accessed when has_scid==false.
>
> or move it into nfsd4proc.c
>
>>
>>> + cstate->has_csid = false;
>>> + }
>>> }
>>>
>>> /*
>>> diff --git a/fs/nfsd/xdr4.h b/fs/nfsd/xdr4.h
>>> index 2ae378e..3692ba8 100644
>>> --- a/fs/nfsd/xdr4.h
>>> +++ b/fs/nfsd/xdr4.h
>>> @@ -54,8 +54,10 @@ struct nfsd4_compound_state {
>>> size_t iovlen;
>>> u32 minorversion;
>>> u32 status;
>>> - const stateid_t *current_stateid;
>>> - const stateid_t *save_stateid;
>>> + stateid_t current_stateid;
>>> + bool has_csid; /* true if compound has current state id */
>>> + stateid_t save_stateid;
>>> + bool has_ssid; /* true if compound has saved state id */
>>
>> This is a pretty big overhead for the status bits.
>> I'm not sure what Bruce's stand but using single-bit bit fields or
>> flags would be more space efficient...
>
> Fine with me.
Great. Thanks.
Benny
>
>>
>> Benny
>>
>>> };
>>>
>>> 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
> --
> 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