[LTP] [PATCH 2/2] Add regression test for epoll_pwait2() timeout

[Martin Doucha]

Hi,
I'll send v2 shortly but some notes below.

On 03. 06. 25 16:33, Andrea Cervesato wrote:
> Hi!
> 
> On 6/3/25 16:13, Martin Doucha wrote:
>> Signed-off-by: Martin Doucha <mdoucha@suse.cz>
>> ---
>>
>> Note: The test will get killed by SIGALRM on failure.
>>
>>   runtest/syscalls                              |  1 +
>>   .../kernel/syscalls/epoll_pwait/.gitignore    |  1 +
>>   .../syscalls/epoll_pwait/epoll_pwait06.c      | 86 +++++++++++++++++++
>>   3 files changed, 88 insertions(+)
>>   create mode 100644 testcases/kernel/syscalls/epoll_pwait/ 
>> epoll_pwait06.c
>>
>> diff --git a/testcases/kernel/syscalls/epoll_pwait/epoll_pwait06.c b/ 
>> testcases/kernel/syscalls/epoll_pwait/epoll_pwait06.c
>> new file mode 100644
>> index 000000000..487992744
>> --- /dev/null
>> +++ b/testcases/kernel/syscalls/epoll_pwait/epoll_pwait06.c
>> @@ -0,0 +1,86 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Copyright (c) 2025 SUSE LLC <mdoucha@suse.cz>
>> + */
>> +
>> +/*\
>> + * Verify that various timeout values don't get misinterpreted as 
>> infinity
>> + * by epoll_pwait() and epoll_pwait2()
> It would be nice to give the reference of the commit which introduced 
> the bug in the tst_test structure.

.tags is for fixes, not buggy kernel commits. V2 will have the fix commit.

>> +
>> +        do {
>> +            alarm(1);
>> +            timeout.tv_nsec /= 10;
>> +            do_epoll_pwait(efd, &e, 1, &timeout, NULL);
>> +            alarm(0);
> What about adding a TINFO here? It should be shown around 10 times, but 
> eventually we would have some debugging data on console to use just in 
> case the test stuck.

The TINFO would only count backwards from 100 million to zero, we don't 
need to know the specific number which triggered the bug. The test won't 
get stuck, SIGALRM will kill it on failure.

>> +static struct tst_test test = {
>> +    .test_all = run,
>> +    .setup = setup,
>> +    .cleanup = cleanup,
>> +    .test_variants = TEST_VARIANTS,
> It's worth to add a timeout for the test, since it's a test which is 
> testing timeout limits.
>> +};
> 
> Also we might move the run() inside a process and check if it eventually 
> gets killed from the parent due to SIGALARM and not other signals.

The test takes 2 seconds on success, 3 seconds in the worst case on 
failure. One inner loop takes ~120ms in total.

We don't care which signal killed it because it's a failure state. And 
since v2 will print the fix commit, we don't need a detailed failure 
explanation either. On success, the test will exit without triggering 
any signals.

-- 
Martin Doucha   mdoucha@suse.cz
SW Quality Engineer
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic