Re: [PATCH v6 2/2] wb: inode writeback list tracking tracepoints

[Jan Kara <jack@suse.cz>]

On Tue 19-01-16 12:59:13, Brian Foster wrote:
> The per-sb inode writeback list tracks inodes currently under writeback
> to facilitate efficient sync processing. In particular, it ensures that
> sync only needs to walk through a list of inodes that were cleaned by
> the sync.
> 
> Add a couple tracepoints to help identify when inodes are added/removed
> to and from the writeback lists. Piggyback off of the writeback lazytime
> tracepoint template as it already tracks the relevant inode information.

The patch looks good. You can add:

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

								Honza

> 
> Signed-off-by: Brian Foster <bfoster@redhat.com>
> ---
>  fs/fs-writeback.c                |  9 +++++++--
>  include/trace/events/writeback.h | 22 ++++++++++++++++++----
>  2 files changed, 25 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> index 63b878b..3a62013 100644
> --- a/fs/fs-writeback.c
> +++ b/fs/fs-writeback.c
> @@ -952,8 +952,10 @@ void sb_mark_inode_writeback(struct inode *inode)
>  
>  	if (list_empty(&inode->i_wb_list)) {
>  		spin_lock_irqsave(&sb->s_inode_wblist_lock, flags);
> -		if (list_empty(&inode->i_wb_list))
> +		if (list_empty(&inode->i_wb_list)) {
>  			list_add_tail(&inode->i_wb_list, &sb->s_inodes_wb);
> +			trace_sb_mark_inode_writeback(inode);
> +		}
>  		spin_unlock_irqrestore(&sb->s_inode_wblist_lock, flags);
>  	}
>  }
> @@ -968,7 +970,10 @@ void sb_clear_inode_writeback(struct inode *inode)
>  
>  	if (!list_empty(&inode->i_wb_list)) {
>  		spin_lock_irqsave(&sb->s_inode_wblist_lock, flags);
> -		list_del_init(&inode->i_wb_list);
> +		if (!list_empty(&inode->i_wb_list)) {
> +			list_del_init(&inode->i_wb_list);
> +			trace_sb_clear_inode_writeback(inode);
> +		}
>  		spin_unlock_irqrestore(&sb->s_inode_wblist_lock, flags);
>  	}
>  }
> diff --git a/include/trace/events/writeback.h b/include/trace/events/writeback.h
> index fff846b..962a7f1 100644
> --- a/include/trace/events/writeback.h
> +++ b/include/trace/events/writeback.h
> @@ -727,7 +727,7 @@ DEFINE_EVENT(writeback_single_inode_template, writeback_single_inode,
>  	TP_ARGS(inode, wbc, nr_to_write)
>  );
>  
> -DECLARE_EVENT_CLASS(writeback_lazytime_template,
> +DECLARE_EVENT_CLASS(writeback_inode_template,
>  	TP_PROTO(struct inode *inode),
>  
>  	TP_ARGS(inode),
> @@ -754,25 +754,39 @@ DECLARE_EVENT_CLASS(writeback_lazytime_template,
>  		  show_inode_state(__entry->state), __entry->mode)
>  );
>  
> -DEFINE_EVENT(writeback_lazytime_template, writeback_lazytime,
> +DEFINE_EVENT(writeback_inode_template, writeback_lazytime,
>  	TP_PROTO(struct inode *inode),
>  
>  	TP_ARGS(inode)
>  );
>  
> -DEFINE_EVENT(writeback_lazytime_template, writeback_lazytime_iput,
> +DEFINE_EVENT(writeback_inode_template, writeback_lazytime_iput,
>  	TP_PROTO(struct inode *inode),
>  
>  	TP_ARGS(inode)
>  );
>  
> -DEFINE_EVENT(writeback_lazytime_template, writeback_dirty_inode_enqueue,
> +DEFINE_EVENT(writeback_inode_template, writeback_dirty_inode_enqueue,
>  
>  	TP_PROTO(struct inode *inode),
>  
>  	TP_ARGS(inode)
>  );
>  
> +/*
> + * Inode writeback list tracking.
> + */
> +
> +DEFINE_EVENT(writeback_inode_template, sb_mark_inode_writeback,
> +	TP_PROTO(struct inode *inode),
> +	TP_ARGS(inode)
> +);
> +
> +DEFINE_EVENT(writeback_inode_template, sb_clear_inode_writeback,
> +	TP_PROTO(struct inode *inode),
> +	TP_ARGS(inode)
> +);
> +
>  #endif /* _TRACE_WRITEBACK_H */
>  
>  /* This part must be outside protection */
> -- 
> 2.4.3
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR