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 for Android: free password hash cracker in your pocket

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

Date:   Tue, 12 Apr 2022 14:27:23 +0000
From:   Trond Myklebust <trondmy@...merspace.com>
To:     "anna@...nel.org" <anna@...nel.org>,
        "chenxiaosong2@...wei.com" <chenxiaosong2@...wei.com>,
        "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

On Tue, 2022-04-12 at 22:12 +0800, chenxiaosong (A) wrote:
> 在 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?
> 

It will clear ENOSPC, EDQUOT and EFBIG. It should not clear other
errors that are not supposed to be reported by write().

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


As I keep repeating, that is _documented behaviour_!

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