Re: [PATCH] mm/migration: Add trace events for THP migrations

[Anshuman Khandual]

On 1/11/22 12:27 PM, Naoya Horiguchi wrote:
> On Tue, Jan 11, 2022 at 10:31:21AM +0530, Anshuman Khandual wrote:
>>
>>
>> On 1/11/22 7:28 AM, Naoya Horiguchi wrote:
>>> Hi Anshuman,
>>>
>>> On Fri, Jan 07, 2022 at 10:29:35AM +0530, Anshuman Khandual 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.
>>>
>>> I often want to check which individual pages are migrated to which places
>>> (or not migrated) for testing, so these new tracepoints could help me.
>>> Maybe these can be much greater if they can handle other types of page
>>> migration for raw pages and hugetlb pages.  Is it hard to cover all such
>>> page migration events?
>>
>> Are you suggesting to cover all migration entry transitions for normal
>> and HugeTLB pages as well ?
> 
> Yes if you like the idea. I think that some events listed below can be grouped
> into one tracepoint event with showing args like pgsize or read/write flags
> (or implementation detail is up to you).

In its simplest form, something like this will work ? Although the THP migration
patch still remains (almost) unchanged.

 include/trace/events/migrate.h | 38 ++++++++++++++++++++++++++++++++++
 mm/migrate.c                   | 10 +++++++--
 mm/rmap.c                      |  3 +++
 3 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/include/trace/events/migrate.h b/include/trace/events/migrate.h
index 779f3fad9ecd..b66652fcc8af 100644
--- a/include/trace/events/migrate.h
+++ b/include/trace/events/migrate.h
@@ -105,6 +105,44 @@ TRACE_EVENT(mm_migrate_pages_start,
 		  __print_symbolic(__entry->reason, MIGRATE_REASON))
 );
 
+TRACE_EVENT(set_migration_pte,
+
+	TP_PROTO(unsigned long addr, unsigned long pte),
+
+	TP_ARGS(addr, pte),
+
+	TP_STRUCT__entry(
+		__field(unsigned long, addr)
+		__field(unsigned long, pte)
+	),
+
+	TP_fast_assign(
+		__entry->addr = addr;
+		__entry->pte = pte;
+	),
+
+	TP_printk("create pte migration entry addr=%lx, pte=%lx", __entry->addr, __entry->pte)
+);
+
+TRACE_EVENT(remove_migration_ptes,
+
+	TP_PROTO(struct page *old_page, struct page *new_page),
+
+	TP_ARGS(old_page, new_page),
+
+	TP_STRUCT__entry(
+		__field(struct page *, old_page)
+		__field(struct page *, new_page)
+	),
+
+	TP_fast_assign(
+		__entry->old_page = old_page;
+		__entry->new_page = new_page;
+	),
+
+	TP_printk("remove pte migration entry old_page=%lx new_page=%lx", __entry->old_page, __entry->new_page);
+);
+
 #endif /* _TRACE_MIGRATE_H */
 
 /* This part must be outside protection */
diff --git a/mm/migrate.c b/mm/migrate.c
index 18ce840914f0..271b1d565642 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -1053,9 +1053,12 @@ static int __unmap_and_move(struct page *page, struct page *newpage,
 	if (!page_mapped(page))
 		rc = move_to_new_page(newpage, page, mode);
 
-	if (page_was_mapped)
+	if (page_was_mapped) {
 		remove_migration_ptes(page,
 			rc == MIGRATEPAGE_SUCCESS ? newpage : page, false);
+		trace_remove_migration_ptes(page,
+			rc == MIGRATEPAGE_SUCCESS ? newpage : page);
+	}
 
 out_unlock_both:
 	unlock_page(newpage);
@@ -1275,9 +1278,12 @@ static int unmap_and_move_huge_page(new_page_t get_new_page,
 	if (!page_mapped(hpage))
 		rc = move_to_new_page(new_hpage, hpage, mode);
 
-	if (page_was_mapped)
+	if (page_was_mapped) {
 		remove_migration_ptes(hpage,
 			rc == MIGRATEPAGE_SUCCESS ? new_hpage : hpage, false);
+		trace_remove_migration_ptes(hpage,
+			rc == MIGRATEPAGE_SUCCESS ? new_hpage : hpage);
+	}
 
 unlock_put_anon:
 	unlock_page(new_hpage);
diff --git a/mm/rmap.c b/mm/rmap.c
index 6a1e8c7f6213..cd373e378694 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -77,6 +77,7 @@
 #include <asm/tlbflush.h>
 
 #include <trace/events/tlb.h>
+#include <trace/events/migrate.h>
 
 #include "internal.h"
 
@@ -1861,6 +1862,7 @@ static bool try_to_migrate_one(struct page *page, struct vm_area_struct *vma,
 			if (pte_swp_uffd_wp(pteval))
 				swp_pte = pte_swp_mkuffd_wp(swp_pte);
 			set_pte_at(mm, pvmw.address, pvmw.pte, swp_pte);
+			trace_set_migration_pte(pvmw.address, pte_val(swp_pte));
 			/*
 			 * No need to invalidate here it will synchronize on
 			 * against the special swap migration pte.
@@ -1929,6 +1931,7 @@ static bool try_to_migrate_one(struct page *page, struct vm_area_struct *vma,
 			if (pte_uffd_wp(pteval))
 				swp_pte = pte_swp_mkuffd_wp(swp_pte);
 			set_pte_at(mm, address, pvmw.pte, swp_pte);
+			trace_set_migration_pte(address, pte_val(swp_pte));
 			/*
 			 * No need to invalidate here it will synchronize on
 			 * against the special swap migration pte.
-- 
2.20.1


> 
>>
>> migrate_pages()
>> 	unmap_and_move_huge_page()
>> 		try_to_migrate()
>> 			make_writable_migration_entry() <---
>> 			make_readable_migration_entry() <---
>> 		remove_migration_ptes() <---
>> 	unmap_and_move()
>> 		__unmap_and_move()
>> 			try_to_migrate()
>> 				make_writable_migration_entry() <---
>> 				make_readable_migration_entry() <---
>> 			remove_migration_ptes() <---
> 
> Thanks,
> Naoya Horiguchi
>