linux-ext4 - Re: [PATCH v3 1/7] iversion: update comments with info about atime updates

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]

Message-ID: <e4815337177c74a9928098940dfdcb371017a40c.camel@hammerspace.com>
Date:   Tue, 30 Aug 2022 14:58:27 +0000
From:   Trond Myklebust <trondmy@...merspace.com>
To:     "bfields@...ldses.org" <bfields@...ldses.org>,
        "jlayton@...nel.org" <jlayton@...nel.org>
CC:     "zohar@...ux.ibm.com" <zohar@...ux.ibm.com>,
        "djwong@...nel.org" <djwong@...nel.org>,
        "xiubli@...hat.com" <xiubli@...hat.com>,
        "brauner@...nel.org" <brauner@...nel.org>,
        "neilb@...e.de" <neilb@...e.de>,
        "linux-api@...r.kernel.org" <linux-api@...r.kernel.org>,
        "linux-xfs@...r.kernel.org" <linux-xfs@...r.kernel.org>,
        "david@...morbit.com" <david@...morbit.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "chuck.lever@...cle.com" <chuck.lever@...cle.com>,
        "linux-ceph@...r.kernel.org" <linux-ceph@...r.kernel.org>,
        "linux-nfs@...r.kernel.org" <linux-nfs@...r.kernel.org>,
        "tytso@....edu" <tytso@....edu>,
        "viro@...iv.linux.org.uk" <viro@...iv.linux.org.uk>,
        "jack@...e.cz" <jack@...e.cz>,
        "linux-ext4@...r.kernel.org" <linux-ext4@...r.kernel.org>,
        "linux-btrfs@...r.kernel.org" <linux-btrfs@...r.kernel.org>,
        "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
        "lczerner@...hat.com" <lczerner@...hat.com>,
        "adilger.kernel@...ger.ca" <adilger.kernel@...ger.ca>,
        "walters@...bum.org" <walters@...bum.org>
Subject: Re: [PATCH v3 1/7] iversion: update comments with info about atime
 updates

On Tue, 2022-08-30 at 10:44 -0400, J. Bruce Fields wrote:
> On Tue, Aug 30, 2022 at 09:50:02AM -0400, Jeff Layton wrote:
> > On Tue, 2022-08-30 at 09:24 -0400, J. Bruce Fields wrote:
> > > On Tue, Aug 30, 2022 at 07:40:02AM -0400, Jeff Layton wrote:
> > > > Yes, saying only that it must be different is intentional. What
> > > > we
> > > > really want is for consumers to treat this as an opaque value
> > > > for the
> > > > most part [1]. Therefore an implementation based on hashing
> > > > would
> > > > conform to the spec, I'd think, as long as all of the relevant
> > > > info is
> > > > part of the hash.
> > > 
> > > It'd conform, but it might not be as useful as an increasing
> > > value.
> > > 
> > > E.g. a client can use that to work out which of a series of
> > > reordered
> > > write replies is the most recent, and I seem to recall that can
> > > prevent
> > > unnecessary invalidations in some cases.
> > > 
> > 
> > That's a good point; the linux client does this. That said, NFSv4
> > has a
> > way for the server to advertise its change attribute behavior [1]
> > (though nfsd hasn't implemented this yet).
> 
> It was implemented and reverted.  The issue was that I thought nfsd
> should mix in the ctime to prevent the change attribute going
> backwards
> on reboot (see fs/nfsd/nfsfh.h:nfsd4_change_attribute()), but Trond
> was
> concerned about the possibility of time going backwards.  See
> 1631087ba872 "Revert "nfsd4: support change_attr_type attribute"".
> There's some mailing list discussion to that I'm not turning up right
> now.

My main concern was that some filesystems (e.g. ext3) were failing to
provide sufficient timestamp resolution to actually label the resulting
'change attribute' as being updated monotonically. If the time stamp
doesn't change when the file data or metadata are changed, then the
client has to perform extra checks to try to figure out whether or not
its caches are up to date.

> 
> Did NFSv4 add change_attr_type because some implementations needed
> the
> unordered case, or because they realized ordering was useful but
> wanted
> to keep backwards compatibility?  I don't know which it was.

We implemented it because, as implied above, knowledge of whether or
not the change attribute behaves monotonically, or strictly
monotonically, enables a number of optimisations.

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