From 40786c92ef20915addbe2bcf8f2d6666a4dd999b Mon Sep 17 00:00:00 2001 From: Scott Mayhew Date: Tue, 16 Jul 2024 16:24:45 +0800 Subject: [PATCH 1/5] nfs: nfs_file_write() should check for writeback errors 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/?id=ce368536dd614452407dc31e2449eb84681a06af -------------------------------- 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 Signed-off-by: Trond Myklebust Signed-off-by: Wang Zhaolong --- 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 54de3e0906dc..7669653bf1af 100644 --- a/fs/nfs/file.c +++ b/fs/nfs/file.c @@ -585,12 +585,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; } @@ -601,6 +603,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) @@ -625,6 +629,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) { @@ -643,7 +648,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; -- Gitee From 11c6e9d709ccfbadf30b92fd9c230a87e2062b88 Mon Sep 17 00:00:00 2001 From: Scott Mayhew Date: Tue, 16 Jul 2024 16:24:46 +0800 Subject: [PATCH 2/5] nfs: ensure correct writeback errors are returned on close() 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/?id=67dd23f9e6fbaf163431912ef5599c5e0693476c -------------------------------- 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 Signed-off-by: Trond Myklebust Signed-off-by: Wang Zhaolong --- 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 7669653bf1af..ad856b7b9a46 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 -- Gitee From 2469941c967c5e4f13f3cb12e780c9d959793a99 Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Tue, 16 Jul 2024 16:24:47 +0800 Subject: [PATCH 3/5] NFS: Use of mapping_set_error() results in spurious errors 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/?id=6c984083ec2453dfd3fcf98f392f34500c73e3f2 -------------------------------- 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 Signed-off-by: Wang Zhaolong --- 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 95d841827e2a..4eca7085f520 100644 --- a/fs/nfs/write.c +++ b/fs/nfs/write.c @@ -310,7 +310,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); } -- Gitee From 39a74684f08f4504bee10e0b6ab7bd55e580cf38 Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Tue, 16 Jul 2024 16:24:48 +0800 Subject: [PATCH 4/5] NFS: Don't report ENOSPC write errors twice 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/?id=e6005436f6cc9ed13288f936903f0151e5543485 -------------------------------- 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 Fixes: ce368536dd61 ("nfs: nfs_file_write() should check for writeback errors") Signed-off-by: Trond Myklebust Signed-off-by: Anna Schumaker Conflicts: fs/nfs/file.c [nfs_file_write() optimized and eager writes feature added] Signed-off-by: Wang Zhaolong --- 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 ad856b7b9a46..1291504fe255 100644 --- a/fs/nfs/file.c +++ b/fs/nfs/file.c @@ -588,18 +588,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; @@ -627,7 +615,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); @@ -646,19 +634,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: -- Gitee From d9d9c382de4904708836db6b8f27c9c98228a957 Mon Sep 17 00:00:00 2001 From: Wang Zhaolong Date: Tue, 16 Jul 2024 16:24:49 +0800 Subject: [PATCH 5/5] nfs: Ensure write and flush consume writeback errors Offering: HULK 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 --- 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 1291504fe255..43a2cf278548 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 @@ -594,7 +592,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); @@ -620,7 +617,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) { @@ -641,7 +637,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; @@ -650,9 +646,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 -- Gitee