Re: [PATCH v2] iomap: Remove pgoff from tracepoints

["Darrick J. Wong" <darrick.wong@oracle.com>]

On Wed, Mar 04, 2020 at 07:47:06AM -0800, Matthew Wilcox wrote:
> From: Matthew Wilcox (Oracle) <willy@infradead.org>
> 
> The 'pgoff' displayed by the tracepoints wasn't a pgoff at all; it
> was a byte offset from the start of the file.  We already emit that in
> the form of the 'offset', so we can just remove pgoff.  That means we
> can remove 'page' as an argument to the tracepoint, and rename this
> type of tracepoint from being a page class to being a range class.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Looks fine to me,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> 
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index 7057ef155a29..cab29ffb2b40 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -487,7 +487,7 @@ EXPORT_SYMBOL_GPL(iomap_is_partially_uptodate);
>  int
>  iomap_releasepage(struct page *page, gfp_t gfp_mask)
>  {
> -	trace_iomap_releasepage(page->mapping->host, page, 0, 0);
> +	trace_iomap_releasepage(page->mapping->host, 0, 0);
>  
>  	/*
>  	 * mm accommodates an old ext3 case where clean pages might not have had
> @@ -504,7 +504,7 @@ EXPORT_SYMBOL_GPL(iomap_releasepage);
>  void
>  iomap_invalidatepage(struct page *page, unsigned int offset, unsigned int len)
>  {
> -	trace_iomap_invalidatepage(page->mapping->host, page, offset, len);
> +	trace_iomap_invalidatepage(page->mapping->host, offset, len);
>  
>  	/*
>  	 * If we are invalidating the entire page, clear the dirty state from it
> @@ -1503,7 +1503,7 @@ iomap_do_writepage(struct page *page, struct writeback_control *wbc, void *data)
>  	u64 end_offset;
>  	loff_t offset;
>  
> -	trace_iomap_writepage(inode, page, 0, 0);
> +	trace_iomap_writepage(inode, 0, 0);
>  
>  	/*
>  	 * Refuse to write the page out if we are called from reclaim context.
> diff --git a/fs/iomap/trace.h b/fs/iomap/trace.h
> index d6ba705f938a..5693a39d52fb 100644
> --- a/fs/iomap/trace.h
> +++ b/fs/iomap/trace.h
> @@ -41,14 +41,12 @@ DEFINE_EVENT(iomap_readpage_class, name,	\
>  DEFINE_READPAGE_EVENT(iomap_readpage);
>  DEFINE_READPAGE_EVENT(iomap_readahead);
>  
> -DECLARE_EVENT_CLASS(iomap_page_class,
> -	TP_PROTO(struct inode *inode, struct page *page, unsigned long off,
> -		 unsigned int len),
> -	TP_ARGS(inode, page, off, len),
> +DECLARE_EVENT_CLASS(iomap_range_class,
> +	TP_PROTO(struct inode *inode, unsigned long off, unsigned int len),
> +	TP_ARGS(inode, off, len),
>  	TP_STRUCT__entry(
>  		__field(dev_t, dev)
>  		__field(u64, ino)
> -		__field(pgoff_t, pgoff)
>  		__field(loff_t, size)
>  		__field(unsigned long, offset)
>  		__field(unsigned int, length)
> @@ -56,29 +54,26 @@ DECLARE_EVENT_CLASS(iomap_page_class,
>  	TP_fast_assign(
>  		__entry->dev = inode->i_sb->s_dev;
>  		__entry->ino = inode->i_ino;
> -		__entry->pgoff = page_offset(page);
>  		__entry->size = i_size_read(inode);
>  		__entry->offset = off;
>  		__entry->length = len;
>  	),
> -	TP_printk("dev %d:%d ino 0x%llx pgoff 0x%lx size 0x%llx offset %lx "
> +	TP_printk("dev %d:%d ino 0x%llx size 0x%llx offset %lx "
>  		  "length %x",
>  		  MAJOR(__entry->dev), MINOR(__entry->dev),
>  		  __entry->ino,
> -		  __entry->pgoff,
>  		  __entry->size,
>  		  __entry->offset,
>  		  __entry->length)
>  )
>  
> -#define DEFINE_PAGE_EVENT(name)		\
> -DEFINE_EVENT(iomap_page_class, name,	\
> -	TP_PROTO(struct inode *inode, struct page *page, unsigned long off, \
> -		 unsigned int len),	\
> -	TP_ARGS(inode, page, off, len))
> -DEFINE_PAGE_EVENT(iomap_writepage);
> -DEFINE_PAGE_EVENT(iomap_releasepage);
> -DEFINE_PAGE_EVENT(iomap_invalidatepage);
> +#define DEFINE_RANGE_EVENT(name)		\
> +DEFINE_EVENT(iomap_range_class, name,	\
> +	TP_PROTO(struct inode *inode, unsigned long off, unsigned int len),\
> +	TP_ARGS(inode, off, len))
> +DEFINE_RANGE_EVENT(iomap_writepage);
> +DEFINE_RANGE_EVENT(iomap_releasepage);
> +DEFINE_RANGE_EVENT(iomap_invalidatepage);
>  
>  #define IOMAP_TYPE_STRINGS \
>  	{ IOMAP_HOLE,		"HOLE" }, \