Re: [PATCH 10/12] selftests/mm: move uffd* routines from vm_util.c to uffd-common.c

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux