Message ID | 20200304142259.GF29971@bombadil.infradead.org (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | iomap: Fix writepage tracepoint pgoff | expand |
On Wed, Mar 04, 2020 at 06:22:59AM -0800, Matthew Wilcox wrote: > From: Matthew Wilcox (Oracle) <willy@infradead.org> > > page_offset() confusingly returns the number of bytes from the > beginning of the file and not the pgoff, which the tracepoint claims > to be returning. We're already returning the number of bytes from the > beginning of the file in the 'offset' parameter, so correct the pgoff > to be what was apparently intended. > > Fixes: 0b1b213fcf3a ("xfs: event tracing support") > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> I wonder if tracing the byte offset and just changing the name might be more useful. But I agree that we should fix it one way or another.
On Wed, Mar 04, 2020 at 07:25:15AM -0800, Christoph Hellwig wrote: > On Wed, Mar 04, 2020 at 06:22:59AM -0800, Matthew Wilcox wrote: > > From: Matthew Wilcox (Oracle) <willy@infradead.org> > > > > page_offset() confusingly returns the number of bytes from the > > beginning of the file and not the pgoff, which the tracepoint claims > > to be returning. We're already returning the number of bytes from the > > beginning of the file in the 'offset' parameter, so correct the pgoff > > to be what was apparently intended. > > > > Fixes: 0b1b213fcf3a ("xfs: event tracing support") > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> > > I wonder if tracing the byte offset and just changing the name > might be more useful. But I agree that we should fix it one way or > another. I covered that -- "We're already returning the number of bytes from the beginning of the file in the 'offset' parameter, so correct the pgoff to be what was apparently intended." I mean, we could just delete the pgoff instead. Apparently nobody's using it, or they would surely have noticed.
On Wed, Mar 04, 2020 at 07:34:00AM -0800, Matthew Wilcox wrote: > I covered that -- "We're already returning the number of bytes from the > beginning of the file in the 'offset' parameter, so correct the pgoff > to be what was apparently intended." > > I mean, we could just delete the pgoff instead. Apparently nobody's > using it, or they would surely have noticed. Let's just kill it.
diff --git a/fs/iomap/trace.h b/fs/iomap/trace.h index d6ba705f938a..ebc89ec5e6c7 100644 --- a/fs/iomap/trace.h +++ b/fs/iomap/trace.h @@ -56,7 +56,7 @@ 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->pgoff = page->index; __entry->size = i_size_read(inode); __entry->offset = off; __entry->length = len;