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

[Richard Palethorpe <rpalethorpe@suse.de>]

Hello,

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

> This test verifies EPOLLONESHOT functionality.
>
> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
> ---
> Make use of SAFE_EPOLL_* macros
> Listen to EPOLLIN event instead of EPOLLOUT
>
>  .../kernel/syscalls/epoll_wait/.gitignore     |  1 +
>  .../kernel/syscalls/epoll_wait/epoll_wait07.c | 78 +++++++++++++++++++
>  2 files changed, 79 insertions(+)
>  create mode 100644 testcases/kernel/syscalls/epoll_wait/epoll_wait07.c
>
> diff --git a/testcases/kernel/syscalls/epoll_wait/.gitignore b/testcases/kernel/syscalls/epoll_wait/.gitignore
> index 8c5ed7c5c..66ac18ae2 100644
> --- a/testcases/kernel/syscalls/epoll_wait/.gitignore
> +++ b/testcases/kernel/syscalls/epoll_wait/.gitignore
> @@ -4,3 +4,4 @@ epoll_wait03
>  epoll_wait04
>  epoll_wait05
>  epoll_wait06
> +epoll_wait07
> diff --git a/testcases/kernel/syscalls/epoll_wait/epoll_wait07.c b/testcases/kernel/syscalls/epoll_wait/epoll_wait07.c
> new file mode 100644
> index 000000000..9a492c148
> --- /dev/null
> +++ b/testcases/kernel/syscalls/epoll_wait/epoll_wait07.c
> @@ -0,0 +1,78 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2022 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
> + */
> +
> +/*\
> + * [Description]
> + *
> + * Verify that EPOLLONESHOT is correctly handled by epoll_wait.
> + * We open a channel, write in it two times and verify that EPOLLIN has been
> + * received only once.
> + */
> +
> +#include <poll.h>
> +#include <sys/epoll.h>
> +#include "tst_test.h"
> +#include "tst_epoll.h"
> +
> +#define WRITE_SIZE 2048
> +
> +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 = EPOLLIN | EPOLLONESHOT;
> +	evt_request.data.fd = fds[1];

I'm not sure why you set the data?

> +
> +	epfd = SAFE_EPOLL_CREATE1(0);
> +
> +	tst_res(TINFO, "Polling channel with EPOLLONESHOT");
> +
> +	SAFE_EPOLL_CTL(epfd, EPOLL_CTL_ADD, fds[0], &evt_request);
> +
> +	tst_res(TINFO, "Write on channel multiple times");
> +
> +	memset(buff, 'a', WRITE_SIZE);
> +	SAFE_WRITE(0, fds[1], buff, WRITE_SIZE);
> +	SAFE_WRITE(0, fds[1], buff, WRITE_SIZE);

Why call write twice?

You don't read the data between waits and it is level triggered not edge
triggered.

> +
> +	SAFE_EPOLL_WAIT(epfd, &evt_receive, 1, 2000);
> +
> +	if ((evt_receive.events & EPOLLIN) == 0) {
> +		tst_res(TFAIL, "No data received");

The chances of this failing are very close to zero, but if it does then
debugging will be difficult.

> +		goto close;
> +	}
> +
> +	tst_res(TINFO, "Received first EPOLLIN event");
> +
> +	TST_EXP_EQ_LI(epoll_wait(epfd, &evt_receive, 1, 0), 0);
> +

The test is valid AFAICT, but there is stuff which confuses it.

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