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

[Richard Palethorpe <rpalethorpe@suse.de>]

Hello,

Andrea Cervesato via ltp <ltp@lists.linux.it> writes:

> This test verifies EPOLLET functionality.
>
> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
> ---
> Make use of SAFE_EPOLL_* macros.
> Wait for EPOLLOUT event instead of EPOLLIN
>
>  .../kernel/syscalls/epoll_wait/.gitignore     |  1 +
>  .../kernel/syscalls/epoll_wait/epoll_wait06.c | 90 +++++++++++++++++++
>  2 files changed, 91 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..f263f9041
> --- /dev/null
> +++ b/testcases/kernel/syscalls/epoll_wait/epoll_wait06.c
> @@ -0,0 +1,90 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
> + */
> +
> +/*\
> + * [Description]
> + *
> + * Verify that EPOLLET is correctly handled by epoll_wait.
> + *
> + * [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 hang (return 0) because there's data left
> + *    to read.
> + */

Do you get the purpose of EPOLLET? It's for when you fill the write
buffer and want to sleep until you can write again.

However you dont' want to receive EPOLLOUT the whole time because
epoll_wait never waits for reads. It will constantly return even when
there are no writes to do and no reads.

> +
> +#include <poll.h>
> +#include <sys/epoll.h>
> +#include "tst_test.h"
> +#include "tst_epoll.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)
> +{
> +	char buff[WRITE_SIZE];
> +	struct epoll_event evt_receive;
> +	struct epoll_event evt_request;
> +
> +	SAFE_PIPE(fds);
> +
> +	evt_request.events = EPOLLOUT | EPOLLET;
> +	evt_request.data.fd = fds[0];
> +
> +	epfd = SAFE_EPOLL_CREATE1(0);
> +
> +	tst_res(TINFO, "Polling channel with EPOLLET");
> +
> +	SAFE_EPOLL_CTL(epfd, EPOLL_CTL_ADD, fds[1], &evt_request);
> +
> +	tst_res(TINFO, "Write bytes on channel");
> +
> +	memset(buff, 'a', WRITE_SIZE);
> +	SAFE_WRITE(0, fds[1], buff, WRITE_SIZE);
> +
> +	SAFE_EPOLL_WAIT(epfd, &evt_receive, 1, 2000);
> +
> +	if ((evt_receive.events & EPOLLOUT) == 0) {
> +		tst_res(TFAIL, "No EPOLLOUT received");
> +		goto close;
> +	}
> +
> +	tst_res(TINFO, "Received EPOLLOUT event. Read half bytes from channel");
> +
> +	memset(buff, 0, READ_SIZE);
> +	SAFE_READ(1, evt_receive.data.fd, buff, READ_SIZE);

This really only tests that you receive one event and that EPOLLET is
not trivially broken.

I'm not sure what the reads and writes are for.

If you want to make a really trivial test, then don't confuse it with
random operations that don't have an obvious impact.

> +
> +	TST_EXP_EQ_LI(epoll_wait(epfd, &evt_receive, 1, 10), 0);
> +
> +close:
> +	SAFE_CLOSE(fds[0]);
> +	SAFE_CLOSE(fds[1]);
> +}
> +
> +static struct tst_test test = {
> +	.cleanup = cleanup,
> +	.test_all = run,
> +};
> -- 
> 2.35.3


-- 
Thank you,
Richard.

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