On 1/10/22 10:33 PM, Steven Rostedt wrote: > On Fri, 7 Jan 2022 10:29:35 +0530 > Anshuman Khandual <anshuman.khandual@xxxxxxx> wrote: > >> This adds two trace events for PMD based THP migration without split. These >> events closely follow the implementation details like setting and removing >> of PMD migration entries, which are essential operations for THP migration. >> >> Cc: Steven Rostedt <rostedt@xxxxxxxxxxx> >> Cc: Ingo Molnar <mingo@xxxxxxxxxx> >> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> >> Cc: Zi Yan <ziy@xxxxxxxxxx> >> Cc: Naoya Horiguchi <n-horiguchi@xxxxxxxxxxxxx> >> Cc: John Hubbard <jhubbard@xxxxxxxxxx> >> Cc: Matthew Wilcox <willy@xxxxxxxxxxxxx> >> Cc: linux-mm@xxxxxxxxx >> Cc: linux-kernel@xxxxxxxxxxxxxxx >> Signed-off-by: Anshuman Khandual <anshuman.khandual@xxxxxxx> >> --- >> This applies on v5.16-rc8 >> >> Changes in V1: >> >> - Dropped mm, pmdp, page arguments from trace >> - Updated trace argument names and output format >> >> Changes in RFC: >> >> https://lore.kernel.org/all/1640328398-20698-1-git-send-email-anshuman.khandual@xxxxxxx/ >> >> include/trace/events/thp.h | 37 +++++++++++++++++++++++++++++++++++++ >> mm/huge_memory.c | 5 +++++ >> 2 files changed, 42 insertions(+) >> >> diff --git a/include/trace/events/thp.h b/include/trace/events/thp.h >> index d7fbbe551841..193a555aa2ea 100644 >> --- a/include/trace/events/thp.h >> +++ b/include/trace/events/thp.h >> @@ -83,6 +83,43 @@ TRACE_EVENT(hugepage_splitting, >> __entry->addr, __entry->pte) >> ); >> >> +TRACE_EVENT(set_migration_pmd, >> + >> + TP_PROTO(unsigned long addr, unsigned long pmd), >> + >> + TP_ARGS(addr, pmd), >> + >> + TP_STRUCT__entry( >> + __field(unsigned long, addr) >> + __field(unsigned long, pmd) >> + ), >> + >> + TP_fast_assign( >> + __entry->addr = addr; >> + __entry->pmd = pmd; >> + ), >> + >> + TP_printk("create pmd migration entry addr=%lx, pmd=%lx", __entry->addr, __entry->pmd) >> +); >> + >> +TRACE_EVENT(remove_migration_pmd, >> + >> + TP_PROTO(unsigned long addr, unsigned long pmd), >> + >> + TP_ARGS(addr, pmd), >> + >> + TP_STRUCT__entry( >> + __field(unsigned long, addr) >> + __field(unsigned long, pmd) >> + ), >> + >> + TP_fast_assign( >> + __entry->addr = addr; >> + __entry->pmd = pmd; >> + ), >> + >> + TP_printk("remove pmd migration entry addr=%lx, val=%lx", __entry->addr, __entry->pmd) > > The two above are pretty much identical, except the first one has "pmd=%lx" > for the pmd, and the second has "val=%lx" for the pmd. I'd suggest they > both be the same, and then you could save memory by combining the two into Right, both can be the same 'pmd=%lx'. > DECLARE_EVENT_CLASS() / DEFINE_EVENT() macros: > > DECLARE_EVENT_CLASS(migration_pmd, > > TP_PROTO(unsigned long addr, unsigned long pmd), > > TP_ARGS(addr, pmd), > > TP_STRUCT__entry( > __field(unsigned long, addr) > __field(unsigned long, pmd) > ), > > TP_fast_assign( > __entry->addr = addr; > __entry->pmd = pmd; > ), > > TP_printk("create pmd migration entry addr=%lx, pmd=%lx", __entry->addr, __entry->pmd) > ); > > DEFINE_EVENT(migration_pmd, set_migration_pmd, > TP_PROTO(unsigned long addr, unsigned long pmd), > TP_ARGS(addr, pmd) > ); > > DEFINE_EVENT(migration_pmd, remove_migration_pmd, > TP_PROTO(unsigned long addr, unsigned long pmd), > TP_ARGS(addr, pmd) > ); > > And then you have the same thing, but it combines the code which saves both > data and text. But both need to print different messages. Hence I am wondering whether a (const char *) based message can be passed down here as well. create pmd migration entry addr=%lx, pmd=%lx remove pmd migration entry addr=%lx, pmd=%lx > > -- Steve > >> +); >> #endif /* _TRACE_THP_H */ >> >> /* This part must be outside protection */ >> diff --git a/mm/huge_memory.c b/mm/huge_memory.c >> index e5483347291c..d0adc019afe0 100644 >> --- a/mm/huge_memory.c >> +++ b/mm/huge_memory.c >> @@ -39,6 +39,9 @@ >> #include <asm/pgalloc.h> >> #include "internal.h" >> >> +#define CREATE_TRACE_POINTS >> +#include <trace/events/thp.h> >> + >> /* >> * By default, transparent hugepage support is disabled in order to avoid >> * risking an increased memory footprint for applications that are not >> @@ -3173,6 +3176,7 @@ void set_pmd_migration_entry(struct page_vma_mapped_walk *pvmw, >> set_pmd_at(mm, address, pvmw->pmd, pmdswp); >> page_remove_rmap(page, true); >> put_page(page); >> + trace_set_migration_pmd(address, pmd_val(pmdswp)); >> } >> >> void remove_migration_pmd(struct page_vma_mapped_walk *pvmw, struct page *new) >> @@ -3206,5 +3210,6 @@ void remove_migration_pmd(struct page_vma_mapped_walk *pvmw, struct page *new) >> if ((vma->vm_flags & VM_LOCKED) && !PageDoubleMap(new)) >> mlock_vma_page(new); >> update_mmu_cache_pmd(vma, address, pvmw->pmd); >> + trace_remove_migration_pmd(address, pmd_val(pmde)); >> } >> #endif >