Andrew Morton <a...@linux-foundation.org> writes: >> + TP_STRUCT__entry( >> + __field(struct page *, page) >> + __field(unsigned long, i_no) > > May as well call this i_ino - there's little benefit in using a > different identifier. Agreed for patch V2. > >> + __field(unsigned long, pageofs) > > "index". Agreed for patch V2.
> >> + __field(dev_t, s_dev) > > Perhaps use super_block.s_id here If you imply by that that dereferencing page->mapping->host->i_sb and looking for field s_dev, then I don't agree. Sometimes i_sb is NULL from what I recall, especially in cases where the read page is from a journaling partition. So unless I didn't understand you, I'll keep this part as well as the following, to cover : - a mapping of an actual file - a mapping of a partition block > >> + ), >> + >> + TP_fast_assign( >> + __entry->page = page; >> + __entry->i_no = page->mapping->host->i_ino; >> + __entry->pageofs = page->index; >> + if (page->mapping->host->i_sb) >> + __entry->s_dev = page->mapping->host->i_sb->s_dev; >> + else >> + __entry->s_dev = page->mapping->host->i_rdev; > > and hence avoid all this stuff. See above. > >> + ), >> + >> + TP_printk("page=%p pfn=%lu blk=%d:%d inode+ofs=%lu+%lu", >> + __entry->page, >> + page_to_pfn(__entry->page), >> + MAJOR(__entry->s_dev), MINOR(__entry->s_dev), >> + __entry->i_no, >> + __entry->pageofs << PAGE_SHIFT) >> +); >> + >> +TRACE_EVENT(mm_filemap_add_to_page_cache, > Agreed for patch V2. Thanks for the review. -- Robert -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/