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

[Jan Kara <jack@suse.cz>]

On Wed 17-12-25 07:09:39, 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>
> Reviewed-by: Jeff Layton <jlayton@kernel.org>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/fs-writeback.c                | 22 +++++++++++++++-------
>  fs/inode.c                       |  5 +----
>  fs/internal.h                    |  3 ++-
>  fs/sync.c                        |  4 ++--
>  include/trace/events/writeback.h |  6 ------
>  5 files changed, 20 insertions(+), 20 deletions(-)
> 
> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> index 7870c158e4a2..fa555e10d8b9 100644
> --- a/fs/fs-writeback.c
> +++ b/fs/fs-writeback.c
> @@ -1711,6 +1711,16 @@ static void requeue_inode(struct inode *inode, struct bdi_writeback *wb,
>  	}
>  }
>  
> +bool sync_lazytime(struct inode *inode)
> +{
> +	if (!(inode_state_read_once(inode) & I_DIRTY_TIME))
> +		return false;
> +
> +	trace_writeback_lazytime(inode);
> +	mark_inode_dirty_sync(inode);
> +	return true;
> +}
> +
>  /*
>   * 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.
> @@ -1750,17 +1760,15 @@ __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_state_read_once(inode) & 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);
> -	}
> +			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 2c0d69f7fd01..f1c09fc0913d 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -1979,11 +1979,8 @@ void iput(struct inode *inode)
>  	if (atomic_add_unless(&inode->i_count, -1, 1))
>  		return;
>  
> -	if ((inode_state_read_once(inode) & I_DIRTY_TIME) && inode->i_nlink) {
> -		trace_writeback_lazytime_iput(inode);
> -		mark_inode_dirty_sync(inode);
> +	if (inode->i_nlink && sync_lazytime(inode))
>  		goto retry;
> -	}
>  
>  	spin_lock(&inode->i_lock);
>  	if (unlikely((inode_state_read(inode) & I_DIRTY_TIME) && inode->i_nlink)) {
> diff --git a/fs/internal.h b/fs/internal.h
> index ab638d41ab81..18a062c1b5b0 100644
> --- a/fs/internal.h
> +++ b/fs/internal.h
> @@ -214,7 +214,8 @@ bool in_group_or_capable(struct mnt_idmap *idmap,
>  /*
>   * fs-writeback.c
>   */
> -extern long get_nr_dirty_inodes(void);
> +long get_nr_dirty_inodes(void);
> +bool sync_lazytime(struct inode *inode);
>  
>  /*
>   * dcache.c
> diff --git a/fs/sync.c b/fs/sync.c
> index 431fc5f5be06..4283af7119d1 100644
> --- a/fs/sync.c
> +++ b/fs/sync.c
> @@ -183,8 +183,8 @@ int vfs_fsync_range(struct file *file, loff_t start, loff_t end, int datasync)
>  
>  	if (!file->f_op->fsync)
>  		return -EINVAL;
> -	if (!datasync && (inode_state_read_once(inode) & I_DIRTY_TIME))
> -		mark_inode_dirty_sync(inode);
> +	if (!datasync)
> +		sync_lazytime(inode);
>  	return file->f_op->fsync(file, start, end, datasync);
>  }
>  EXPORT_SYMBOL(vfs_fsync_range);
> diff --git a/include/trace/events/writeback.h b/include/trace/events/writeback.h
> index 311a341e6fe4..7162d03e69a5 100644
> --- a/include/trace/events/writeback.h
> +++ b/include/trace/events/writeback.h
> @@ -856,12 +856,6 @@ DEFINE_EVENT(writeback_inode_template, writeback_lazytime,
>  	TP_ARGS(inode)
>  );
>  
> -DEFINE_EVENT(writeback_inode_template, writeback_lazytime_iput,
> -	TP_PROTO(struct inode *inode),
> -
> -	TP_ARGS(inode)
> -);
> -
>  DEFINE_EVENT(writeback_inode_template, writeback_dirty_inode_enqueue,
>  
>  	TP_PROTO(struct inode *inode),
> -- 
> 2.47.3
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR