On Sun, 2022-03-06 at 09:04 -0500, Trond Myklebust wrote: > 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. > Just to clarify a little. I don't see a need to consume the writeback errors on close(), unless other filesystems do the same. If the intention is that fsync() should see _all_ errors that haven't already been seen, then NFS should follow the same semantics as all the other filesystems. However, if that is the case (i.e. if the semantics for filemap_sample_wb_err() are deliberate, and the intention is that open() should behave as it does today) then we should fix the various instances of filemap_sample_wb_err() / filemap_check_wb_err() in the NFS and nfsd code to ignore the old errors. Their intention is definitely to only report errors that occur in the timeframe between the two calls. -- Trond Myklebust Linux NFS client maintainer, Hammerspace trond.myklebust@xxxxxxxxxxxxxxx
