| Message ID | 5eb439e690e72e3a69ad0a5bb48827c7e5ce32a8.1549451324.git.bcodding@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] NFS: Don't use page_file_mapping after removing the page | expand |
Hi Trond and Anna, I want to bump this. The problem we created is easier to hit now than the problem we were fixing, so something should be done for 5.0 if possible. Ben On 6 Feb 2019, at 6:09, Benjamin Coddington wrote: > If nfs_page_async_flush() removes the page from the mapping, then we > can't > use page_file_mapping() on it as nfs_updatepate() is wont to do when > receiving an error. Instead, push the mapping to the stack before the > page > is possibly truncated. > > Fixes: 8fc75bed96bb ("NFS: Fix up return value on fatal errors in > nfs_page_async_flush()") > Signed-off-by: Benjamin Coddington <bcodding@redhat.com> > --- > fs/nfs/write.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/fs/nfs/write.c b/fs/nfs/write.c > index f12cb31a41e5..d09c9f878141 100644 > --- a/fs/nfs/write.c > +++ b/fs/nfs/write.c > @@ -238,9 +238,9 @@ static void nfs_grow_file(struct page *page, > unsigned int offset, unsigned int c > } > > /* A writeback failed: mark the page as bad, and invalidate the page > cache */ > -static void nfs_set_pageerror(struct page *page) > +static void nfs_set_pageerror(struct address_space *mapping) > { > - nfs_zap_mapping(page_file_mapping(page)->host, > page_file_mapping(page)); > + nfs_zap_mapping(mapping->host, mapping); > } > > /* > @@ -994,7 +994,7 @@ static void nfs_write_completion(struct > nfs_pgio_header *hdr) > nfs_list_remove_request(req); > if (test_bit(NFS_IOHDR_ERROR, &hdr->flags) && > (hdr->good_bytes < bytes)) { > - nfs_set_pageerror(req->wb_page); > + nfs_set_pageerror(page_file_mapping(req->wb_page)); > nfs_context_set_write_error(req->wb_context, hdr->error); > goto remove_req; > } > @@ -1348,7 +1348,8 @@ int nfs_updatepage(struct file *file, struct > page *page, > unsigned int offset, unsigned int count) > { > struct nfs_open_context *ctx = nfs_file_open_context(file); > - struct inode *inode = page_file_mapping(page)->host; > + struct address_space *mapping = page_file_mapping(page); > + struct inode *inode = mapping->host; > int status = 0; > > nfs_inc_stats(inode, NFSIOS_VFSUPDATEPAGE); > @@ -1366,7 +1367,7 @@ int nfs_updatepage(struct file *file, struct > page *page, > > status = nfs_writepage_setup(ctx, page, offset, count); > if (status < 0) > - nfs_set_pageerror(page); > + nfs_set_pageerror(mapping); > else > __set_page_dirty_nobuffers(page); > out: > -- > 2.14.3
On Tue, 2019-02-12 at 13:52 -0500, Benjamin Coddington wrote: > Hi Trond and Anna, > > I want to bump this. The problem we created is easier to hit now than > the problem we were fixing, so something should be done for 5.0 if > possible. Sounds good. I'll send it in a pull request this week. Thanks, Anna > > Ben > > On 6 Feb 2019, at 6:09, Benjamin Coddington wrote: > > > If nfs_page_async_flush() removes the page from the mapping, then we > > can't > > use page_file_mapping() on it as nfs_updatepate() is wont to do when > > receiving an error. Instead, push the mapping to the stack before the > > page > > is possibly truncated. > > > > Fixes: 8fc75bed96bb ("NFS: Fix up return value on fatal errors in > > nfs_page_async_flush()") > > Signed-off-by: Benjamin Coddington <bcodding@redhat.com> > > --- > > fs/nfs/write.c | 11 ++++++----- > > 1 file changed, 6 insertions(+), 5 deletions(-) > > > > diff --git a/fs/nfs/write.c b/fs/nfs/write.c > > index f12cb31a41e5..d09c9f878141 100644 > > --- a/fs/nfs/write.c > > +++ b/fs/nfs/write.c > > @@ -238,9 +238,9 @@ static void nfs_grow_file(struct page *page, > > unsigned int offset, unsigned int c > > } > > > > /* A writeback failed: mark the page as bad, and invalidate the page > > cache */ > > -static void nfs_set_pageerror(struct page *page) > > +static void nfs_set_pageerror(struct address_space *mapping) > > { > > - nfs_zap_mapping(page_file_mapping(page)->host, > > page_file_mapping(page)); > > + nfs_zap_mapping(mapping->host, mapping); > > } > > > > /* > > @@ -994,7 +994,7 @@ static void nfs_write_completion(struct > > nfs_pgio_header *hdr) > > nfs_list_remove_request(req); > > if (test_bit(NFS_IOHDR_ERROR, &hdr->flags) && > > (hdr->good_bytes < bytes)) { > > - nfs_set_pageerror(req->wb_page); > > + nfs_set_pageerror(page_file_mapping(req->wb_page)); > > nfs_context_set_write_error(req->wb_context, hdr- > > >error); > > goto remove_req; > > } > > @@ -1348,7 +1348,8 @@ int nfs_updatepage(struct file *file, struct > > page *page, > > unsigned int offset, unsigned int count) > > { > > struct nfs_open_context *ctx = nfs_file_open_context(file); > > - struct inode *inode = page_file_mapping(page)->host; > > + struct address_space *mapping = page_file_mapping(page); > > + struct inode *inode = mapping->host; > > int status = 0; > > > > nfs_inc_stats(inode, NFSIOS_VFSUPDATEPAGE); > > @@ -1366,7 +1367,7 @@ int nfs_updatepage(struct file *file, struct > > page *page, > > > > status = nfs_writepage_setup(ctx, page, offset, count); > > if (status < 0) > > - nfs_set_pageerror(page); > > + nfs_set_pageerror(mapping); > > else > > __set_page_dirty_nobuffers(page); > > out: > > -- > > 2.14.3
diff --git a/fs/nfs/write.c b/fs/nfs/write.c index f12cb31a41e5..d09c9f878141 100644 --- a/fs/nfs/write.c +++ b/fs/nfs/write.c @@ -238,9 +238,9 @@ static void nfs_grow_file(struct page *page, unsigned int offset, unsigned int c } /* A writeback failed: mark the page as bad, and invalidate the page cache */ -static void nfs_set_pageerror(struct page *page) +static void nfs_set_pageerror(struct address_space *mapping) { - nfs_zap_mapping(page_file_mapping(page)->host, page_file_mapping(page)); + nfs_zap_mapping(mapping->host, mapping); } /* @@ -994,7 +994,7 @@ static void nfs_write_completion(struct nfs_pgio_header *hdr) nfs_list_remove_request(req); if (test_bit(NFS_IOHDR_ERROR, &hdr->flags) && (hdr->good_bytes < bytes)) { - nfs_set_pageerror(req->wb_page); + nfs_set_pageerror(page_file_mapping(req->wb_page)); nfs_context_set_write_error(req->wb_context, hdr->error); goto remove_req; } @@ -1348,7 +1348,8 @@ int nfs_updatepage(struct file *file, struct page *page, unsigned int offset, unsigned int count) { struct nfs_open_context *ctx = nfs_file_open_context(file); - struct inode *inode = page_file_mapping(page)->host; + struct address_space *mapping = page_file_mapping(page); + struct inode *inode = mapping->host; int status = 0; nfs_inc_stats(inode, NFSIOS_VFSUPDATEPAGE); @@ -1366,7 +1367,7 @@ int nfs_updatepage(struct file *file, struct page *page, status = nfs_writepage_setup(ctx, page, offset, count); if (status < 0) - nfs_set_pageerror(page); + nfs_set_pageerror(mapping); else __set_page_dirty_nobuffers(page); out:
If nfs_page_async_flush() removes the page from the mapping, then we can't use page_file_mapping() on it as nfs_updatepate() is wont to do when receiving an error. Instead, push the mapping to the stack before the page is possibly truncated. Fixes: 8fc75bed96bb ("NFS: Fix up return value on fatal errors in nfs_page_async_flush()") Signed-off-by: Benjamin Coddington <bcodding@redhat.com> --- fs/nfs/write.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)