linux-kernel - Re: [PATCH 10/14] fs: factor out 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: <cff9dffc-dd49-45db-bc47-efab498065c4@nvidia.com>
Date: Mon, 17 Nov 2025 07:38:29 +0000
From: Chaitanya Kulkarni <chaitanyak@...dia.com>
To: Christoph Hellwig <hch@....de>, Christian Brauner <brauner@...nel.org>
CC: 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-kernel@...r.kernel.org>,
	"linux-btrfs@...r.kernel.org" <linux-btrfs@...r.kernel.org>,
	"gfs2@...ts.linux.dev" <gfs2@...ts.linux.dev>, "io-uring@...r.kernel.org"
	<io-uring@...r.kernel.org>, "devel@...ts.orangefs.org"
	<devel@...ts.orangefs.org>, "linux-unionfs@...r.kernel.org"
	<linux-unionfs@...r.kernel.org>, "linux-mtd@...ts.infradead.org"
	<linux-mtd@...ts.infradead.org>, "linux-xfs@...r.kernel.org"
	<linux-xfs@...r.kernel.org>, "linux-nfs@...r.kernel.org"
	<linux-nfs@...r.kernel.org>
Subject: Re: [PATCH 10/14] fs: factor out a sync_lazytime helper

On 11/13/25 22:26, Christoph Hellwig wrote:
> Centralize how we synchronize a lazytime update into the actual on-disk
> timestamp into a single helper.
>
> Signed-off-by: Christoph Hellwig <hch@....de>
> ---
>   fs/fs-writeback.c                | 27 +++++++++++++++++----------
>   fs/inode.c                       |  5 +----
>   fs/internal.h                    |  3 ++-
>   fs/sync.c                        |  4 ++--
>   include/trace/events/writeback.h |  6 ------
>   5 files changed, 22 insertions(+), 23 deletions(-)
>
> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> index 930697f39153..ae6d1f1ccc71 100644
> --- a/fs/fs-writeback.c
> +++ b/fs/fs-writeback.c
> @@ -1693,6 +1693,16 @@ static void requeue_inode(struct inode *inode, struct bdi_writeback *wb,
>   	}
>   }
>   
> +bool sync_lazytime(struct inode *inode)
> +{
> +	if (!(inode->i_state & I_DIRTY_TIME))
> +		return false;
> +
> +	trace_writeback_lazytime(inode);
> +	mark_inode_dirty_sync(inode);
> +	return false;
> +}
> +

This sync_lazytime() will always return false ?
shouldn't this be returning true at sometime if not then why not
change return type to void ?

returning same value doesn't add any value here ..

>   /*
>    * Write out an inode and its dirty pages (or some of its dirty pages, depending
>    * on @wbc->nr_to_write), and clear the relevant dirty flags from i_state.
> @@ -1732,17 +1742,14 @@ __writeback_single_inode(struct inode *inode, struct writeback_control *wbc)
>   	}
>   
>   	/*
> -	 * If the inode has dirty timestamps and we need to write them, call
> -	 * mark_inode_dirty_sync() to notify the filesystem about it and to
> -	 * change I_DIRTY_TIME into I_DIRTY_SYNC.
> +	 * For data integrity writeback, or when the dirty interval expired,
> +	 * ask the file system to propagata lazy timestamp updates into real
> +	 * dirty state.
>   	 */
> -	if ((inode->i_state & I_DIRTY_TIME) &&
> -	    (wbc->sync_mode == WB_SYNC_ALL ||
> -	     time_after(jiffies, inode->dirtied_time_when +
> -			dirtytime_expire_interval * HZ))) {
> -		trace_writeback_lazytime(inode);
> -		mark_inode_dirty_sync(inode);
> -	}
> +	if (wbc->sync_mode == WB_SYNC_ALL ||
> +	    time_after(jiffies, inode->dirtied_time_when +
> +			dirtytime_expire_interval * HZ))
> +		sync_lazytime(inode);
>   
>   	/*
>   	 * Get and clear the dirty flags from i_state.  This needs to be done
> diff --git a/fs/inode.c b/fs/inode.c
> index 559ce5c07188..34d572c99313 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -1942,11 +1942,8 @@ void iput(struct inode *inode)
>   	if (atomic_add_unless(&inode->i_count, -1, 1))
>   		return;
>   
> -	if ((inode->i_state & I_DIRTY_TIME) && inode->i_nlink) {
> -		trace_writeback_lazytime_iput(inode);
> -		mark_inode_dirty_sync(inode);
> +	if (inode->i_nlink && sync_lazytime(inode))

since sync_lazytime() is always returning false goto below will
never execute ? which makes following goto dead code in this patch ?


otherwise, looks good.

Reviewed-by: Chaitanya Kulkarni <kch@...dia.com>

-ck