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: <2hnq54zc4x2fpxkpuprnrutrwfp3yi5ojntu3e3xfcpeh6ztei@2fwwsemx4y5z>
Date: Wed, 17 Dec 2025 13:42:20 +0100
From: Jan Kara <jack@...e.cz>
To: Christoph Hellwig <hch@....de>
Cc: Christian Brauner <brauner@...nel.org>, 
	Al Viro <viro@...iv.linux.org.uk>, David Sterba <dsterba@...e.com>, Jan Kara <jack@...e.cz>, 
	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 Wed 17-12-25 07:09:41, Christoph Hellwig wrote:
> Currently file_update_time_flags unconditionally returns -EAGAIN if any
> timestamp needs to be updated and IOCB_NOWAIT is passed.  This makes
> non-blocking direct writes impossible on file systems with granular
> enough timestamps.
> 
> Add a S_NOWAIT to ask for timestamps to not block, and return -EAGAIN in
> all methods for now.
> 
> Signed-off-by: Christoph Hellwig <hch@....de>
> Reviewed-by: Jeff Layton <jlayton@...nel.org>

...

> @@ -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?

								Honza

> +			} else {
> +				if (inode_maybe_inc_iversion(inode, updated))
> +					updated |= S_VERSION;
> +			}
>  		}
> -		if (IS_I_VERSION(inode) && inode_maybe_inc_iversion(inode, updated))
> -			updated |= S_VERSION;
> +
> +		if (updated & S_MTIME)
> +			inode_set_mtime_to_ts(inode, now);
>  	} else {
>  		now = current_time(inode);
>  	}
> @@ -2131,6 +2145,9 @@ int inode_update_timestamps(struct inode *inode, int *flags)
>  
>  	*flags = updated;
>  	return 0;
> +bail:
> +	*flags = 0;
> +	return -EAGAIN;
>  }
>  EXPORT_SYMBOL(inode_update_timestamps);
>  
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR