| Message ID | 20170207091244.GA14911@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <hch@lst.de> > Reported-by: Dave Jones <davej@codemonkey.org.uk> 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); -- 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 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 <hch@lst.de> > > Reported-by: Dave Jones <davej@codemonkey.org.uk> > > 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. -- 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 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 <hch@lst.de> > > > Reported-by: Dave Jones <davej@codemonkey.org.uk> > > > > 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. -- 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 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.
--
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 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. -- 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 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. -- 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/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);
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 <hch@lst.de> Reported-by: Dave Jones <davej@codemonkey.org.uk> -- 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