Turns out doing mnt_want_write twice for the same process makes
lockdep unhappy, so move the fh_want_write down to after calling
vfs_truncate in nfsd_setattr. No changes to error handling required
as the want write state is automatically cleaned up by the caller
based on a flag in the svc_fh.
Fixes: 41f53350 ("nfsd: special case truncates some more")
Signed-off-by: Christoph Hellwig <[email protected]>
Reported-by: Dave Jones <[email protected]>
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index ca13236dbb1f..a974368026a1 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -359,11 +359,6 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp, struct iattr *iap,
err = fh_verify(rqstp, fhp, ftype, accmode);
if (err)
return err;
- if (get_write_count) {
- host_err = fh_want_write(fhp);
- if (host_err)
- goto out_host_err;
- }
dentry = fhp->fh_dentry;
inode = d_inode(dentry);
@@ -416,6 +411,12 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp, struct iattr *iap,
iap->ia_valid |= ATTR_CTIME;
+ if (get_write_count) {
+ host_err = fh_want_write(fhp);
+ if (host_err)
+ goto out_host_err;
+ }
+
fh_lock(fhp);
host_err = notify_change(dentry, iap, NULL);
fh_unlock(fhp);
On Wed, Feb 08, 2017 at 10:50:38PM +0100, Christoph Hellwig wrote:
> On Wed, Feb 08, 2017 at 04:45:38PM -0500, J. Bruce Fields wrote:
> > On Tue, Feb 07, 2017 at 10:12:44AM +0100, Christoph Hellwig wrote:
> > > Turns out doing mnt_want_write twice for the same process makes
> > > lockdep unhappy, so move the fh_want_write down to after calling
> > > vfs_truncate in nfsd_setattr. No changes to error handling required
> > > as the want write state is automatically cleaned up by the caller
> > > based on a flag in the svc_fh.
> > >
> > > Fixes: 41f53350 ("nfsd: special case truncates some more")
> > > Signed-off-by: Christoph Hellwig <[email protected]>
> > > Reported-by: Dave Jones <[email protected]>
> >
> > Thanks, I'll see if I can squeeze this into 4.10.--b.
>
> FYI, Chuck just reported another issue, let's wait for that for now.
OK.--b.
On Wed, Feb 08, 2017 at 04:45:38PM -0500, J. Bruce Fields wrote:
> On Tue, Feb 07, 2017 at 10:12:44AM +0100, Christoph Hellwig wrote:
> > Turns out doing mnt_want_write twice for the same process makes
> > lockdep unhappy, so move the fh_want_write down to after calling
> > vfs_truncate in nfsd_setattr. No changes to error handling required
> > as the want write state is automatically cleaned up by the caller
> > based on a flag in the svc_fh.
> >
> > Fixes: 41f53350 ("nfsd: special case truncates some more")
> > Signed-off-by: Christoph Hellwig <[email protected]>
> > Reported-by: Dave Jones <[email protected]>
>
> Thanks, I'll see if I can squeeze this into 4.10.--b.
FYI, Chuck just reported another issue, let's wait for that for now.
On Tue, Feb 07, 2017 at 10:12:44AM +0100, Christoph Hellwig wrote:
> Turns out doing mnt_want_write twice for the same process makes
> lockdep unhappy, so move the fh_want_write down to after calling
> vfs_truncate in nfsd_setattr. No changes to error handling required
> as the want write state is automatically cleaned up by the caller
> based on a flag in the svc_fh.
>
> Fixes: 41f53350 ("nfsd: special case truncates some more")
> Signed-off-by: Christoph Hellwig <[email protected]>
> Reported-by: Dave Jones <[email protected]>
Thanks, I'll see if I can squeeze this into 4.10.--b.
>
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index ca13236dbb1f..a974368026a1 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -359,11 +359,6 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp, struct iattr *iap,
> err = fh_verify(rqstp, fhp, ftype, accmode);
> if (err)
> return err;
> - if (get_write_count) {
> - host_err = fh_want_write(fhp);
> - if (host_err)
> - goto out_host_err;
> - }
>
> dentry = fhp->fh_dentry;
> inode = d_inode(dentry);
> @@ -416,6 +411,12 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp, struct iattr *iap,
>
> iap->ia_valid |= ATTR_CTIME;
>
> + if (get_write_count) {
> + host_err = fh_want_write(fhp);
> + if (host_err)
> + goto out_host_err;
> + }
> +
> fh_lock(fhp);
> host_err = notify_change(dentry, iap, NULL);
> fh_unlock(fhp);
On Wed, Feb 08, 2017 at 04:51:01PM -0500, J. Bruce Fields wrote:
> > FYI, Chuck just reported another issue, let's wait for that for now.
I can't really reproduce the issue Chuck reported. So let's get this
fixup in for now, especially as Linus replied to the thread it was
reported in and asked for it.
On Thu, Feb 09, 2017 at 10:43:07AM -0500, J. Bruce Fields wrote:
> On Thu, Feb 09, 2017 at 02:25:16PM +0100, Christoph Hellwig wrote:
> > On Wed, Feb 08, 2017 at 04:51:01PM -0500, J. Bruce Fields wrote:
> > > > FYI, Chuck just reported another issue, let's wait for that for now.
> >
> > I can't really reproduce the issue Chuck reported. So let's get this
> > fixup in for now, especially as Linus replied to the thread it was
> > reported in and asked for it.
>
> I can't find that reply--am I missing a thread?
Dave reported it in reply to Linus' 4.10-rc7 announcement.
Looks like neither you nor linux-nfs were Cc'ed on that.
> OK, so for 4.10 I have:
>
> nfsd: don't get write access twice in nfsd_setattr
> nfsd: restore owner override for truncate
Yepp. Sorry for the mess.
On Thu, Feb 09, 2017 at 02:25:16PM +0100, Christoph Hellwig wrote:
> On Wed, Feb 08, 2017 at 04:51:01PM -0500, J. Bruce Fields wrote:
> > > FYI, Chuck just reported another issue, let's wait for that for now.
>
> I can't really reproduce the issue Chuck reported. So let's get this
> fixup in for now, especially as Linus replied to the thread it was
> reported in and asked for it.
I can't find that reply--am I missing a thread?
OK, so for 4.10 I have:
nfsd: don't get write access twice in nfsd_setattr
nfsd: restore owner override for truncate
--b.