diff mbox series

[3/3] syscalls/io_submit: Convert libaio wrapper function to kernel syscall

Message ID 20210429115021.24128-4-xieziyao@huawei.com
State Changes Requested
Headers show
Series syscalls/aio: Convert libaio wrapper function to kernel syscall | expand

Commit Message

Xie Ziyao April 29, 2021, 11:50 a.m. UTC
Instead of using the libaio wrapper function, the system call is changed to be invoked via syscall(2).

Signed-off-by: Xie Ziyao <xieziyao@huawei.com>
---
 .../kernel/syscalls/io_submit/io_submit01.c   | 110 ++++++++----------
 1 file changed, 48 insertions(+), 62 deletions(-)

--
2.17.1

Comments

Cyril Hrubis May 4, 2021, 12:09 p.m. UTC | #1
Hi!
> Instead of using the libaio wrapper function, the system call is
> changed to be invoked via syscall(2).

Ideally we should test _both_ syscall() and the library to maximize the
coverage. We can easily do that with .test_variants, have a look at
stime tests and testcases/kernel/syscalls/stime/stime_var.h how this is
done.
Xie Ziyao May 6, 2021, 12:27 p.m. UTC | #2
Hi, Cyril,

If we should test _both_ syscall() and the library, I would prefer to 
split libaio and native aio into two testcases in this testsuite, since 
<libaio.h> conflicts with <linux/aio_abi.h> during actual modification.

Excuse me, is there any other good way to solve this problem?

Thanks very much!

Kind Regards,
Ziyao

On 2021/5/4 20:09, Cyril Hrubis wrote:
> Hi!
>> Instead of using the libaio wrapper function, the system call is
>> changed to be invoked via syscall(2).
> 
> Ideally we should test _both_ syscall() and the library to maximize the
> coverage. We can easily do that with .test_variants, have a look at
> stime tests and testcases/kernel/syscalls/stime/stime_var.h how this is
> done.
>
Petr Vorel May 6, 2021, 12:57 p.m. UTC | #3
Hi Ziyao,

> Hi, Cyril,

> If we should test _both_ syscall() and the library, I would prefer to split
> libaio and native aio into two testcases in this testsuite, since <libaio.h>
> conflicts with <linux/aio_abi.h> during actual modification.

> Excuse me, is there any other good way to solve this problem?
NOTE: if most of the test the same, with little help of #ifdef and maybe 1-2
macros we can have single source for both variants (compiling 2 binaries).
See setdomainname and sethostname tests (setdomainname.h).

Kind regards,
Petr

> Thanks very much!

> Kind Regards,
> Ziyao

> On 2021/5/4 20:09, Cyril Hrubis wrote:
> > Hi!
> > > Instead of using the libaio wrapper function, the system call is
> > > changed to be invoked via syscall(2).

> > Ideally we should test _both_ syscall() and the library to maximize the
> > coverage. We can easily do that with .test_variants, have a look at
> > stime tests and testcases/kernel/syscalls/stime/stime_var.h how this is
> > done.
Cyril Hrubis May 6, 2021, 1:58 p.m. UTC | #4
Hi!
> > If we should test _both_ syscall() and the library, I would prefer to split
> > libaio and native aio into two testcases in this testsuite, since <libaio.h>
> > conflicts with <linux/aio_abi.h> during actual modification.
> 
> > Excuse me, is there any other good way to solve this problem?
> NOTE: if most of the test the same, with little help of #ifdef and maybe 1-2
> macros we can have single source for both variants (compiling 2 binaries).
> See setdomainname and sethostname tests (setdomainname.h).

Not sure that it's that easy here, since we cannot include libaio.h and
aio_abi.h in a single file without getting conflicts, they define
structures with the same name with possibly different layout, which
makes this very tricky.

I guess that having separate tests for different interface would
probably be easiest solution in this case.
Petr Vorel May 6, 2021, 6:22 p.m. UTC | #5
> Hi!
> > > If we should test _both_ syscall() and the library, I would prefer to split
> > > libaio and native aio into two testcases in this testsuite, since <libaio.h>
> > > conflicts with <linux/aio_abi.h> during actual modification.

> > > Excuse me, is there any other good way to solve this problem?
> > NOTE: if most of the test the same, with little help of #ifdef and maybe 1-2
> > macros we can have single source for both variants (compiling 2 binaries).
> > See setdomainname and sethostname tests (setdomainname.h).

> Not sure that it's that easy here, since we cannot include libaio.h and
> aio_abi.h in a single file without getting conflicts, they define
> structures with the same name with possibly different layout, which
> makes this very tricky.

> I guess that having separate tests for different interface would
> probably be easiest solution in this case.

Sounds reasonable, thanks for info.

Kind regards,
Petr
Xie Ziyao May 7, 2021, 8:35 a.m. UTC | #6
Hi,

Thanks a lot for your suggestions. I just submitted new testcases for 
native AIO.

Please see: https://patchwork.ozlabs.org/project/ltp/list/?series=242617

And if there are no questions, I'll continue with the following:
1. Convert the original libaio testcases to the new API;
2. Add native AIO testcases for other testsuites in kernel/syscalls.

Thanks very much!

Kind Regards,
Ziyao
diff mbox series

Patch

diff --git a/testcases/kernel/syscalls/io_submit/io_submit01.c b/testcases/kernel/syscalls/io_submit/io_submit01.c
index bbbbc9101..702a7721b 100644
--- a/testcases/kernel/syscalls/io_submit/io_submit01.c
+++ b/testcases/kernel/syscalls/io_submit/io_submit01.c
@@ -1,24 +1,29 @@ 
 // SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright (c) Crackerjack Project., 2007
+ * Ported from Crackerjack to LTP by Masatake YAMATO <yamato@redhat.com>
  * Copyright (c) 2011-2017 Cyril Hrubis <chrubis@suse.cz>
  */

-/* Porting from Crackerjack to LTP is done
-   by Masatake YAMATO <yamato@redhat.com> */
+/*\
+ * [Description]
+ *
+ * - io_submit(2) fails and returns EINVAL if ctx is invalid;
+ * - io_submit(2) fails and returns EINVAL if nr is invalid;
+ * - io_submit(2) fails and returns EFAULT if iocbpp pointer is invalid;
+ * - io_submit(2) fails and returns EBADF if fd is invalid;
+ * - io_submit(2) should work fine if no-op;
+ */

-#include <errno.h>
-#include <string.h>
-#include <fcntl.h>
+#include <linux/aio_abi.h>

 #include "config.h"
 #include "tst_test.h"
+#include "lapi/syscalls.h"

-#ifdef HAVE_LIBAIO
-#include <libaio.h>
-
-static io_context_t ctx;
-static io_context_t invalid_ctx;
+static aio_context_t ctx;
+static aio_context_t invalid_ctx;
+static char buf[100];

 static struct iocb iocb;
 static struct iocb *iocbs[] = {&iocb};
@@ -39,93 +44,74 @@  static struct iocb *zero_buf_iocbs[] = {&zero_buf_iocb};

 static struct iocb *zero_iocbs[1];

-static char buf[100];
-
 static struct tcase {
-	io_context_t *ctx;
+	aio_context_t *ctx;
 	long nr;
 	struct iocb **iocbs;
 	int exp_errno;
 	const char *desc;
-} tcases[] = {
+} tc[] = {
 	/* Invalid ctx */
-	{&invalid_ctx, 1, iocbs, -EINVAL, "invalid ctx"},
+	{&invalid_ctx, 1, iocbs, EINVAL, "invalid ctx"},
 	/* Invalid nr */
-	{&ctx, -1, iocbs, -EINVAL, "invalid nr"},
+	{&ctx, -1, iocbs, EINVAL, "invalid nr"},
 	/* Invalid pointer */
-	{&ctx, 1, (void*)-1, -EFAULT, "invalid iocbpp pointer"},
-	{&ctx, 1, zero_iocbs, -EFAULT, "NULL iocb pointers"},
+	{&ctx, 1, (void*)-1, EFAULT, "invalid iocbpp pointer"},
+	{&ctx, 1, zero_iocbs, EFAULT, "NULL iocb pointers"},
 	/* Invalid fd */
-	{&ctx, 1, inv_fd_iocbs, -EBADF, "invalid fd"},
-	{&ctx, 1, rdonly_fd_iocbs, -EBADF, "readonly fd for write"},
-	{&ctx, 1, wronly_fd_iocbs, -EBADF, "writeonly fd for read"},
+	{&ctx, 1, inv_fd_iocbs, EBADF, "invalid fd"},
+	{&ctx, 1, rdonly_fd_iocbs, EBADF, "readonly fd for write"},
+	{&ctx, 1, wronly_fd_iocbs, EBADF, "writeonly fd for read"},
 	/* No-op but should work fine */
-	{&ctx, 1, zero_buf_iocbs, 1, "zero buf size"},
+	{&ctx, 1, zero_buf_iocbs, 0, "zero buf size"},
 	{&ctx, 0, NULL, 0, "zero nr"},
 };

-static void setup(void)
+static inline void io_prep_option(struct iocb *cb, int fd, void *buf,
+			size_t count, long long offset, unsigned opcode)
 {
-	TEST(io_setup(1, &ctx));
-	if (TST_RET == -ENOSYS)
-		tst_brk(TCONF | TRERRNO, "io_setup(): AIO not supported by kernel");
-	else if (TST_RET)
-		tst_brk(TBROK | TRERRNO, "io_setup() failed");
+	memset(cb, 0, sizeof(*cb));
+	cb->aio_fildes = fd;
+	cb->aio_lio_opcode = opcode;
+	cb->aio_buf = (uint64_t)buf;
+	cb->aio_offset = offset;
+	cb->aio_nbytes = count;
+}

-	io_prep_pread(&inv_fd_iocb, -1, buf, sizeof(buf), 0);
+static void setup(void)
+{
+	TST_EXP_PASS(tst_syscall(__NR_io_setup, 1, &ctx));
+	io_prep_option(&inv_fd_iocb, -1, buf, sizeof(buf), 0, IOCB_CMD_PREAD);

 	rdonly_fd = SAFE_OPEN("rdonly_file", O_RDONLY | O_CREAT, 0777);
-	io_prep_pwrite(&rdonly_fd_iocb, rdonly_fd, buf, sizeof(buf), 0);
+	io_prep_option(&rdonly_fd_iocb, rdonly_fd, buf, sizeof(buf), 0, IOCB_CMD_PWRITE);

-	io_prep_pread(&zero_buf_iocb, rdonly_fd, buf, 0, 0);
+	io_prep_option(&zero_buf_iocb, rdonly_fd, buf, 0, 0, IOCB_CMD_PREAD);

 	wronly_fd = SAFE_OPEN("wronly_file", O_WRONLY | O_CREAT, 0777);
-	io_prep_pread(&wronly_fd_iocb, wronly_fd, buf, sizeof(buf), 0);
+	io_prep_option(&wronly_fd_iocb, wronly_fd, buf, sizeof(buf), 0, IOCB_CMD_PREAD);
 }

 static void cleanup(void)
 {
 	if (rdonly_fd > 0)
 		SAFE_CLOSE(rdonly_fd);
-
 	if (wronly_fd > 0)
 		SAFE_CLOSE(wronly_fd);
 }

-static const char *errno_name(int err)
+static void run(unsigned int i)
 {
-	if (err <= 0)
-		return tst_strerrno(-err);
-
-	return "SUCCESS";
-}
-
-static void verify_io_submit(unsigned int n)
-{
-	struct tcase *t = &tcases[n];
-	int ret;
-
-	ret = io_submit(*t->ctx, t->nr, t->iocbs);
-
-	if (ret == t->exp_errno) {
-		tst_res(TPASS, "io_submit() with %s failed with %s",
-			t->desc, errno_name(t->exp_errno));
-		return;
-	}
-
-	tst_res(TFAIL, "io_submit() returned %i(%s), expected %s(%i)",
-		ret, ret < 0 ? tst_strerrno(-ret) : "SUCCESS",
-		errno_name(t->exp_errno), t->exp_errno);
+	TEST(tst_syscall(__NR_io_submit, *tc[i].ctx, tc[i].nr, tc[i].iocbs));
+	tst_res(TST_ERR == tc[i].exp_errno ? TPASS : TFAIL,
+		"io_submit(2) with %s returns %s, expected %s",
+		tc[i].desc, tst_strerrno(TST_ERR), tst_strerrno(tc[i].exp_errno));
 }

 static struct tst_test test = {
 	.setup = setup,
 	.cleanup = cleanup,
-	.test = verify_io_submit,
-	.tcnt = ARRAY_SIZE(tcases),
+	.test = run,
+	.tcnt = ARRAY_SIZE(tc),
 	.needs_tmpdir = 1,
 };
-
-#else
-	TST_TEST_TCONF("test requires libaio and it's development packages");
-#endif