linux-kernel - Re: [PATCH 08/10] fs: add support for non-blocking timestamp 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] [day] [month] [year] [list]

Message-ID: <wynhubqgvknr3fl4umfst62xyacck3avmg6qnbp2na6w7ee3qf@odetcif4kozl>
Date: Fri, 19 Dec 2025 16:12:01 +0100
From: Jan Kara <jack@...e.cz>
To: Christoph Hellwig <hch@....de>
Cc: Jan Kara <jack@...e.cz>, Christian Brauner <brauner@...nel.org>, 
	Al Viro <viro@...iv.linux.org.uk>, David Sterba <dsterba@...e.com>, 
	Mike Marshall <hubcap@...ibond.com>, Martin Brandenburg <martin@...ibond.com>, 
	Carlos Maiolino <cem@...nel.org>, Stefan Roesch <shr@...com>, Jeff Layton <jlayton@...nel.org>, 
	linux-kernel@...r.kernel.org, linux-btrfs@...r.kernel.org, linux-fsdevel@...r.kernel.org, 
	gfs2@...ts.linux.dev, io-uring@...r.kernel.org, devel@...ts.orangefs.org, 
	linux-unionfs@...r.kernel.org, linux-mtd@...ts.infradead.org, linux-xfs@...r.kernel.org, 
	linux-nfs@...r.kernel.org
Subject: Re: [PATCH 08/10] fs: add support for non-blocking timestamp updates

On Thu 18-12-25 07:19:00, Christoph Hellwig wrote:
> On Wed, Dec 17, 2025 at 01:42:20PM +0100, Jan Kara wrote:
> > > @@ -2110,12 +2110,26 @@ int inode_update_timestamps(struct inode *inode, int *flags)
> > >  		now = inode_set_ctime_current(inode);
> > >  		if (!timespec64_equal(&now, &ctime))
> > >  			updated |= S_CTIME;
> > > -		if (!timespec64_equal(&now, &mtime)) {
> > > -			inode_set_mtime_to_ts(inode, now);
> > > +		if (!timespec64_equal(&now, &mtime))
> > >  			updated |= S_MTIME;
> > > +
> > > +		if (IS_I_VERSION(inode)) {
> > > +			if (*flags & S_NOWAIT) {
> > > +				/*
> > > +				 * Error out if we'd need timestamp updates, as
> > > +				 * the generally requires blocking to dirty the
> > > +				 * inode in one form or another.
> > > +				 */
> > > +				if (updated && inode_iversion_need_inc(inode))
> > > +					goto bail;
> > 
> > I'm confused here. What the code does is that if S_NOWAIT is set and
> > i_version needs increment we bail. However the comment as well as the
> > changelog speaks about timestamps needing update and not about i_version.
> > And intuitively I agree that if any timestamp is updated, inode needs
> > dirtying and thus we should bail regardless of whether i_version is updated
> > as well or not. What am I missing?
> 
> With lazytime timestamp updates that don't require i_version updates
> are performed in-memory only, and we'll only reach this with S_NOWAIT
> set for those (later in the series, it can't be reached at all as
> of this patch).

Ah, I see now. Thanks for explanation. This interplay between filesystem's
.update_time() helper and inode_update_timestamps() is rather subtle.
Cannot we move the SB_LAZYTIME checking from .update_time() to
inode_update_timestamps() to have it all in one function? The hunk you're
adding to xfs_vn_update_time() later in the series looks like what the
other filesystems using it will want as well?

BTW, I've noticed that ovl_update_time() and fat_update_time() should be
safe wrt NOWAIT IO so perhaps you don't have to disable it in your patch
(or maybe reenable explicitly?).

And I don't really now what orangefs_update_time() is trying to do with its
__orangefs_setattr() call which just copies the zeroed-out timestamps from
iattr into the inode? Mike?

								Honza

-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR