Hi Trond:
OK, I understand, I will not clear wb err in close(). I will not change
rules that apply to all filesystems.
But if old wb err (such as -ERESTARTSYS or -EINTR, etc.) exist in struct
address_space->wb_err, nfs_file_write() will always return the
unexpected error. 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 // never be error
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
```
I will fix this by store old err before filemap_sample_wb_err(), and
restore old err after filemap_check_wb_err():
```c
// Store the wb err
old_err = file_check_and_advance_wb_err(file)
since = filemap_sample_wb_err()
nfs_wb_all // detect new wb err
error = filemap_check_wb_err(..., since)
// Restore old wb err if no new err is detected
if (!error && old_err)
errseq_set(&file->f_mapping->wb_err, old_err);
```
Is my fixes reasonable ?
On Wed, 2022-06-15 at 20:43 +0800, chenxiaosong (A) wrote:
> Hi Trond:
>
> OK, I understand, I will not clear wb err in close(). I will not
> change
> rules that apply to all filesystems.
>
> But if old wb err (such as -ERESTARTSYS or -EINTR, etc.) exist in
> struct
> address_space->wb_err, nfs_file_write() will always return the
> unexpected error. 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 // never be error
> 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
> ```
>
> I will fix this by store old err before filemap_sample_wb_err(), and
> restore old err after filemap_check_wb_err():
> ```c
> // Store the wb err
> old_err = file_check_and_advance_wb_err(file)
> since = filemap_sample_wb_err()
> nfs_wb_all // detect new wb err
> error = filemap_check_wb_err(..., since)
> // Restore old wb err if no new err is detected
> if (!error && old_err)
> errseq_set(&file->f_mapping->wb_err, old_err);
> ```
>
> Is my fixes reasonable ?
No! That is still special casing the way NFS treats errors vs the way
all other filesystems do.
Either the current VFS implementation of error logging is correct, or
it is not. If it is incorrect, then let's fix it to do the right thing
for everyone.
Either way, please stop posting these NFS-only workarounds.
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected]
在 2022/6/15 21:35, Trond Myklebust 写道:
>
> No! That is still special casing the way NFS treats errors vs the way
> all other filesystems do.
>
> Either the current VFS implementation of error logging is correct, or
> it is not. If it is incorrect, then let's fix it to do the right thing
> for everyone.
>
> Either way, please stop posting these NFS-only workarounds.
>
Thanks for your advice, I will try to fix the sequence error mechanism.