Re: [LTP] [PATCH v2] Add epoll_wait06 test

[Richard Palethorpe <rpalethorpe@suse.de>]

Hello,

Cyril Hrubis <chrubis@suse.cz> writes:

> Hi!
>> This test verifies EPOLLET functionality.
>> 
>> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
>> ---
>>  .../kernel/syscalls/epoll_wait/.gitignore     |  1 +
>>  .../kernel/syscalls/epoll_wait/epoll_wait06.c | 97 +++++++++++++++++++
>>  2 files changed, 98 insertions(+)
>>  create mode 100644 testcases/kernel/syscalls/epoll_wait/epoll_wait06.c
>> 
>> diff --git a/testcases/kernel/syscalls/epoll_wait/.gitignore b/testcases/kernel/syscalls/epoll_wait/.gitignore
>> index ab5a9c010..8c5ed7c5c 100644
>> --- a/testcases/kernel/syscalls/epoll_wait/.gitignore
>> +++ b/testcases/kernel/syscalls/epoll_wait/.gitignore
>> @@ -3,3 +3,4 @@ epoll_wait02
>>  epoll_wait03
>>  epoll_wait04
>>  epoll_wait05
>> +epoll_wait06
>> diff --git a/testcases/kernel/syscalls/epoll_wait/epoll_wait06.c b/testcases/kernel/syscalls/epoll_wait/epoll_wait06.c
>> new file mode 100644
>> index 000000000..20f6233c9
>> --- /dev/null
>> +++ b/testcases/kernel/syscalls/epoll_wait/epoll_wait06.c
>> @@ -0,0 +1,97 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
>> + */
>> +
>> +/*\
>> + * [Description]
>> + *
>> + * Verify that edge triggered behavior is correctly handled by epoll.
>> + *
>> + * [Algorithm]
>> + *
>> + * 1. The file descriptor that represents the read side of a pipe (rfd) is
>> + *    registered on the epoll instance.
>> + * 2. A pipe writer writes 2 kB of data on the write side of the pipe.
>> + * 3. A call to epoll_wait(2) is done that will return rfd as a ready file
>> + *    descriptor.
>> + * 4. The pipe reader reads 1 kB of data from rfd.
>> + * 5. A call to epoll_wait(2) should fail because there's data left to read.
>> + */
>
> I do not like this description that much. First of all the the flag we
> are testing is not mentioned there at all, the EPOLLET should be
> mentioned in the [Description].
>
> The second call to epoll_wait() does not fail, failure for syscall means
> that -1 is returned and errno is set. It does not report any data ready
> on the file descriptor, that is not failure at all, that is how the
> EPOLLET flag is supposed to work.

I'll add to this that testing EPOLLET makes most sense with EPOLLOUT
first. Usually it is used to detect when writing becomes possible again
after a transport becomes full. Without it then epoll_wait would
instantly return whenever writing is possible which is most of the
time.

>
>> +#include <poll.h>
>> +#include <sys/epoll.h>
>> +#include "tst_test.h"
>> +
>> +#define WRITE_SIZE 2048
>> +#define READ_SIZE (WRITE_SIZE / 2)
>> +
>> +static int fds[2];
>> +static int epfd;
>> +
>> +static void cleanup(void)
>> +{
>> +	if (epfd > 0)
>> +		SAFE_CLOSE(epfd);
>> +
>> +	if (fds[0] > 0)
>> +		SAFE_CLOSE(fds[0]);
>> +
>> +	if (fds[1] > 0)
>> +		SAFE_CLOSE(fds[1]);
>> +}
>> +
>> +static void run(void)
>> +{
>> +	int res;
>> +	char buff[WRITE_SIZE];
>> +	struct epoll_event evt_receive;
>> +	struct epoll_event evt_request;
>> +
>> +	SAFE_PIPE(fds);
>> +
>> +	evt_request.events = EPOLLIN | EPOLLET;
>> +	evt_request.data.fd = fds[0];
>> +
>> +	epfd = epoll_create(2);

Also again, please use the SAFE_* macros I just merged.

>> +	if (epfd == -1)
>> +		tst_brk(TBROK | TERRNO, "fail to create epoll instance");
>> +
>> +	tst_res(TINFO, "Polling channel with EPOLLET");
>> +
>> +	res = epoll_ctl(epfd, EPOLL_CTL_ADD, fds[0], &evt_request);
>> +	if (res == -1)
>> +		tst_brk(TBROK | TERRNO, "epoll_ctl failure");
>> +
>> +	tst_res(TINFO, "Write bytes on channel");
>> +
>> +	memset(buff, 'a', WRITE_SIZE);
>> +	SAFE_WRITE(0, fds[1], buff, WRITE_SIZE);
>> +
>> +	res = epoll_wait(epfd, &evt_receive, 1, 2000);
>> +	if (res <= 0) {
>> +		tst_res(TFAIL | TERRNO, "epoll_wait() returned %i", res);
>> +		goto close;
>> +	}
>> +
>> +	if ((evt_receive.events & EPOLLIN) == 0) {
>> +		tst_res(TFAIL, "No data received");
>> +		goto close;
>> +	}
>> +
>> +	tst_res(TINFO, "Received EPOLLIN event. Read half bytes from channel");
>> +
>> +	memset(buff, 0, READ_SIZE);
>> +	SAFE_READ(1, evt_receive.data.fd, buff, READ_SIZE);
>> +
>> +	TST_EXP_EQ_LI(epoll_wait(epfd, &evt_receive, 1, 10), 0);
>> +
>> +close:
>
> 	SAFE_CLOSE(epfd) ?
>
>> +	SAFE_CLOSE(fds[0]);
>> +	SAFE_CLOSE(fds[1]);
>> +}
>> +
>> +static struct tst_test test = {
>> +	.cleanup = cleanup,
>> +	.test_all = run,
>> +};
>> -- 
>> 2.35.3
>> 
>> 
>> -- 
>> Mailing list info: https://lists.linux.it/listinfo/ltp
>
> -- 
> Cyril Hrubis
> chrubis@suse.cz


-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp