Re: [PATCH 10/14] fs: factor out a sync_lazytime helper

[Chaitanya Kulkarni <chaitanyak@nvidia.com>]

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@lst.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@nvidia.com>

-ck