From: "chenxiaosong (A)" <chenxiaosong2@huawei.com>
To: <trondmy@kernel.org>, <linux-nfs@vger.kernel.org>
Cc: Scott Mayhew <smayhew@redhat.com>,
"zhangxiaoxu (A)" <zhangxiaoxu5@huawei.com>,
"zhangyi (F)" <yi.zhang@huawei.com>
Subject: Re: [PATCH v2 3/5] NFS: Don't report ENOSPC write errors twice
Date: Tue, 12 Apr 2022 14:24:40 +0800 [thread overview]
Message-ID: <a2babe9f-e85e-3c72-9132-35aa6ae3888b@huawei.com> (raw)
In-Reply-To: <20220411213346.762302-4-trondmy@kernel.org>
在 2022/4/12 5:33, trondmy@kernel.org 写道:
> From: Trond Myklebust <trond.myklebust@hammerspace.com>
>
> 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>
> ---
> fs/nfs/file.c | 33 +++++++++++++--------------------
> 1 file changed, 13 insertions(+), 20 deletions(-)
# 1. wb error mechanism of other filesystem
Other filesystem only clear the wb error when calling fsync(), async
write will not clear the wb error.
# 2. still report unexpected error ... again
After merging this patchset(5 patches), second `dd` of the following
reproducer will still report unexpected error: No space left on device.
Reproducer:
nfs server | nfs client
------------------------------|---------------------------------------------
# No space left on server |
fallocate -l 100G /svr/nospc |
| mount -t nfs $nfs_server_ip:/ /mnt
|
| # Expected error: No space left on device
| dd if=/dev/zero of=/mnt/file count=1
ibs=10K
|
| # Release space on mountpoint
| rm /mnt/nospc
|
| # Unexpected error: No space left on device
| dd if=/dev/zero of=/mnt/file count=1
ibs=10K
# 3. my patchset
https://patchwork.kernel.org/project/linux-nfs/list/?series=628066
My patchset can fix bug of above reproducer.
filemap_sample_wb_err() always return 0 if old writeback error
have not been consumed. filemap_check_wb_err() will return the old error
even if there is no new writeback error between filemap_sample_wb_err() and
filemap_check_wb_err().
```c
since = filemap_sample_wb_err() = 0
errseq_sample
if (!(old & ERRSEQ_SEEN)) // nobody see the error
return 0;
nfs_wb_all // no new error
error = filemap_check_wb_err(..., since) != 0 // unexpected error
```
So we still need record writeback error in address_space flags. The
writeback
error in address_space flags is not used to be reported to userspace, it
is just
used to detect if there is new error while writeback.
if we want to report nuanced writeback error, it is better to detect wb
error from filemap_check_errors(), and then return
-(file->f_mapping->wb_err & MAX_ERRNO) to userspace without consume it.
```c
nfs_mapping_set_error
mapping_set_error
__filemap_set_wb_err // record error sequence
errseq_set
set_bit(..., &mapping->flags) // record address_space flag
// it is not used to be reported, just used to detect
error = filemap_check_errors // -ENOSPC or -EIO
test_and_clear_bit(..., &mapping->flags) // error bit cleared
// now we try to return nuanced writeback error
if (error)
return filemap_check_wb_err(file->f_mapping, 0);
return -(file->f_mapping->wb_err & MAX_ERRNO)
```
next prev parent reply other threads:[~2022-04-12 6:24 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-11 21:33 [PATCH v2 0/5] Ensure mapping errors are reported only once trondmy
2022-04-11 21:33 ` [PATCH v2 1/5] NFS: Do not report EINTR/ERESTARTSYS as mapping errors trondmy
2022-04-11 21:33 ` [PATCH v2 2/5] NFS: fsync() should report filesystem errors over EINTR/ERESTARTSYS trondmy
2022-04-11 21:33 ` [PATCH v2 3/5] NFS: Don't report ENOSPC write errors twice trondmy
2022-04-11 21:33 ` [PATCH v2 4/5] NFS: Do not report flush errors in nfs_write_end() trondmy
2022-04-11 21:33 ` [PATCH v2 5/5] NFS: Don't report errors from nfs_pageio_complete() more than once trondmy
2022-04-12 6:24 ` chenxiaosong (A) [this message]
2022-04-12 12:16 ` [PATCH v2 3/5] NFS: Don't report ENOSPC write errors twice Trond Myklebust
2022-04-12 13:13 ` chenxiaosong (A)
2022-04-12 13:29 ` chenxiaosong (A)
2022-04-12 14:25 ` chenxiaosong (A)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a2babe9f-e85e-3c72-9132-35aa6ae3888b@huawei.com \
--to=chenxiaosong2@huawei.com \
--cc=linux-nfs@vger.kernel.org \
--cc=smayhew@redhat.com \
--cc=trondmy@kernel.org \
--cc=yi.zhang@huawei.com \
--cc=zhangxiaoxu5@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.