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@xxxxxxxxxx> --- 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
