| Message ID | 20180302160038.1598-1-smayhew@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 2018-03-02 at 11:00 -0500, Scott Mayhew wrote: > It seems that nfs_commit_inode can be called where the nfs_inode has > outstanding requests and the commit lists are empty. That can lead > to > invalidate_complete_page2 failing due to the associated page having > private data which in turn leads to invalidate_inode_pages2_range > returning -EBUSY. > > Instead of having nfs_commit_inode exit early when the commit lists > are > empty, only do so if nrequests is also 0. > > I'm not seeing how this failure would happen. Is there a bug in nfs_launder_page() and/or nfs_wb_page()? -- Trond Myklebust Linux NFS client maintainer, PrimaryData trond.myklebust@primarydata.com
On Fri, 2018-03-02 at 16:52 +0000, Trond Myklebust wrote: > On Fri, 2018-03-02 at 11:00 -0500, Scott Mayhew wrote: > > It seems that nfs_commit_inode can be called where the nfs_inode > > has > > outstanding requests and the commit lists are empty. That can lead > > to > > invalidate_complete_page2 failing due to the associated page having > > private data which in turn leads to invalidate_inode_pages2_range > > returning -EBUSY. > > > > Instead of having nfs_commit_inode exit early when the commit lists > > are > > empty, only do so if nrequests is also 0. > > > > > > I'm not seeing how this failure would happen. Is there a bug in > nfs_launder_page() and/or nfs_wb_page()? > > Ah... It looks as if do_launder_page() only checks for PageDirty(), so it won't catch the unstable write state. Can't we fix that? It looks as if all the other filesystem launder_page() handlers would be safe with a check for PageDirty || PagePrivate. -- Trond Myklebust Linux NFS client maintainer, PrimaryData trond.myklebust@primarydata.com
On Fri, Mar 02, 2018 at 11:00:38AM -0500, Scott Mayhew wrote: > It seems that nfs_commit_inode can be called where the nfs_inode has > outstanding requests and the commit lists are empty. That can lead to > invalidate_complete_page2 failing due to the associated page having > private data which in turn leads to invalidate_inode_pages2_range > returning -EBUSY. For what it's worth, I verified that this fixes the EBUSY I was seeing: http://marc.info/?i=20180223160350.GF15876@fieldses.org --b. > > Instead of having nfs_commit_inode exit early when the commit lists are > empty, only do so if nrequests is also 0. > > Fixes: dc4fd9ab01 ("nfs: don't wait on commit in nfs_commit_inode() if there were no commit requests") > Signed-off-by: Scott Mayhew <smayhew@redhat.com> > --- > fs/nfs/write.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/nfs/write.c b/fs/nfs/write.c > index 7428a66..0268bd1 100644 > --- a/fs/nfs/write.c > +++ b/fs/nfs/write.c > @@ -1890,7 +1890,7 @@ int nfs_commit_inode(struct inode *inode, int how) > if (res) > error = nfs_generic_commit_list(inode, &head, how, &cinfo); > nfs_commit_end(cinfo.mds); > - if (res == 0) > + if (res == 0 && !nfs_have_writebacks(inode)) > return res; > if (error < 0) > goto out_error; > -- > 2.9.5 > > -- > 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 -- 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 Mon, 2018-03-05 at 16:16 -0500, J. Bruce Fields wrote: > On Fri, Mar 02, 2018 at 11:00:38AM -0500, Scott Mayhew wrote: > > It seems that nfs_commit_inode can be called where the nfs_inode > > has > > outstanding requests and the commit lists are empty. That can lead > > to > > invalidate_complete_page2 failing due to the associated page having > > private data which in turn leads to invalidate_inode_pages2_range > > returning -EBUSY. > > For what it's worth, I verified that this fixes the EBUSY I was > seeing: > > http://marc.info/?i=20180223160350.GF15876@fieldses.org > Fine, but the patch will also cause the inode to be marked as dirty in cases where there are no unstable writes to commit, but there are pages undergoing writeback. IOW: it regresses the fix that was made in dc4fd9ab01 So please do look into fixing do_launder_page(). > > > > > Instead of having nfs_commit_inode exit early when the commit lists > > are > > empty, only do so if nrequests is also 0. > > > > Fixes: dc4fd9ab01 ("nfs: don't wait on commit in nfs_commit_inode() > > if there were no commit requests") > > Signed-off-by: Scott Mayhew <smayhew@redhat.com> > > --- > > fs/nfs/write.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/fs/nfs/write.c b/fs/nfs/write.c > > index 7428a66..0268bd1 100644 > > --- a/fs/nfs/write.c > > +++ b/fs/nfs/write.c > > @@ -1890,7 +1890,7 @@ int nfs_commit_inode(struct inode *inode, int > > how) > > if (res) > > error = nfs_generic_commit_list(inode, &head, how, > > &cinfo); > > nfs_commit_end(cinfo.mds); > > - if (res == 0) > > + if (res == 0 && !nfs_have_writebacks(inode)) > > return res; > > if (error < 0) > > goto out_error; > > -- > > 2.9.5 > > > > -- > > 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 > > -- Trond Myklebust Linux NFS client maintainer, PrimaryData trond.myklebust@primarydata.com
diff --git a/fs/nfs/write.c b/fs/nfs/write.c index 7428a66..0268bd1 100644 --- a/fs/nfs/write.c +++ b/fs/nfs/write.c @@ -1890,7 +1890,7 @@ int nfs_commit_inode(struct inode *inode, int how) if (res) error = nfs_generic_commit_list(inode, &head, how, &cinfo); nfs_commit_end(cinfo.mds); - if (res == 0) + if (res == 0 && !nfs_have_writebacks(inode)) return res; if (error < 0) goto out_error;
It seems that nfs_commit_inode can be called where the nfs_inode has outstanding requests and the commit lists are empty. That can lead to invalidate_complete_page2 failing due to the associated page having private data which in turn leads to invalidate_inode_pages2_range returning -EBUSY. Instead of having nfs_commit_inode exit early when the commit lists are empty, only do so if nrequests is also 0. Fixes: dc4fd9ab01 ("nfs: don't wait on commit in nfs_commit_inode() if there were no commit requests") Signed-off-by: Scott Mayhew <smayhew@redhat.com> --- fs/nfs/write.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)