Messages in this thread | | | Subject | Re: [PATCH v4 06/12] mm/debug_vm_pgtable: Use struct pgtable_debug_args in migration and thp tests | From | Gavin Shan <> | Date | Thu, 29 Jul 2021 09:54:06 +1000 |
| |
Hi Anshuman,
On 7/28/21 9:08 PM, Anshuman Khandual wrote: > On 7/27/21 11:43 AM, Gavin Shan wrote: >> This uses struct pgtable_debug_args in the migration and thp test >> functions. It's notable that the pre-allocated page is used in >> swap_migration_tests() as set_pte_at() is used there. >> >> Signed-off-by: Gavin Shan <gshan@redhat.com> >> --- >> mm/debug_vm_pgtable.c | 28 ++++++++++++++-------------- >> 1 file changed, 14 insertions(+), 14 deletions(-) >> >> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c >> index bc153cad9045..9136195efde3 100644 >> --- a/mm/debug_vm_pgtable.c >> +++ b/mm/debug_vm_pgtable.c >> @@ -845,7 +845,7 @@ static void __init pmd_swap_tests(struct pgtable_debug_args *args) >> static void __init pmd_swap_tests(struct pgtable_debug_args *args) { } >> #endif /* CONFIG_ARCH_ENABLE_THP_MIGRATION */ >> >> -static void __init swap_migration_tests(void) >> +static void __init swap_migration_tests(struct pgtable_debug_args *args) >> { >> struct page *page; >> swp_entry_t swp; >> @@ -861,9 +861,10 @@ static void __init swap_migration_tests(void) >> * problematic. Lets allocate a dedicated page explicitly for this >> * purpose that will be freed subsequently. >> */ >> - page = alloc_page(GFP_KERNEL); >> + page = (args->pte_pfn != ULONG_MAX) ? >> + pfn_to_page(args->pte_pfn) : NULL; >> if (!page) { >> - pr_err("page allocation failed\n"); >> + pr_err("no page available\n"); >> return; >> } > > Please check for a valid page earlier in the function and return. Otherwise > this calls out the page unavailability (after starting the test), which is > inconsistent with all other functions like pxx_advanced_tests(). > > [ 1.051633] debug_vm_pgtable: [pte_swap_tests ]: Validating PTE swap > [ 1.052697] debug_vm_pgtable: [pmd_swap_tests ]: Validating PMD swap > [ 1.053765] debug_vm_pgtable: [swap_migration_tests ]: Validating swap migration <===== > [ 1.054900] debug_vm_pgtable: [swap_migration_tests ]: no page available <===== > > Should do this just before pr_info("Validating swap migration\n"). > > ...... > page = (args->pte_pfn != ULONG_MAX) ? pfn_to_page(args->pte_pfn) : NULL; > if (!page) > return; > ..... >
Yes. The order of error messages are sticky to original implementation, but it'd better to be consistent with the new order we have in this series. I will adjust in v5.
Thanks, Gavin
| |