linux-kernel - Re: [PATCH 07/10] fs: add a ->sync

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: <20251218061317.GA2775@lst.de>
Date: Thu, 18 Dec 2025 07:13:17 +0100
From: Christoph Hellwig <hch@....de>
To: Jan Kara <jack@...e.cz>
Cc: Christoph Hellwig <hch@....de>, 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 07/10] fs: add a ->sync_lazytime method

On Wed, Dec 17, 2025 at 01:30:18PM +0100, Jan Kara wrote:
> >  	if (flags & I_DIRTY_INODE) {
> > +		bool was_dirty_time =
> > +			inode_state_read_once(inode) & I_DIRTY_TIME;
> > +
> >  		/*
> >  		 * Inode timestamp update will piggback on this dirtying.
> >  		 * We tell ->dirty_inode callback that timestamps need to
> >  		 * be updated by setting I_DIRTY_TIME in flags.
> >  		 */
> > -		if (inode_state_read_once(inode) & I_DIRTY_TIME) {
> > +		if (was_dirty_time) {
> >  			spin_lock(&inode->i_lock);
> >  			if (inode_state_read(inode) & I_DIRTY_TIME) {
> >  				inode_state_clear(inode, I_DIRTY_TIME);
> >  				flags |= I_DIRTY_TIME;
> > +				was_dirty_time = true;
> 
> This looks bogus. was_dirty_time is already true here. What I think you
> wanted here is to set it to false if locked I_DIRTY_TIME check failed.
> Otherwise the patch looks good.

Or better set it to false at initialization time and only set it to
true here to simply things a bit.  But otherwise: yes.