Re: [PATCH V4] mm/vmstat: Add events for THP migration without split
From: Daniel Jordan <daniel.m.jordan@oracle.com>
Date: 2020-07-24 15:06:48
Also in:
linux-mm, lkml
I'm assuming the newly-enlarged positive error return of migrate_pages(2) won't have adverse effects in userspace. Didn't see issues with any user in debian codesearch, and can't imagine how it could be relied on. This look ok. Just some nits, take them or leave them as you prefer. Reviewed-by: Daniel Jordan <daniel.m.jordan@oracle.com>
quoted hunk ↗ jump to hunk
diff --git a/include/trace/events/migrate.h b/include/trace/events/migrate.h index 705b33d1e395..4d434398d64d 100644 --- a/include/trace/events/migrate.h +++ b/include/trace/events/migrate.h@@ -46,13 +46,18 @@ MIGRATE_REASON TRACE_EVENT(mm_migrate_pages, TP_PROTO(unsigned long succeeded, unsigned long failed, - enum migrate_mode mode, int reason), + unsigned long thp_succeeded, unsigned long thp_failed, + unsigned long thp_split, enum migrate_mode mode, int reason), - TP_ARGS(succeeded, failed, mode, reason), + TP_ARGS(succeeded, failed, thp_succeeded, thp_failed, + thp_split, mode, reason), TP_STRUCT__entry( __field( unsigned long, succeeded) __field( unsigned long, failed) + __field( unsigned long, thp_succeeded) + __field( unsigned long, thp_failed) + __field( unsigned long, thp_split)
These three are ints in the code, not unsigned long. It can save space in the trace event struct, 8 bytes on my machine.
quoted
quoted
quoted
quoted
diff --git a/mm/migrate.c b/mm/migrate.c@@ -1429,22 +1429,35 @@ int migrate_pages(struct list_head *from, new_page_t get_new_page,
...
quoted
quoted
quoted
quoted
+ bool is_thp = false;
Don't need to initialize, could declare with rc/nr_subpages.
quoted
quoted
quoted
quoted
+ if (is_thp) { + nr_thp_failed++; + nr_failed += nr_subpages; + goto out; + } nr_failed++; goto out;
This instead, in each of the three places with this pattern?:
if (is_thp)
nr_thp_failed++;
nr_failed += nr_subpages;
goto out;
quoted hunk ↗ jump to hunk
diff --git a/Documentation/vm/page_migration.rst b/Documentation/vm/page_migration.rst
...
+5. THP_MIGRATION_SPLIT: A THP was migrated, but not as such: first, the THP had + to be split. After splitting, a migration retry was used for it's sub-pages.
The first part of this might be misinterpreted for the same reason Anshuman changed this earlier (migration didn't necessarily happen). We could just delete "A THP was migrated, but not as such: first, "