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

Chaitanya Kulkarni
2025-11-17T07:38:33Z

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