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

[Andrea Cervesato]

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.
>
> 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.
>
>> +		}
>> +
>> +		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