Re: [LTP] [PATCH v2 2/3] Add process_mrelease01 test

Wei Gao via ltp
2025-09-10T10:50:06Z

On Thu, Oct 10, 2024 at 02:33:09PM +0200, Andrea Cervesato via ltp wrote:
> Hi!
>
> On 9/12/24 18:28, Cyril Hrubis wrote:
> > Hi!
> > > +static void run(void)
> > > +{
> > > + int ret;
> > > + int pidfd;
> > > + int status;
> > > + pid_t pid;
> > > + int restart;
> > > +
> > > + for (mem_size = CHUNK; mem_size < MAX_SIZE_MB; mem_size += CHUNK) {
> > > + restart = 0;
> > > +
> > > + pid = SAFE_FORK();
> > > + if (!pid) {
> > > + do_child(mem_size);
> > > + exit(0);
> > > + }
> > > +
> > > + TST_CHECKPOINT_WAIT(0);
> > > +
> > > + tst_disable_oom_protection(pid);
> > > +
> > > + if (!memory_is_mapped(pid, *mem_addr, *mem_addr + mem_size)) {
> > > + tst_res(TFAIL, "Memory is not mapped");
> > > + break;
> > > + }
> > > +
> > > + pidfd = SAFE_PIDFD_OPEN(pid, 0);
> > > +
> > > + tst_res(TINFO, "Parent: killing child with PID=%d", pid);
> > > +
> > > + SAFE_KILL(pid, SIGKILL);
> > > +
> > > + ret = tst_syscall(__NR_process_mrelease, pidfd, 0);
> > > + if (ret == -1) {
> > > + if (errno == ESRCH) {
> > > + tst_res(TINFO, "Parent: child terminated before "
> > > + "process_mrelease(). Increase memory size and "
> > > + "restart test");
> > > +
> > > + restart = 1;
> > Does this even happen? I suppose that until the child has been waited
> > for you shouldn't get ESRCH at all. The memory may be freed
> > asynchronously but the pidfd is valid until we do waitpid, at least
> > that's what the description says:
> >
> > https://lore.kernel.org/linux-mm/20210902220029.bfau3YxNP%25akpm@linux-foundation.org/
> >
> > But selftest seems to do the same loop on ESRCH so either the test or
> > the documentation is wrong.
If you add extra sleep between SAFE_KILL(pid, SIGKILL) and tst_syscall
you will get ESRCH, so i guess child process has chance to be fully
reaped before process_mrelease syscall.
> >
> > Michal any idea which is correct?
> >
> > > + } else {
> > > + tst_res(TFAIL | TERRNO, "process_mrelease(%d,0) error", pidfd);
> > > + }
> > > + } else {
> > > + int timeout_ms = 1000;
> > > +
> > > + tst_res(TPASS, "process_mrelease(%d,0) passed", pidfd);
> > > +
> > > + while (memory_is_mapped(pid, *mem_addr, *mem_addr + mem_size) &&
> > > + timeout_ms--)
> > > + usleep(1000);
> > > +
> > > + if (memory_is_mapped(pid, *mem_addr, *mem_addr + mem_size))
> > > + tst_res(TFAIL, "Memory is still mapped inside child memory");
> > > + else
> > > + tst_res(TPASS, "Memory has been released");
> > As far as I understand this this will likely pass even without the
> > process_mrelease() call since the process address space is being teared
> > down anyways. But I do not have an idea how to make things better. I
> > guess that if we wanted to know for sure we would have to run some
> > complex statistics with and without the syscall and compare the
> > timings...
> I don't know, I tried to port the kselftest that seemed to be reasonable.
> Let me know if this is still good, otherwise we need to change the whole
> algorithm. But honestly I don't see many other options than the current one.
kselftest does not has this memory check, i have done some test in my
env, this memory check can pass even without the process_mrelease, so i
think we do not need this check.
> >
> > > + }
> > > +
> > > + SAFE_WAITPID(-1, &status, 0);
> > > + SAFE_CLOSE(pidfd);
> > > +
> > > + if (!restart)
> > > + break;
> > > + }
> > > +}
> > > +
> > > +static void setup(void)
> > > +{
> > > + mem_addr = SAFE_MMAP(NULL,
> > > + sizeof(unsigned long),
> > > + PROT_READ | PROT_WRITE,
> > > + MAP_SHARED | MAP_ANON,
> > > + 0, 0);
> > > +}
> > > +
> > > +static void cleanup(void)
> > > +{
> > > + if (mem_addr)
> > > + SAFE_MUNMAP(mem_addr, sizeof(unsigned long));
> > > +}
> > > +
> > > +static struct tst_test test = {
> > > + .test_all = run,
> > > + .setup = setup,
> > > + .cleanup = cleanup,
> > > + .needs_root = 1,
> > > + .forks_child = 1,
> > > + .min_kver = "5.15",
> > > + .needs_checkpoints = 1,
> > > +};
> > >
> > > --
> > > 2.43.0
> > >
> > >
> > > --
> > > Mailing list info: https://lists.linux.it/listinfo/ltp
> Andrea
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp