Re: [PATCH v2 1/1] selftests: vm: add process_mrelease tests

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

 



On 5/16/22 2:47 PM, Suren Baghdasaryan wrote:
On Mon, May 16, 2022 at 1:29 PM Shuah Khan <skhan@xxxxxxxxxxxxxxxxxxx> wrote:

On 5/16/22 1:55 AM, Suren Baghdasaryan wrote:
Introduce process_mrelease syscall sanity tests which include tests
which expect to fail:
- process_mrelease with invalid pidfd and flags inputs
- process_mrelease on a live process with no pending signals
and valid process_mrelease usage which is expected to succeed.
Because process_mrelease has to be used against a process with a pending
SIGKILL, it's possible that the process exits before process_mrelease
gets called. In such cases we retry the test with a victim that allocates
twice more memory up to 1GB. This would require the victim process to
spend more time during exit and process_mrelease has a better chance of
catching the process before it exits and succeeding.

On success the test reports the amount of memory the child had to
allocate for reaping to succeed. Sample output:
      Success reaping a child with 1MB of memory allocations

On failure the test reports the failure. Sample outputs:
      All process_mrelease attempts failed!
      process_mrelease: Invalid argument


Nit: Please format this better - include actual example output from the
command and how to run the test examples.

Hmm... Those are the actual outputs from the command and it does not
take any input arguments. Do you mean smth like this:

$ mrelease_test
Success reaping a child with 1MB of memory allocations

$ mrelease_test
All process_mrelease attempts failed!

$ mrelease_test
process_mrelease: Invalid argument

?

This looks good.



Signed-off-by: Suren Baghdasaryan <surenb@xxxxxxxxxx>
---
   tools/testing/selftests/vm/.gitignore      |   1 +
   tools/testing/selftests/vm/Makefile        |   1 +
   tools/testing/selftests/vm/mrelease_test.c | 214 +++++++++++++++++++++
   tools/testing/selftests/vm/run_vmtests.sh  |  16 ++
   4 files changed, 232 insertions(+)
   create mode 100644 tools/testing/selftests/vm/mrelease_test.c


[snip]


Okay these above 3 routines are called once. I am not seeing any point
in making these separate routines. I made the same comment on v1.

I must have misunderstood your previous comment. Will change.


Thank you.




Now the above code can be a separate function which will make it readable.

Ack.



+

Why do you need these ifdefs - syscall will return ENOSYS and you can
key off that. Please take a look at other usages of syscall in the
repo.

The issue is that I need to provide the syscall number when calling
syscall() (in my case __NR_pidfd_open and __NR_process_mrelease) and
if that number is not defined in the userspace headers on a given
system then what should I pass instead?
When implementing this I followed the examples of
https://elixir.bootlin.com/linux/latest/source/tools/testing/selftests/vm/memfd_secret.c#L30
and https://elixir.bootlin.com/linux/latest/source/tools/testing/selftests/vm/userfaultfd.c#L65.
My original implementation was modeled after this approach:
https://elixir.bootlin.com/linux/latest/source/tools/testing/selftests/vm/mlock2.h#L15.
If none of these are correct, could you please point me to the example
you want me to follow?


kselftests include kernel headers. As long as these syscalls are
defined in the kernel headers, the test will build.

Looks it is defined in include/uapi/asm-generic/unistd.h

You can assume it is defined and then if we find architectures that
don't, you can follow what tools/testing/selftests/pidfd/pidfd.h
does.

This way the test can simply call syscall and handle ENOSYS.

thanks,
-- Shuah







[Index of Archives]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux