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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 12 Apr 2022 22:12:57 +0800
From:   "chenxiaosong (A)" <chenxiaosong2@...wei.com>
To:     Trond Myklebust <trondmy@...merspace.com>,
        "anna@...nel.org" <anna@...nel.org>,
        "smayhew@...hat.com" <smayhew@...hat.com>
CC:     "linux-nfs@...r.kernel.org" <linux-nfs@...r.kernel.org>,
        "liuyongqiang13@...wei.com" <liuyongqiang13@...wei.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "yi.zhang@...wei.com" <yi.zhang@...wei.com>,
        "zhangxiaoxu5@...wei.com" <zhangxiaoxu5@...wei.com>
Subject: Re: [PATCH -next 1/2] nfs: nfs{,4}_file_flush should consume
 writeback error

在 2022/4/12 21:56, Trond Myklebust 写道:
> On Tue, 2022-04-12 at 21:46 +0800, chenxiaosong (A) wrote:
>>
>> Other filesystem will _not_ clear writeback error on close().
>> And other filesystem will _not_ clear writeback error on async
>> write() too.
>>
>> Other filesystem _only_ clear writeback error on fsync() or sync
>> write().
>>
> 
> Yes. We might even consider not reporting writeback errors at all in
> close(), since most developers don't check it. We certainly don't want
> to clear those errors there because the manpages don't document that as
> being the case.
> 
>> Should NFS follow the same semantics as all the other filesystems?
> 
> It needs to follow the semantics described in the manpage for write(2)
> and fsync(2) as closely as possible, yes. That documentation is
> supposed to be normative for application developers.
> 
> We won't guarantee to immediately report ENOSPC like other filesystems
> do (because that would require us to only support synchronous writes),
> however that behaviour is already documented in the manpage.
> 
> We may also report some errors that are not documented in the manpage
> (e.g. EACCES or EROFS) simply because those errors cannot always be
> reported at open() time, as would be the case for a local filesystem.
> That's just how the NFS protocol works (particularly for the case of
> the stateless NFSv3 protocol).
> 

After merging your patchset, NFS will clear wb error on async write(), 
is this reasonable?

And more importantly, we can not detect new error by using 
filemap_sample_wb_err()/filemap_sample_wb_err() while nfs_wb_all(),just 
as I described:

```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
```