On 6/2/23 18:18, Peter Xu wrote: ...
...whoops, correction, our very own David Hildenbrand recently made changes that contradict the claim that "ksm and uffd selftests are independent". In fact, ksm now *intentionally* depends upon uffd, as of commit 93fb70aa5904c ("selftests/vm: add KSM unmerge tests"), aha! That added commit added a call to test_unmerge_uffd_wp(), to ksm_functional_tests.c . So this needs to stay approximately as-is, it seems.So I think it depends on what is "as-is" to me in the above sentence. :) test_unmerge_uffd_wp() impled its own uffd ioctls, and it still doesn't use any of uffd-common.h of now (e.g. uffd_test_ops). IMHO if we want we can let test_unmerge_uffd_wp() reuse either uffd_get_features(), uffd_open(), uffd_register() etc., but still all of them are provided by vm_util.h not uffd-common.h for now, and that's intended (vm_util.h can contain uffd helpers, or whatever helpers as long as generic mm/ unit tests need).
ksm_functional_tests.c calls uffd_register(). That's about as clear as it gets: this file distinctly depends upon uffd test functionality. The goal here is to put uffd*() routines into uffd-common.[ch], and everything else into vm_utils.[ch]. Because that's what you do, when you have such named files. Putting uffd*() routines somewhere other than uffd-common.* requires some...reason. And all I've heard so far is, "it was already scrambled--as intended, don't mess with it!" :)
We can even move wp_range() from uffd-common.[ch] into vm_utils.[ch], then it can also share that (need to replace err(), that's uffd-common specific). Not necessary anything must be done in this series, though.
But wp_range(), despite its generic-sounding name, is another example of something that remains tightly coupled to the uffd code: it uses UFFDIO_WRITEPROTECT to get its work done. So I'd recommend leaving this one in uffd-common.c. thanks, -- John Hubbard NVIDIA
