On 2025/9/17 18:51, David Hildenbrand wrote:
On 17.09.25 07:59, Lance Yang wrote:
From: Lance Yang lance.yang@linux.dev
The madv_populate and soft-dirty kselftests currently fail on systems where CONFIG_MEM_SOFT_DIRTY is disabled.
Introduce a new helper softdirty_is_supported() into vm_util.c/h to ensure tests are properly skipped when the feature is not enabled.
I'll note that tools/testing/selftests/mm/config contains
CONFIG_MEM_SOFT_DIRTY=y
But yes, the current arm64 handling is nasty, because some other archs (e.g., riscv) also don't support it yet.
Yep.
LGTM, some nits below:
Thanks for taking time to review!
Suggested-by: David Hildenbrand david@redhat.com Signed-off-by: Lance Yang lance.yang@linux.dev
tools/testing/selftests/mm/madv_populate.c | 21 ++-------------- tools/testing/selftests/mm/soft-dirty.c | 5 +++- tools/testing/selftests/mm/vm_util.c | 28 ++++++++++++++++++++++ tools/testing/selftests/mm/vm_util.h | 1 + 4 files changed, 35 insertions(+), 20 deletions(-)
diff --git a/tools/testing/selftests/mm/madv_populate.c b/tools/ testing/selftests/mm/madv_populate.c index b6fabd5c27ed..43dac7783004 100644 --- a/tools/testing/selftests/mm/madv_populate.c +++ b/tools/testing/selftests/mm/madv_populate.c @@ -264,23 +264,6 @@ static void test_softdirty(void) munmap(addr, SIZE); } -static int system_has_softdirty(void) -{ - /* - * There is no way to check if the kernel supports soft-dirty, other - * than by writing to a page and seeing if the bit was set. But the - * tests are intended to check that the bit gets set when it should, so - * doing that check would turn a potentially legitimate fail into a - * skip. Fortunately, we know for sure that arm64 does not support - * soft-dirty. So for now, let's just use the arch as a corse guide. - */ -#if defined(__aarch64__) - return 0; -#else - return 1; -#endif -}
int main(int argc, char **argv) { int nr_tests = 16; @@ -288,7 +271,7 @@ int main(int argc, char **argv) pagesize = getpagesize(); - if (system_has_softdirty()) + if (softdirty_is_supported()) nr_tests += 5; ksft_print_header(); @@ -300,7 +283,7 @@ int main(int argc, char **argv) test_holes(); test_populate_read(); test_populate_write(); - if (system_has_softdirty()) + if (softdirty_is_supported()) test_softdirty(); err = ksft_get_fail_cnt(); diff --git a/tools/testing/selftests/mm/soft-dirty.c b/tools/testing/ selftests/mm/soft-dirty.c index 8a3f2b4b2186..98e42d2ac32a 100644 --- a/tools/testing/selftests/mm/soft-dirty.c +++ b/tools/testing/selftests/mm/soft-dirty.c @@ -200,8 +200,11 @@ int main(int argc, char **argv) int pagesize; ksft_print_header(); - ksft_set_plan(15); + if (!softdirty_is_supported()) + ksft_exit_skip("soft-dirty is not support\n");
+ ksft_set_plan(15); pagemap_fd = open(PAGEMAP_FILE_PATH, O_RDONLY); if (pagemap_fd < 0) ksft_exit_fail_msg("Failed to open %s\n", PAGEMAP_FILE_PATH); diff --git a/tools/testing/selftests/mm/vm_util.c b/tools/testing/ selftests/mm/vm_util.c index 56e9bd541edd..3173335df775 100644 --- a/tools/testing/selftests/mm/vm_util.c +++ b/tools/testing/selftests/mm/vm_util.c @@ -449,6 +449,34 @@ bool check_vmflag_pfnmap(void *addr) return check_vmflag(addr, "pf"); } +bool softdirty_is_supported(void)
I'd just call it "softdirty_supported" similar to "pagemap_scan_supported()".
Got it.
+{ + char *addr; + int ret = 0;
bool supported = false;
+ size_t pagesize;
+ /* We know for sure that arm64 does not support soft-dirty. */ +#if defined(__aarch64__) + return ret; +#endif
Just drop this arm special case now.
OK
+ pagesize = getpagesize();
const size_t pagesize = getpagesize();
+ /* + * __mmap_complete() always sets VM_SOFTDIRTY for new VMAs, so we + * just mmap a small region and check its VmFlags in /proc/self/ smaps + * for the "sd" flag. + */
/* New mappings are expected to be marked with VM_SOFTDIRTY (sd). */
Cool. Much better!
+ addr = mmap(0, pagesize, PROT_READ | PROT_WRITE, + MAP_ANONYMOUS | MAP_PRIVATE, 0, 0); + if (!addr) + ksft_exit_fail_msg("mmap failed\n");
+ if (check_vmflag(addr, "sd")) + ret = 1;
supported = true;
I'll adjust as you suggested ;)
+ munmap(addr, pagesize); + return ret; +}
Cheers, Lance