| Message ID | 533299E9.6010806@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 > -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
? 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 >> > -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 */
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(+)