Re: [PATCH -next 1/2] nfs: nfs{,4}_file_flush should consume writeback error

[Trond Myklebust <trondmy@hammerspace.com>]

On Sun, 2022-03-06 at 11:54 +0800, chenxiaosong (A) wrote:
> It would be more clear if I update the reproducer like this:
> 
>          nfs server                 |       nfs client
>   --------------------------------- |--------------------------------
> -
>   # No space left on server         |
>   fallocate -l 100G /server/nospace |
>                                     | mount -t nfs $nfs_server_ip:/
> /mnt
>                                     |
>                                     | # Expected error
>                                     | dd if=/dev/zero of=/mnt/file
>                                     |
>                                     | # Release space on mountpoint
>                                     | rm /mnt/nospace
>                                     |
>                                     | # Unexpected error
>                                     | dd if=/dev/zero of=/mnt/file
> 
> The Unexpected error (No space left on device) when doing second
> `dd`, 
> is from unconsumed writeback error after close() the file when doing 
> first `dd`. There is enough space when doing second `dd`, we should
> not 
> report the nospace error.
> 
> We should report and consume the writeback error when userspace call 
> close()->flush(), the writeback error should not be left for next
> open().
> 
> Currently, fsync() will consume the writeback error while calling 
> file_check_and_advance_wb_err(), close()->flush() should also consume
> the writeback error.

No. That's not part of the API of any other filesystem. Why should we
make an exception for NFS?

The problem here seems to be the way that filemap_sample_wb_err() is
defined, and how it initialises f->f_wb_err in the function
do_dentry_open(). It is designed to do exactly what you see above.



-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com