diff mbox series

[v2] iomap: Remove pgoff from tracepoints

Message ID 20200304154706.GH29971@bombadil.infradead.org (mailing list archive)
State New, archived
Headers show
Series [v2] iomap: Remove pgoff from tracepoints | expand

Commit Message

Matthew Wilcox March 4, 2020, 3:47 p.m. UTC 
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>

Comments

Darrick J. Wong March 4, 2020, 3:48 p.m. UTC | #1 
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" }, \
Christoph Hellwig March 4, 2020, 3:55 p.m. UTC | #2 
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>
> 
> 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);

I think we should pass page_offset() for the offset here now.  Maybe
also PAGE_SIZE len for completeness while we're at 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);

Same here.
diff mbox series

Patch

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" }, \