Hi Sarthak, (please trim your responses next time)
On Tue, Apr 28, 2026 at 06:35:51PM +0530, Sarthak Sharma wrote: > On 4/18/26 4:24 PM, Mike Rapoport wrote: > > From: "Mike Rapoport (Microsoft)" <[email protected]> > > > > Convert khugepaged tests to use kselftest framework for reporting and > > tracking successful and failing runs. ... > > @@ -830,16 +783,12 @@ static void collapse_compound_extreme(struct > > collapse_context *c, struct mem_ops > > int i; > > > > p = ops->setup_area(1); > > + ksft_print_msg("Construct PTE page table full of different PTE-mapped > > compound pagesd\n"); > > A small typo here, "pagesd" should be "pages". Thanks. > > for (i = 0; i < hpage_pmd_nr; i++) { > > - printf("\rConstruct PTE page table full of different PTE-mapped > > compound pages %3d/%d...", > > - i + 1, hpage_pmd_nr); > > - > > madvise(BASE_ADDR, hpage_pmd_size, MADV_HUGEPAGE); > > ops->fault(BASE_ADDR, 0, hpage_pmd_size); > > - if (!ops->check_huge(BASE_ADDR, 1)) { > > - printf("Failed to allocate huge page\n"); > > - exit(EXIT_FAILURE); > > - } > > + if (!ops->check_huge(BASE_ADDR, 1)) > > + ksft_exit_fail_msg("Failed to allocate huge page\n"); > > In case we are not able to allocate a hugepage at fault, should this be > reported as a per-test failure instead of bailing out the whole binary > with ksft_exit_fail_msg()? In alloc_at_fault(), a failure to allocate a > hugepage is reported as a normal test result via > ksft_test_result_report(), so should this case behave similarly or is it > intentionally treated as fatal? I realize it was being treated as fatal > before this patch as well, but just wanted to check whether that is > necessary. I don't think it's necessary, but let's address it separately. Here I only mechanically convert the code to use kselftest helpers. > Also, when ksft_exit_fail_msg() is called, totals are printed twice: > once from ksft_exit_fail_msg() itself, and then again when exit() runs > the restore_settings_atexit handler. This is temporal, restore_settings_atexit() is removed from khugepaged in later patches. If anybody feels strongly about it I can fix, but IMO it's not really important. > > @@ -1242,8 +1186,6 @@ int main(int argc, char **argv) > > save_settings(); > > thp_push_settings(&default_settings); > > Also, can save_settings() and thp_push_settings() be moved below > ksft_set_plan(), preserving their order? Yes, you are welcome to send a patch ;-) There a lot of possible improvements all over the place and I wanted to keep changes here to minimum. Let's keep it for the next round. > Since save_settings() is printing a debug line of its own, it would look > better if ksft plan is printed just below the TAP version line. The message from save_settings() is printed after the TAP header: # TAP version 13 # # Save THP and khugepaged settings... OK # 1..26 -- Sincerely yours, Mike.
