From: "NeilBrown" <email@example.com>
To: "Dave Chinner" <firstname.lastname@example.org>
Cc: "Jeff Layton" <email@example.com>,
"Darrick J. Wong" <firstname.lastname@example.org>,
"Darrick J . Wong" <email@example.com>
Subject: Re: [PATCH] xfs: fix i_version handling in xfs
Date: Fri, 19 Aug 2022 10:35:27 +1000 [thread overview]
Message-ID: <firstname.lastname@example.org> (raw)
On Thu, 18 Aug 2022, Dave Chinner wrote:
> On Thu, Aug 18, 2022 at 11:52:12AM +1000, NeilBrown wrote:
> > On Thu, 18 Aug 2022, Dave Chinner wrote:
> > >
> > > > Maybe we should just go back to using ctime. ctime is *exactly* what
> > > > NFSv4 wants, as long as its granularity is sufficient to catch every
> > > > single change. Presumably XFS doesn't try to ensure this. How hard
> > > > would it be to get any ctime update to add at least one nanosecond?
> > > > This would be enabled by a mount option, or possibly be a direct request
> > > > from nfsd.
> > >
> > > We can't rely on ctime to be changed during a modification because
> > > O_NOCMTIME exists to enable "user invisible" modifications to be
> > > made. On XFS these still bump iversion, so while they are invisible
> > > to the user, they are still tracked by the filesystem and anything
> > > that wants to know if the inode data/metadata changed.
> > >
> > O_NOCMTIME isn't mentioned in the man page, so it doesn't exist :-(
> > If they are "user invisible", should they then also be "NFS invisible"?
> > I think so.
> Maybe, but now you're making big assumptions about what is being
> done by those operations. Userspace can write whatever it likes,
> nothing says that O_NOCMTIME can't change user visible data or
Nope. The only assumption I'm making is that if the ctime/mtime don't
change, then it is safe to trust any cached content. I think that is
broadly assumed in the Posix world. Anyone who uses O_NOCMTIME must
understand the risks (not currently documented ....) and it must be
assumed they will handled them properly. We cannot allow the addition
of O_NOCMTIME to make us think "ctime and mtime don't mean what they
used to, we cannot trust them any more".
> But having uses of it that don't change user visible data does not
> mean it can't be used for changing user visible data. Hence we made
> the defensive assumption that O_NOCMTIME was a mechanism that could
> be used to hide data changes from forensic analysis. With that in
> mind, it was important that the change counter captured changes made
> even when O_NOCMTIME was specified to leave behind a breadcrumb to
> indicate unexpected changes may had been made to the file.
Having a breadcrumb seems reasonable. Calling that breadcrumb "i_version"
might be questionable - though specifications seem to be vague so this
decision is probably defensible.
> Yeah, we had lots of different requirements for the XFS on-disk
> change counter when we were considering adding it. NFSv4 was one of
> the least demanding and least defined requirements; it's taken a
> *decade* for this atime issue to be noticed, so I really don't think
> there's anything wrong with how XFs has implemented persistent
> change counters.
> What it tells me is that the VFS needs more appropriate atime
> filtering for NFSv4's change attribute requirements....
I don't agree with that last point. I think "atime == mtime" and
"atime > mtime" are distinctly different states which should recorded.
I think Trond's' observation about implicit updates is on-point.
There is no need to include implicit atime updates in i_version. If
anyone cares about those they can combine i_version and atime into a
single value. If that value changes, then something changed, possibly
an implicit atime update.
Excluding implicit atime updates makes i_version strictly more useful.
It doesn't lose any value and does gain some.
next prev parent reply other threads:[~2022-08-19 0:35 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-16 13:17 [PATCH] xfs: fix i_version handling in xfs Jeff Layton
2022-08-16 15:43 ` Darrick J. Wong
2022-08-16 15:58 ` Jeff Layton
2022-08-16 22:42 ` Dave Chinner
2022-08-16 23:57 ` Dave Chinner
2022-08-17 12:02 ` Jeff Layton
2022-08-18 1:07 ` Dave Chinner
2022-08-18 11:12 ` Jeff Layton
2022-08-18 0:34 ` NeilBrown
2022-08-18 1:32 ` Dave Chinner
2022-08-18 1:52 ` NeilBrown
2022-08-18 2:22 ` Trond Myklebust
2022-08-18 3:00 ` Dave Chinner
2022-08-19 0:35 ` NeilBrown [this message]
2022-08-18 11:00 ` Jeff Layton
2022-08-18 23:43 ` NeilBrown
2022-08-18 1:11 ` Trond Myklebust
2022-08-18 3:37 ` Dave Chinner
2022-08-18 4:15 ` Trond Myklebust
2022-08-18 11:03 ` Jeff Layton
2022-08-23 0:05 ` Dave Chinner
2022-08-23 1:33 ` Trond Myklebust
2022-08-16 17:14 ` David Wysochanski
2022-08-16 23:37 ` Dave Chinner
2022-08-17 12:10 ` Jeff Layton
2022-08-17 21:57 ` Dave Chinner
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).