Messages in this thread Patch in this message | | | Date | Mon, 30 May 2022 16:38:24 +0200 | From | Miklos Szeredi <> | Subject | Re: [PATCH -next,v2] fuse: return the more nuanced writeback error on close() |
| |
On Mon, May 30, 2022 at 10:02:06AM -0400, Jeff Layton wrote:
> The main difference is that ->flush is called from filp_close, so it's > called when a file descriptor (or equivalent) is being torn down out, > whereas ->fsync is (obviously) called from the fsync codepath. > > We _must_ report writeback errors on fsync, but reporting them on the > close() syscall is less clear. The thing about close() is that it's > going be successful no matter what is returned. The file descriptor will > no longer work afterward regardless. > > fsync also must also initiate writeback of all the buffered data, but > it's not required for filesystems to do that on close() (and in fact, > there are good reasons not to if you can). A successful close() tells > you nothing about whether your data made it to the backing store. It > might just not have been synced out yet. > > Personally, I think it's probably best to _not_ return writeback errors > on close at all. The only "legitimate" error on close is -EBADF. > Arguably, we should make ->flush be void return. Note that most > filp_close callers ignore the error anyway, so it's not much of a > stretch. > > In any case, if you do decide to return errors in fuse_flush, then > advancing the cursor would also have the effect of masking writeback > errors on dup()'ed file descriptors, and I don't think you want to do > that.
Thanks for clarifying.
Chen, would the following patch make sense for your case?
Thanks, Miklos
--- fs/fuse/file.c | 5 ----- 1 file changed, 5 deletions(-)
--- a/fs/fuse/file.c +++ b/fs/fuse/file.c @@ -487,11 +487,6 @@ static int fuse_flush(struct file *file, fuse_sync_writes(inode); inode_unlock(inode); - err = filemap_check_errors(file->f_mapping); - if (err) - return err; - - err = 0; if (fm->fc->no_flush) goto inval_attr_out;
| |