* [PATCH 2/2] NFSD: Re-initialize fh_post/pre_saved between two operations
@ 2014-03-26 9:12 Kinglong Mee
2014-03-28 21:19 ` J. Bruce Fields
0 siblings, 1 reply; 6+ messages in thread
From: Kinglong Mee @ 2014-03-26 9:12 UTC (permalink / raw)
To: bfields; +Cc: linux-nfs
Testing NFS4.0 by pynfs, I got some messeages as,
"nfsd: inode locked twice during operation."
When one compound RPC contains two or more SETATTR operation
for one filehandle,the second SETATTR will cause the message.
Because after the first SETATTR, nfsd will not call fh_put()
to release current filehandle, it means filehandle have unlocked
with fh_post_saved = 1.
The second SETATTR find fh_post_saved = 1, and printk the message.
This patch re-initialize fh_post/pre_saved between two operations.
Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
---
fs/nfsd/nfs4proc.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 550faf2..103d1ac 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1356,6 +1356,9 @@ nfsd4_proc_compound(struct svc_rqst *rqstp,
!(opdesc->op_flags & ALLOWED_ON_ABSENT_FS)) {
op->status = nfserr_moved;
goto encode_op;
+ } else {
+ current_fh->fh_post_saved = 0;
+ current_fh->fh_pre_saved = 0;
}
/* If op is non-idempotent */
--
1.8.5.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] NFSD: Re-initialize fh_post/pre_saved between two operations
2014-03-26 9:12 [PATCH 2/2] NFSD: Re-initialize fh_post/pre_saved between two operations Kinglong Mee
@ 2014-03-28 21:19 ` J. Bruce Fields
2014-03-28 22:41 ` Kinglong Mee
0 siblings, 1 reply; 6+ messages in thread
From: J. Bruce Fields @ 2014-03-28 21:19 UTC (permalink / raw)
To: Kinglong Mee; +Cc: linux-nfs
On Wed, Mar 26, 2014 at 05:12:09PM +0800, Kinglong Mee wrote:
> Testing NFS4.0 by pynfs, I got some messeages as,
> "nfsd: inode locked twice during operation."
Thanks for looking into this. I agree that we should clear fh_pre_saved
and fh_post_saved between compound ops.
This is kind of non-obvious, though, so I think it would be worth moving
these two assignments to a little helper function (how about
"fh_clear_wcc_data()" for a name?) and adding a comment with the
definition of the function, explaining why we need it.
> When one compound RPC contains two or more SETATTR operation
> for one filehandle,the second SETATTR will cause the message.
Also worth noting that this affects any op that locks the filehandle
(e.g. a compound with two LINK ops would probably trigger the same
warning.)
--b.
>
> Because after the first SETATTR, nfsd will not call fh_put()
> to release current filehandle, it means filehandle have unlocked
> with fh_post_saved = 1.
> The second SETATTR find fh_post_saved = 1, and printk the message.
>
> This patch re-initialize fh_post/pre_saved between two operations.
>
> Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
> ---
> fs/nfsd/nfs4proc.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 550faf2..103d1ac 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -1356,6 +1356,9 @@ nfsd4_proc_compound(struct svc_rqst *rqstp,
> !(opdesc->op_flags & ALLOWED_ON_ABSENT_FS)) {
> op->status = nfserr_moved;
> goto encode_op;
> + } else {
> + current_fh->fh_post_saved = 0;
> + current_fh->fh_pre_saved = 0;
> }
>
> /* If op is non-idempotent */
> --
> 1.8.5.3
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] NFSD: Re-initialize fh_post/pre_saved between two operations
2014-03-28 21:19 ` J. Bruce Fields
@ 2014-03-28 22:41 ` Kinglong Mee
2014-03-29 0:58 ` J. Bruce Fields
0 siblings, 1 reply; 6+ messages in thread
From: Kinglong Mee @ 2014-03-28 22:41 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: linux-nfs
于 2014/3/29 05:19, J. Bruce Fields 写道:
> On Wed, Mar 26, 2014 at 05:12:09PM +0800, Kinglong Mee wrote:
>> Testing NFS4.0 by pynfs, I got some messeages as,
>> "nfsd: inode locked twice during operation."
>
> Thanks for looking into this. I agree that we should clear fh_pre_saved
> and fh_post_saved between compound ops.
>
> This is kind of non-obvious, though, so I think it would be worth moving
> these two assignments to a little helper function (how about
> "fh_clear_wcc_data()" for a name?) and adding a comment with the
> definition of the function, explaining why we need it.
That's great.
Thanks for your advice.
>
>> When one compound RPC contains two or more SETATTR operation
>> for one filehandle,the second SETATTR will cause the message.
>
> Also worth noting that this affects any op that locks the filehandle
> (e.g. a compound with two LINK ops would probably trigger the same
> warning.)
Yes, that's right.
Thanks,
Kinglong Mee
>
> --b.
>
>>
>> Because after the first SETATTR, nfsd will not call fh_put()
>> to release current filehandle, it means filehandle have unlocked
>> with fh_post_saved = 1.
>> The second SETATTR find fh_post_saved = 1, and printk the message.
>>
>> This patch re-initialize fh_post/pre_saved between two operations.
>>
>> Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
>> ---
>> fs/nfsd/nfs4proc.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
>> index 550faf2..103d1ac 100644
>> --- a/fs/nfsd/nfs4proc.c
>> +++ b/fs/nfsd/nfs4proc.c
>> @@ -1356,6 +1356,9 @@ nfsd4_proc_compound(struct svc_rqst *rqstp,
>> !(opdesc->op_flags & ALLOWED_ON_ABSENT_FS)) {
>> op->status = nfserr_moved;
>> goto encode_op;
>> + } else {
>> + current_fh->fh_post_saved = 0;
>> + current_fh->fh_pre_saved = 0;
>> }
>>
>> /* If op is non-idempotent */
>> --
>> 1.8.5.3
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] NFSD: Re-initialize fh_post/pre_saved between two operations
2014-03-28 22:41 ` Kinglong Mee
@ 2014-03-29 0:58 ` J. Bruce Fields
2014-03-29 2:23 ` [PATCH v2] NFSD: Clear wcc data between compound ops Kinglong Mee
0 siblings, 1 reply; 6+ messages in thread
From: J. Bruce Fields @ 2014-03-29 0:58 UTC (permalink / raw)
To: Kinglong Mee; +Cc: linux-nfs
On Sat, Mar 29, 2014 at 06:41:54AM +0800, Kinglong Mee wrote:
>
>
> 于 2014/3/29 05:19, J. Bruce Fields 写道:
> >On Wed, Mar 26, 2014 at 05:12:09PM +0800, Kinglong Mee wrote:
> >>Testing NFS4.0 by pynfs, I got some messeages as,
> >>"nfsd: inode locked twice during operation."
> >
> >Thanks for looking into this. I agree that we should clear fh_pre_saved
> >and fh_post_saved between compound ops.
> >
> >This is kind of non-obvious, though, so I think it would be worth moving
> >these two assignments to a little helper function (how about
> >"fh_clear_wcc_data()" for a name?) and adding a comment with the
> >definition of the function, explaining why we need it.
>
> That's great.
> Thanks for your advice.
>
> >
> >>When one compound RPC contains two or more SETATTR operation
> >>for one filehandle,the second SETATTR will cause the message.
> >
> >Also worth noting that this affects any op that locks the filehandle
> >(e.g. a compound with two LINK ops would probably trigger the same
> >warning.)
>
> Yes, that's right.
OK, thanks, I'll assume you're sending a revised patch.
--b.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] NFSD: Clear wcc data between compound ops
2014-03-29 0:58 ` J. Bruce Fields
@ 2014-03-29 2:23 ` Kinglong Mee
2014-03-29 19:46 ` J. Bruce Fields
0 siblings, 1 reply; 6+ messages in thread
From: Kinglong Mee @ 2014-03-29 2:23 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: linux-nfs
Testing NFS4.0 by pynfs, I got some messeages as,
"nfsd: inode locked twice during operation."
When one compound RPC contains two or more ops that locks
the filehandle,the second op will cause the message.
As two SETATTR ops, after the first SETATTR, nfsd will not call
fh_put() to release current filehandle, it means filehandle have
unlocked with fh_post_saved = 1.
The second SETATTR find fh_post_saved = 1, and printk the message.
v2: introduce helper fh_clear_wcc().
Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
---
fs/nfsd/nfs4proc.c | 2 ++
fs/nfsd/nfsfh.h | 14 +++++++++++++-
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 550faf2..9849c61 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1358,6 +1358,8 @@ nfsd4_proc_compound(struct svc_rqst *rqstp,
goto encode_op;
}
+ fh_clear_wcc(current_fh);
+
/* If op is non-idempotent */
if (opdesc->op_flags & OP_MODIFIES_SOMETHING) {
plen = opdesc->op_rsize_bop(rqstp, op);
diff --git a/fs/nfsd/nfsfh.h b/fs/nfsd/nfsfh.h
index 4775bc4..ad67964 100644
--- a/fs/nfsd/nfsfh.h
+++ b/fs/nfsd/nfsfh.h
@@ -133,6 +133,17 @@ fh_init(struct svc_fh *fhp, int maxsize)
#ifdef CONFIG_NFSD_V3
/*
+ * The wcc data stored in current_fh should be cleared
+ * between compound ops.
+ */
+static inline void
+fh_clear_wcc(struct svc_fh *fhp)
+{
+ fhp->fh_post_saved = 0;
+ fhp->fh_pre_saved = 0;
+}
+
+/*
* Fill in the pre_op attr for the wcc data
*/
static inline void
@@ -152,7 +163,8 @@ fill_pre_wcc(struct svc_fh *fhp)
extern void fill_post_wcc(struct svc_fh *);
#else
-#define fill_pre_wcc(ignored)
+#define fh_clear_wcc(ignored)
+#define fill_pre_wcc(ignored)
#define fill_post_wcc(notused)
#endif /* CONFIG_NFSD_V3 */
--
1.9.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] NFSD: Clear wcc data between compound ops
2014-03-29 2:23 ` [PATCH v2] NFSD: Clear wcc data between compound ops Kinglong Mee
@ 2014-03-29 19:46 ` J. Bruce Fields
0 siblings, 0 replies; 6+ messages in thread
From: J. Bruce Fields @ 2014-03-29 19:46 UTC (permalink / raw)
To: Kinglong Mee; +Cc: linux-nfs
On Sat, Mar 29, 2014 at 10:23:47AM +0800, Kinglong Mee wrote:
> Testing NFS4.0 by pynfs, I got some messeages as,
> "nfsd: inode locked twice during operation."
>
> When one compound RPC contains two or more ops that locks
> the filehandle,the second op will cause the message.
>
> As two SETATTR ops, after the first SETATTR, nfsd will not call
> fh_put() to release current filehandle, it means filehandle have
> unlocked with fh_post_saved = 1.
> The second SETATTR find fh_post_saved = 1, and printk the message.
>
> v2: introduce helper fh_clear_wcc().
Thanks, applying.--b.
>
> Signed-off-by: Kinglong Mee <kinglongmee@gmail.com>
> ---
> fs/nfsd/nfs4proc.c | 2 ++
> fs/nfsd/nfsfh.h | 14 +++++++++++++-
> 2 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 550faf2..9849c61 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -1358,6 +1358,8 @@ nfsd4_proc_compound(struct svc_rqst *rqstp,
> goto encode_op;
> }
>
> + fh_clear_wcc(current_fh);
> +
> /* If op is non-idempotent */
> if (opdesc->op_flags & OP_MODIFIES_SOMETHING) {
> plen = opdesc->op_rsize_bop(rqstp, op);
> diff --git a/fs/nfsd/nfsfh.h b/fs/nfsd/nfsfh.h
> index 4775bc4..ad67964 100644
> --- a/fs/nfsd/nfsfh.h
> +++ b/fs/nfsd/nfsfh.h
> @@ -133,6 +133,17 @@ fh_init(struct svc_fh *fhp, int maxsize)
>
> #ifdef CONFIG_NFSD_V3
> /*
> + * The wcc data stored in current_fh should be cleared
> + * between compound ops.
> + */
> +static inline void
> +fh_clear_wcc(struct svc_fh *fhp)
> +{
> + fhp->fh_post_saved = 0;
> + fhp->fh_pre_saved = 0;
> +}
> +
> +/*
> * Fill in the pre_op attr for the wcc data
> */
> static inline void
> @@ -152,7 +163,8 @@ fill_pre_wcc(struct svc_fh *fhp)
>
> extern void fill_post_wcc(struct svc_fh *);
> #else
> -#define fill_pre_wcc(ignored)
> +#define fh_clear_wcc(ignored)
> +#define fill_pre_wcc(ignored)
> #define fill_post_wcc(notused)
> #endif /* CONFIG_NFSD_V3 */
>
> --
> 1.9.0
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-03-29 19:47 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-26 9:12 [PATCH 2/2] NFSD: Re-initialize fh_post/pre_saved between two operations Kinglong Mee
2014-03-28 21:19 ` J. Bruce Fields
2014-03-28 22:41 ` Kinglong Mee
2014-03-29 0:58 ` J. Bruce Fields
2014-03-29 2:23 ` [PATCH v2] NFSD: Clear wcc data between compound ops Kinglong Mee
2014-03-29 19:46 ` J. Bruce Fields
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).