[PATCH OLK-5.10 0/5] nfs: Ensure that the NFS client returns a correct writeback error code
Backport community mainline patch to make close and writeback return correct return values:
Scott Mayhew (2): nfs: nfs_file_write() should check for writeback errors nfs: ensure correct writeback errors are returned on close()
Trond Myklebust (2): NFS: Use of mapping_set_error() results in spurious errors NFS: Don't report ENOSPC write errors twice
HULK solution to avoid residual error codes:
Wang Zhaolong (1): nfs: Ensure write and flush consume writeback errors
fs/nfs/file.c | 39 +++++++++++++++++++-------------------- fs/nfs/nfs4file.c | 3 ++- fs/nfs/write.c | 5 ++++- 3 files changed, 25 insertions(+), 22 deletions(-)
From: Scott Mayhew smayhew@redhat.com
mainline inclusion from mainline-v5.9-rc1 commit ce368536dd614452407dc31e2449eb84681a06af category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/IACBGS
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
--------------------------------
The NFS_CONTEXT_ERROR_WRITE flag (as well as the check of said flag) was removed by commit 6fbda89b257f. The absence of an error check allows writes to be continually queued up for a server that may no longer be able to handle them. Fix it by adding an error check using the generic error reporting functions.
Fixes: 6fbda89b257f ("NFS: Replace custom error reporting mechanism with generic one") Signed-off-by: Scott Mayhew smayhew@redhat.com Signed-off-by: Trond Myklebust trond.myklebust@hammerspace.com Signed-off-by: Wang Zhaolong wangzhaolong1@huawei.com --- fs/nfs/file.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/fs/nfs/file.c b/fs/nfs/file.c index 92a267da3356..4b90c57a449f 100644 --- a/fs/nfs/file.c +++ b/fs/nfs/file.c @@ -594,12 +594,14 @@ static const struct vm_operations_struct nfs_file_vm_ops = { .page_mkwrite = nfs_vm_page_mkwrite, };
-static int nfs_need_check_write(struct file *filp, struct inode *inode) +static int nfs_need_check_write(struct file *filp, struct inode *inode, + int error) { struct nfs_open_context *ctx;
ctx = nfs_file_open_context(filp); - if (nfs_ctx_key_to_expire(ctx, inode)) + if (nfs_error_is_fatal_on_server(error) || + nfs_ctx_key_to_expire(ctx, inode)) return 1; return 0; } @@ -610,6 +612,8 @@ ssize_t nfs_file_write(struct kiocb *iocb, struct iov_iter *from) struct inode *inode = file_inode(file); unsigned long written = 0; ssize_t result; + errseq_t since; + int error;
result = nfs_key_timeout_notify(file, inode); if (result) @@ -634,6 +638,7 @@ ssize_t nfs_file_write(struct kiocb *iocb, struct iov_iter *from) if (iocb->ki_pos > i_size_read(inode)) nfs_revalidate_mapping(inode, file->f_mapping);
+ since = filemap_sample_wb_err(file->f_mapping); nfs_start_io_write(inode); result = generic_write_checks(iocb, from); if (result > 0) { @@ -652,7 +657,8 @@ ssize_t nfs_file_write(struct kiocb *iocb, struct iov_iter *from) goto out;
/* Return error values */ - if (nfs_need_check_write(file, inode)) { + error = filemap_check_wb_err(file->f_mapping, since); + if (nfs_need_check_write(file, inode, error)) { int err = nfs_wb_all(inode); if (err < 0) result = err;
From: Scott Mayhew smayhew@redhat.com
mainline inclusion from mainline-v5.9-rc1 commit 67dd23f9e6fbaf163431912ef5599c5e0693476c category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/IACBGS
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
--------------------------------
nfs_wb_all() calls filemap_write_and_wait(), which uses filemap_check_errors() to determine the error to return. filemap_check_errors() only looks at the mapping->flags and will therefore only return either -ENOSPC or -EIO. To ensure that the correct error is returned on close(), nfs{,4}_file_flush() should call filemap_check_wb_err() which looks at the errseq value in mapping->wb_err without consuming it.
Fixes: 6fbda89b257f ("NFS: Replace custom error reporting mechanism with generic one") Signed-off-by: Scott Mayhew smayhew@redhat.com Signed-off-by: Trond Myklebust trond.myklebust@hammerspace.com Signed-off-by: Wang Zhaolong wangzhaolong1@huawei.com --- fs/nfs/file.c | 5 ++++- fs/nfs/nfs4file.c | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/fs/nfs/file.c b/fs/nfs/file.c index 4b90c57a449f..7be1a7f7fcb2 100644 --- a/fs/nfs/file.c +++ b/fs/nfs/file.c @@ -140,6 +140,7 @@ static int nfs_file_flush(struct file *file, fl_owner_t id) { struct inode *inode = file_inode(file); + errseq_t since;
dprintk("NFS: flush(%pD2)\n", file);
@@ -148,7 +149,9 @@ nfs_file_flush(struct file *file, fl_owner_t id) return 0;
/* Flush writes to the server and return any errors */ - return nfs_wb_all(inode); + since = filemap_sample_wb_err(file->f_mapping); + nfs_wb_all(inode); + return filemap_check_wb_err(file->f_mapping, since); }
ssize_t diff --git a/fs/nfs/nfs4file.c b/fs/nfs/nfs4file.c index fc95b2a0ef9f..70cd0d764c44 100644 --- a/fs/nfs/nfs4file.c +++ b/fs/nfs/nfs4file.c @@ -113,6 +113,7 @@ static int nfs4_file_flush(struct file *file, fl_owner_t id) { struct inode *inode = file_inode(file); + errseq_t since;
dprintk("NFS: flush(%pD2)\n", file);
@@ -128,7 +129,9 @@ nfs4_file_flush(struct file *file, fl_owner_t id) return filemap_fdatawrite(file->f_mapping);
/* Flush writes to the server and return any errors */ - return nfs_wb_all(inode); + since = filemap_sample_wb_err(file->f_mapping); + nfs_wb_all(inode); + return filemap_check_wb_err(file->f_mapping, since); }
#ifdef CONFIG_NFS_V4_2
From: Trond Myklebust trond.myklebust@hammerspace.com
mainline inclusion from mainline-v5.18-rc1 commit 6c984083ec2453dfd3fcf98f392f34500c73e3f2 category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/IACBGS
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
--------------------------------
The use of mapping_set_error() in conjunction with calls to filemap_check_errors() is problematic because every error gets reported as either an EIO or an ENOSPC by filemap_check_errors() in functions such as filemap_write_and_wait() or filemap_write_and_wait_range(). In almost all cases, we prefer to use the more nuanced wb errors.
Fixes: b8946d7bfb94 ("NFS: Revalidate the file mapping on all fatal writeback errors") Signed-off-by: Trond Myklebust trond.myklebust@hammerspace.com Signed-off-by: Wang Zhaolong wangzhaolong1@huawei.com --- fs/nfs/write.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/fs/nfs/write.c b/fs/nfs/write.c index 11a5e1ef9781..4cf060691979 100644 --- a/fs/nfs/write.c +++ b/fs/nfs/write.c @@ -311,7 +311,10 @@ static void nfs_mapping_set_error(struct page *page, int error) struct address_space *mapping = page_file_mapping(page);
SetPageError(page); - mapping_set_error(mapping, error); + filemap_set_wb_err(mapping, error); + if (mapping->host) + errseq_set(&mapping->host->i_sb->s_wb_err, + error == -ENOSPC ? -ENOSPC : -EIO); nfs_set_pageerror(mapping); }
From: Trond Myklebust trond.myklebust@hammerspace.com
mainline inclusion from mainline-v5.19-rc1 commit e6005436f6cc9ed13288f936903f0151e5543485 category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/IACBGS
Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
--------------------------------
Any errors reported by the write() system call need to be cleared from the file descriptor's error tracking. The current call to nfs_wb_all() causes the error to be reported, but since it doesn't call file_check_and_advance_wb_err(), we can end up reporting the same error a second time when the application calls fsync().
Note that since Linux 4.13, the rule is that EIO may be reported for write(), but it must be reported by a subsequent fsync(), so let's just drop reporting it in write.
The check for nfs_ctx_key_to_expire() is just a duplicate to the one already in nfs_write_end(), so let's drop that too.
Reported-by: ChenXiaoSong chenxiaosong2@huawei.com Fixes: ce368536dd61 ("nfs: nfs_file_write() should check for writeback errors") Signed-off-by: Trond Myklebust trond.myklebust@hammerspace.com Signed-off-by: Anna Schumaker Anna.Schumaker@Netapp.com Conflicts: fs/nfs/file.c [nfs_file_write() optimized and eager writes feature added] Signed-off-by: Wang Zhaolong wangzhaolong1@huawei.com --- fs/nfs/file.c | 36 +++++++++++++++--------------------- 1 file changed, 15 insertions(+), 21 deletions(-)
diff --git a/fs/nfs/file.c b/fs/nfs/file.c index 7be1a7f7fcb2..35198bf6e8a5 100644 --- a/fs/nfs/file.c +++ b/fs/nfs/file.c @@ -597,18 +597,6 @@ static const struct vm_operations_struct nfs_file_vm_ops = { .page_mkwrite = nfs_vm_page_mkwrite, };
-static int nfs_need_check_write(struct file *filp, struct inode *inode, - int error) -{ - struct nfs_open_context *ctx; - - ctx = nfs_file_open_context(filp); - if (nfs_error_is_fatal_on_server(error) || - nfs_ctx_key_to_expire(ctx, inode)) - return 1; - return 0; -} - ssize_t nfs_file_write(struct kiocb *iocb, struct iov_iter *from) { struct file *file = iocb->ki_filp; @@ -636,7 +624,7 @@ ssize_t nfs_file_write(struct kiocb *iocb, struct iov_iter *from) if (iocb->ki_flags & IOCB_APPEND) { result = nfs_revalidate_file_size(inode, file); if (result) - goto out; + return result; } if (iocb->ki_pos > i_size_read(inode)) nfs_revalidate_mapping(inode, file->f_mapping); @@ -655,19 +643,25 @@ ssize_t nfs_file_write(struct kiocb *iocb, struct iov_iter *from)
written = result; iocb->ki_pos += written; + nfs_add_stats(inode, NFSIOS_NORMALWRITTENBYTES, written); + result = generic_write_sync(iocb, written); if (result < 0) - goto out; - + return result; +out: /* Return error values */ error = filemap_check_wb_err(file->f_mapping, since); - if (nfs_need_check_write(file, inode, error)) { - int err = nfs_wb_all(inode); - if (err < 0) - result = err; + switch (error) { + default: + break; + case -EDQUOT: + case -EFBIG: + case -ENOSPC: + nfs_wb_all(inode); + error = file_check_and_advance_wb_err(file); + if (error < 0) + result = error; } - nfs_add_stats(inode, NFSIOS_NORMALWRITTENBYTES, written); -out: return result;
out_swapfile:
hulk inclusion category: bugfix bugzilla: https://gitee.com/openeuler/kernel/issues/IACBGS
--------------------------------
When the file is closed and the ->flush is executed, the writeback error is not consumed. As a result, when the file is written next time, the writeback error still exists, causing a new write failure.
Another problem is that in the write process, only special writeback error codes such as -EDQUOT, -EFBIG, and -ENOSPC are detected, and the error codes are consumed. In this case, other error codes may be left.
The writeback error code should be consumed unconditionally in the write(), fsync(), and close() to avoid sampling residual expired codes that are left over from the last invoking.
In the fsync process, the error code can be consumed. Therefore, this patch ensures that write and flush can consume error codes.
Fixes: 6fbda89b257f ("NFS: Replace custom error reporting mechanism with generic one") Signed-off-by: Wang Zhaolong wangzhaolong1@huawei.com --- fs/nfs/file.c | 12 ++++-------- fs/nfs/nfs4file.c | 4 +--- 2 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/fs/nfs/file.c b/fs/nfs/file.c index 35198bf6e8a5..83fec4c204b1 100644 --- a/fs/nfs/file.c +++ b/fs/nfs/file.c @@ -140,7 +140,6 @@ static int nfs_file_flush(struct file *file, fl_owner_t id) { struct inode *inode = file_inode(file); - errseq_t since;
dprintk("NFS: flush(%pD2)\n", file);
@@ -149,9 +148,8 @@ nfs_file_flush(struct file *file, fl_owner_t id) return 0;
/* Flush writes to the server and return any errors */ - since = filemap_sample_wb_err(file->f_mapping); nfs_wb_all(inode); - return filemap_check_wb_err(file->f_mapping, since); + return file_check_and_advance_wb_err(file); }
ssize_t @@ -603,7 +601,6 @@ ssize_t nfs_file_write(struct kiocb *iocb, struct iov_iter *from) struct inode *inode = file_inode(file); unsigned long written = 0; ssize_t result; - errseq_t since; int error;
result = nfs_key_timeout_notify(file, inode); @@ -629,7 +626,6 @@ ssize_t nfs_file_write(struct kiocb *iocb, struct iov_iter *from) if (iocb->ki_pos > i_size_read(inode)) nfs_revalidate_mapping(inode, file->f_mapping);
- since = filemap_sample_wb_err(file->f_mapping); nfs_start_io_write(inode); result = generic_write_checks(iocb, from); if (result > 0) { @@ -650,7 +646,7 @@ ssize_t nfs_file_write(struct kiocb *iocb, struct iov_iter *from) return result; out: /* Return error values */ - error = filemap_check_wb_err(file->f_mapping, since); + error = file_check_and_advance_wb_err(file); switch (error) { default: break; @@ -659,9 +655,9 @@ ssize_t nfs_file_write(struct kiocb *iocb, struct iov_iter *from) case -ENOSPC: nfs_wb_all(inode); error = file_check_and_advance_wb_err(file); - if (error < 0) - result = error; } + if (error < 0) + result = error; return result;
out_swapfile: diff --git a/fs/nfs/nfs4file.c b/fs/nfs/nfs4file.c index 70cd0d764c44..32784a435750 100644 --- a/fs/nfs/nfs4file.c +++ b/fs/nfs/nfs4file.c @@ -113,7 +113,6 @@ static int nfs4_file_flush(struct file *file, fl_owner_t id) { struct inode *inode = file_inode(file); - errseq_t since;
dprintk("NFS: flush(%pD2)\n", file);
@@ -129,9 +128,8 @@ nfs4_file_flush(struct file *file, fl_owner_t id) return filemap_fdatawrite(file->f_mapping);
/* Flush writes to the server and return any errors */ - since = filemap_sample_wb_err(file->f_mapping); nfs_wb_all(inode); - return filemap_check_wb_err(file->f_mapping, since); + return file_check_and_advance_wb_err(file); }
#ifdef CONFIG_NFS_V4_2
反馈： 您发送到kernel@openeuler.org的补丁/补丁集，已成功转换为PR！ PR链接地址： https://gitee.com/openeuler/kernel/pulls/10046 邮件列表地址：https://mailweb.openeuler.org/hyperkitty/list/kernel@openeuler.org/message/A...
FeedBack: The patch(es) which you have sent to kernel@openeuler.org mailing list has been converted to a pull request successfully! Pull request link: https://gitee.com/openeuler/kernel/pulls/10046 Mailing list address: https://mailweb.openeuler.org/hyperkitty/list/kernel@openeuler.org/message/A...
-
patchwork bot -
Wang Zhaolong