All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH v1 0/4] *** Add LOOP_CONFIGURE ioctl test ***
@ 2020-06-11 10:35 Yang Xu
  2020-06-11 10:35 ` [LTP] [PATCH v1 1/4] lapi: Add fallback for LOOP_CONFIGURE ioctl and struct loop_config Yang Xu
                   ` (4 more replies)
  0 siblings, 5 replies; 29+ messages in thread
From: Yang Xu @ 2020-06-11 10:35 UTC (permalink / raw)
  To: ltp

Since kernel commit[1], it has added LOOP_CONFIGURE ioctl test.
From this commit, loop_set_fd calls loop_configure with zero
loop_config.

struct loop_config {
	__u32			fd;
	__u32                   block_size;
	struct loop_info64	info;
	__u64			__reserved[8];
}

In addition to doing what LOOP_SET_STATUS can do, LOOP_CONFIGURE can
also be used to:
- Set the correct block size immediately by setting
  loop_config.block_size (I test this in ioctl_loop08.c, like old
ioctl_loop06.c) 
- Explicitly request direct I/O mode by setting LO_FLAGS_DIRECT_IO
  in loop_config.info.lo_flags (I test this in ioctl_loop09.c, like old
ioctl_loop05.c) 
- Explicitly request read-only mode by setting LO_FLAGS_READ_ONLY
  in loop_config.info.lo_flags (I test this in old ioctl_loop02.c)


[1]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3448914e8cc550ba792d4ccc74471d1ca4293a

Yang Xu (4):
  lapi: Add fallback for LOOP_CONFIGURE ioctl and struct loop_config
  syscalls/ioctl_loop02: Using LOOP_CONFIGURE to set read_only
  syscalls/ioctl_loop08: Add LOOP_CONFIGURE error test with invalid
    block size
  syscalls/ioctl_loop09: Add LOOP_CONFIGURE ioctl test with direct I/O
    flag

 configure.ac                                  |   1 +
 include/lapi/loop.h                           |  23 +++
 runtest/syscalls                              |   2 +
 testcases/kernel/syscalls/ioctl/.gitignore    |   2 +
 .../kernel/syscalls/ioctl/ioctl_loop02.c      |  53 ++++--
 .../kernel/syscalls/ioctl/ioctl_loop08.c      | 101 ++++++++++++
 .../kernel/syscalls/ioctl/ioctl_loop09.c      | 151 ++++++++++++++++++
 7 files changed, 324 insertions(+), 9 deletions(-)
 create mode 100644 testcases/kernel/syscalls/ioctl/ioctl_loop08.c
 create mode 100644 testcases/kernel/syscalls/ioctl/ioctl_loop09.c

-- 
2.23.0




^ permalink raw reply	[flat|nested] 29+ messages in thread
* [LTP] [PATCH v1 1/4] lapi: Add fallback for LOOP_CONFIGURE ioctl and struct loop_config
  2020-06-11 10:35 [LTP] [PATCH v1 0/4] *** Add LOOP_CONFIGURE ioctl test *** Yang Xu
@ 2020-06-11 10:35 ` Yang Xu
  2020-07-08 13:18   ` Cyril Hrubis
  2020-06-11 10:35 ` [LTP] [PATCH v1 2/4] syscalls/ioctl_loop02: Using LOOP_CONFIGURE to set read_only Yang Xu
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 29+ messages in thread
From: Yang Xu @ 2020-06-11 10:35 UTC (permalink / raw)
  To: ltp

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
 configure.ac        |  1 +
 include/lapi/loop.h | 23 +++++++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/configure.ac b/configure.ac
index 1d3ea58d0..7d2be1df7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -150,6 +150,7 @@ AC_CHECK_TYPES([struct file_dedupe_range],,,[#include <linux/fs.h>])
 AC_CHECK_TYPES([struct fs_quota_statv],,,[#include <xfs/xqm.h>])
 AC_CHECK_TYPES([struct if_nextdqblk],,,[#include <linux/quota.h>])
 AC_CHECK_TYPES([struct iovec],,,[#include <sys/uio.h>])
+AC_CHECK_TYPES([struct loop_config],,,[#include <linux/loop.h>])
 
 AC_CHECK_TYPES([struct mmsghdr],,,[
 #define _GNU_SOURCE
diff --git a/include/lapi/loop.h b/include/lapi/loop.h
index c69328613..87a902317 100644
--- a/include/lapi/loop.h
+++ b/include/lapi/loop.h
@@ -6,6 +6,7 @@
 #ifndef LAPI_LOOP_H
 #define LAPI_LOOP_H
 
+#include "config.h"
 #include <linux/types.h>
 #include <linux/loop.h>
 
@@ -29,4 +30,26 @@
 # define LOOP_SET_BLOCK_SIZE 0x4C09
 #endif
 
+#ifndef LOOP_CONFIGURE
+# define LOOP_CONFIGURE 0x4C0A
+#endif
+
+#ifndef HAVE_STRUCT_LOOP_CONFIG
+/*
+ * struct loop_config - Complete configuration for a loop device.
+ * @fd: fd of the file to be used as a backing file for the loop device.
+ * @block_size: block size to use; ignored if 0.
+ * @info: struct loop_info64 to configure the loop device with.
+ *
+ * This structure is used with the LOOP_CONFIGURE ioctl, and can be used to
+ * atomically setup and configure all loop device parameters at once.
+ */
+struct loop_config {
+	__u32			fd;
+	__u32                   block_size;
+	struct loop_info64	info;
+	__u64			__reserved[8];
+};
+#endif
+
 #endif
-- 
2.23.0




^ permalink raw reply related	[flat|nested] 29+ messages in thread
* [LTP] [PATCH v1 2/4] syscalls/ioctl_loop02: Using LOOP_CONFIGURE to set read_only
  2020-06-11 10:35 [LTP] [PATCH v1 0/4] *** Add LOOP_CONFIGURE ioctl test *** Yang Xu
  2020-06-11 10:35 ` [LTP] [PATCH v1 1/4] lapi: Add fallback for LOOP_CONFIGURE ioctl and struct loop_config Yang Xu
@ 2020-06-11 10:35 ` Yang Xu
  2020-07-08 13:15   ` Cyril Hrubis
  2020-06-11 10:35 ` [LTP] [PATCH v1 3/4] syscalls/ioctl_loop08: Add LOOP_CONFIGURE error test with invalid block size Yang Xu
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 29+ messages in thread
From: Yang Xu @ 2020-06-11 10:35 UTC (permalink / raw)
  To: ltp

Since kernel commit 3448914e8cc5("loop: Add LOOP_CONFIGURE ioctl"),
it can explicitly request read-only mode by setting LO_FLAGS_READ_ONLY
in loop_config.info.lo_flags regardless of backing file open mode.

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
 .../kernel/syscalls/ioctl/ioctl_loop02.c      | 53 +++++++++++++++----
 1 file changed, 44 insertions(+), 9 deletions(-)

diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop02.c b/testcases/kernel/syscalls/ioctl/ioctl_loop02.c
index c43172ca1..c497017f5 100644
--- a/testcases/kernel/syscalls/ioctl/ioctl_loop02.c
+++ b/testcases/kernel/syscalls/ioctl/ioctl_loop02.c
@@ -11,6 +11,9 @@
  * For LOOP_CHANGE_FD, this operation is possible only if the loop device
  * is read-only and the new backing store is the same size and type as the
  * old backing store.
+ *
+ * If using LOOP_CONFIGURE ioctl, we can set LO_FLAGS_READ_ONLY
+ * flag even though backing file with write mode.
  */
 
 #include <stdio.h>
@@ -22,15 +25,39 @@
 
 static int file_fd, file_change_fd, file_fd_invalid;
 static char backing_path[1024], backing_file_path[1024], backing_file_change_path[1024];
-static int attach_flag, dev_fd, file_fd;
+static int attach_flag, dev_fd, loop_configure_sup = 1;
 static char loop_ro_path[1024], dev_path[1024];
+static struct loop_config loopconfig;
+
+static struct tcase {
+	int mode;
+	int ioctl;
+	char *message;
+} tcases[] = {
+	{O_RDONLY, LOOP_SET_FD, "Using LOOP_SET_FD to setup loopdevice"},
+	{O_RDWR, LOOP_CONFIGURE, "Using LOOP_CONFIGURE with read_only flag to setup loopdevice"},
+};
 
-static void verify_ioctl_loop(void)
+static void verify_ioctl_loop(unsigned int n)
 {
+	struct tcase *tc = &tcases[n];
 	struct loop_info loopinfoget;
 
+	tst_res(TINFO, "%s", tc->message);
+	file_fd = SAFE_OPEN("test.img", tc->mode);
 	dev_fd = SAFE_OPEN(dev_path, O_RDWR);
-	SAFE_IOCTL(dev_fd, LOOP_SET_FD, file_fd);
+
+	if (tc->ioctl == LOOP_SET_FD) {
+		SAFE_IOCTL(dev_fd, LOOP_SET_FD, file_fd);
+	} else {
+		if (!loop_configure_sup) {
+			tst_res(TCONF,
+				"Current environmet doesn't support LOOP_CONFIGURE ioctl, skip this");
+			return;
+		}
+		loopconfig.fd = file_fd;
+		SAFE_IOCTL(dev_fd, LOOP_CONFIGURE, &loopconfig);
+	}
 	attach_flag = 1;
 
 	TST_ASSERT_INT(loop_ro_path, 1);
@@ -38,11 +65,6 @@ static void verify_ioctl_loop(void)
 
 	memset(&loopinfoget, 0, sizeof(loopinfoget));
 
-	/*
-	 * In drivers/block/loop.c code, set status function doesn't handle
-	 * LO_FLAGS_READ_ONLY flag and ignore it. Only loop_set_fd with readonly
-	 * mode, lo_flags will include LO_FLAGS_READ_ONLY.
-	 */
 	SAFE_IOCTL(dev_fd, LOOP_GET_STATUS, &loopinfoget);
 
 	if (loopinfoget.lo_flags & ~LO_FLAGS_READ_ONLY)
@@ -83,6 +105,7 @@ static void verify_ioctl_loop(void)
 static void setup(void)
 {
 	int dev_num;
+	int ret;
 
 	char *tmpdir = tst_get_tmpdir();
 	dev_num = tst_find_free_loopdev(dev_path, sizeof(dev_path));
@@ -103,6 +126,17 @@ static void setup(void)
 	file_fd = SAFE_OPEN("test.img", O_RDONLY);
 	file_change_fd = SAFE_OPEN("test1.img", O_RDWR);
 	file_fd_invalid = SAFE_OPEN("test2.img", O_RDWR);
+
+	dev_fd = SAFE_OPEN(dev_path, O_RDWR);
+	loopconfig.fd = -1;
+	ret = ioctl(dev_fd, LOOP_CONFIGURE, &loopconfig);
+
+	if (ret && errno != EBADF) {
+		tst_res(TINFO | TERRNO, "LOOP_CONFIGURE is not supported");
+		loop_configure_sup = 0;
+	}
+	loopconfig.info.lo_flags = LO_FLAGS_READ_ONLY;
+	SAFE_CLOSE(dev_fd);
 }
 
 static void cleanup(void)
@@ -122,7 +156,8 @@ static void cleanup(void)
 static struct tst_test test = {
 	.setup = setup,
 	.cleanup = cleanup,
-	.test_all = verify_ioctl_loop,
+	.tcnt = ARRAY_SIZE(tcases),
+	.test = verify_ioctl_loop,
 	.needs_root = 1,
 	.needs_tmpdir = 1,
 	.needs_drivers = (const char *const []) {
-- 
2.23.0




^ permalink raw reply related	[flat|nested] 29+ messages in thread
* [LTP] [PATCH v1 3/4] syscalls/ioctl_loop08: Add LOOP_CONFIGURE error test with invalid block size
  2020-06-11 10:35 [LTP] [PATCH v1 0/4] *** Add LOOP_CONFIGURE ioctl test *** Yang Xu
  2020-06-11 10:35 ` [LTP] [PATCH v1 1/4] lapi: Add fallback for LOOP_CONFIGURE ioctl and struct loop_config Yang Xu
  2020-06-11 10:35 ` [LTP] [PATCH v1 2/4] syscalls/ioctl_loop02: Using LOOP_CONFIGURE to set read_only Yang Xu
@ 2020-06-11 10:35 ` Yang Xu
  2020-07-08 14:00   ` Cyril Hrubis
  2020-06-11 10:35 ` [LTP] [PATCH v1 4/4] syscalls/ioctl_loop09: Add LOOP_CONFIGURE ioctl test with direct I/O flag Yang Xu
  2020-07-06  1:45 ` [LTP] [PATCH v1 0/4] *** Add LOOP_CONFIGURE ioctl test *** Yang Xu
  4 siblings, 1 reply; 29+ messages in thread
From: Yang Xu @ 2020-06-11 10:35 UTC (permalink / raw)
  To: ltp

Since kernel commit 3448914e8cc5("loop: Add LOOP_CONFIGURE ioctl"),
it can set the correct block size immediately by setting loop_config.block_size.

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
 runtest/syscalls                              |   1 +
 testcases/kernel/syscalls/ioctl/.gitignore    |   1 +
 .../kernel/syscalls/ioctl/ioctl_loop08.c      | 101 ++++++++++++++++++
 3 files changed, 103 insertions(+)
 create mode 100644 testcases/kernel/syscalls/ioctl/ioctl_loop08.c

diff --git a/runtest/syscalls b/runtest/syscalls
index cd0c65094..7259f2a92 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -536,6 +536,7 @@ ioctl_loop04 ioctl_loop04
 ioctl_loop05 ioctl_loop05
 ioctl_loop06 ioctl_loop06
 ioctl_loop07 ioctl_loop07
+ioctl_loop08 ioctl_loop08
 
 ioctl_ns01 ioctl_ns01
 ioctl_ns02 ioctl_ns02
diff --git a/testcases/kernel/syscalls/ioctl/.gitignore b/testcases/kernel/syscalls/ioctl/.gitignore
index 3a3d49adc..97134aa0b 100644
--- a/testcases/kernel/syscalls/ioctl/.gitignore
+++ b/testcases/kernel/syscalls/ioctl/.gitignore
@@ -13,6 +13,7 @@
 /ioctl_loop05
 /ioctl_loop06
 /ioctl_loop07
+/ioctl_loop08
 /ioctl_ns01
 /ioctl_ns02
 /ioctl_ns03
diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop08.c b/testcases/kernel/syscalls/ioctl/ioctl_loop08.c
new file mode 100644
index 000000000..93b75a381
--- /dev/null
+++ b/testcases/kernel/syscalls/ioctl/ioctl_loop08.c
@@ -0,0 +1,101 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 FUJITSU LIMITED. All rights reserved.
+ * Author: Yang Xu <xuyang2018.jy@cn.jujitsu.com>
+ *
+ * This is a basic ioctl test about loopdevice.
+ * It is designed to test LOOP_CONFIGURE ioctl with invalid block size config.
+ * It is similar with ioctl_loop06.c.
+ */
+
+#include <stdio.h>
+#include <unistd.h>
+#include <sys/types.h>
+#include <stdlib.h>
+#include "lapi/loop.h"
+#include "tst_test.h"
+
+static char dev_path[1024];
+static int dev_num, dev_fd, file_fd;
+static unsigned int invalid_value, half_value, unalign_value;
+static struct loop_config loopconfig;
+
+static struct tcase {
+	unsigned int *setvalue;
+	int exp_err;
+	char *message;
+} tcases[] = {
+	{&half_value, EINVAL, "arg < 512"},
+	{&invalid_value, EINVAL, "arg > PAGE_SIZE"},
+	{&unalign_value, EINVAL, "arg != power_of_2"},
+};
+
+static void verify_ioctl_loop(unsigned int n)
+{
+	struct tcase *tc = &tcases[n];
+
+	tst_res(TINFO, "%s", tc->message);
+	loopconfig.block_size = *(tc->setvalue);
+	TEST(ioctl(dev_fd, LOOP_CONFIGURE, &loopconfig));
+	if (TST_RET == 0) {
+		tst_res(TFAIL, "LOOP_CONFIGURE block_size succeed unexpectedly");
+		TST_RETRY_FUNC(ioctl(dev_fd, LOOP_CLR_FD, 0), TST_RETVAL_EQ0);
+		return;
+	}
+
+	if (TST_ERR == tc->exp_err) {
+		tst_res(TPASS | TTERRNO, "LOOP_CONFIGURE block failed as expected");
+	} else {
+		tst_res(TFAIL | TTERRNO, "LOOP_CONFIGURE block size failed expected %s got",
+				tst_strerrno(tc->exp_err));
+	}
+}
+
+static void setup(void)
+{
+	unsigned int pg_size;
+	int ret;
+
+	dev_num = tst_find_free_loopdev(dev_path, sizeof(dev_path));
+	if (dev_num < 0)
+		tst_brk(TBROK, "Failed to find free loop device");
+
+	tst_fill_file("test.img", 0, 1024, 1024);
+	half_value = 256;
+	pg_size = getpagesize();
+	invalid_value = pg_size * 2 ;
+	unalign_value = pg_size - 1;
+
+	dev_fd = SAFE_OPEN(dev_path, O_RDWR);
+	file_fd = SAFE_OPEN("test.img", O_RDWR);
+	loopconfig.fd = file_fd;
+
+	ret = ioctl(dev_fd, LOOP_CONFIGURE, &loopconfig);
+	if (ret == 0) {
+		TST_RETRY_FUNC(ioctl(dev_fd, LOOP_CLR_FD, 0), TST_RETVAL_EQ0);
+		return;
+	}
+	if (errno == EINVAL)
+		tst_brk(TCONF, "LOOP_CONFIGURE is not supported");
+}
+
+static void cleanup(void)
+{
+	if (dev_fd > 0)
+		SAFE_CLOSE(dev_fd);
+	if (file_fd > 0)
+		SAFE_CLOSE(file_fd);
+}
+
+static struct tst_test test = {
+	.setup = setup,
+	.cleanup = cleanup,
+	.test = verify_ioctl_loop,
+	.tcnt = ARRAY_SIZE(tcases),
+	.needs_root = 1,
+	.needs_tmpdir = 1,
+	.needs_drivers = (const char *const []) {
+		"loop",
+		NULL
+	}
+};
-- 
2.23.0




^ permalink raw reply related	[flat|nested] 29+ messages in thread
* [LTP] [PATCH v1 4/4] syscalls/ioctl_loop09: Add LOOP_CONFIGURE ioctl test with direct I/O flag
  2020-06-11 10:35 [LTP] [PATCH v1 0/4] *** Add LOOP_CONFIGURE ioctl test *** Yang Xu
                   ` (2 preceding siblings ...)
  2020-06-11 10:35 ` [LTP] [PATCH v1 3/4] syscalls/ioctl_loop08: Add LOOP_CONFIGURE error test with invalid block size Yang Xu
@ 2020-06-11 10:35 ` Yang Xu
  2020-07-08 14:00   ` Cyril Hrubis
  2020-07-06  1:45 ` [LTP] [PATCH v1 0/4] *** Add LOOP_CONFIGURE ioctl test *** Yang Xu
  4 siblings, 1 reply; 29+ messages in thread
From: Yang Xu @ 2020-06-11 10:35 UTC (permalink / raw)
  To: ltp

Since kernel commit 3448914e8cc5("loop: Add LOOP_CONFIGURE ioctl"),
it can explicitly request direct I/O mode by setting LO_FLAGS_DIRECT_IO
in loop_config.info.lo_flags.

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
 runtest/syscalls                              |   1 +
 testcases/kernel/syscalls/ioctl/.gitignore    |   1 +
 .../kernel/syscalls/ioctl/ioctl_loop09.c      | 151 ++++++++++++++++++
 3 files changed, 153 insertions(+)
 create mode 100644 testcases/kernel/syscalls/ioctl/ioctl_loop09.c

diff --git a/runtest/syscalls b/runtest/syscalls
index 7259f2a92..df99a0600 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -537,6 +537,7 @@ ioctl_loop05 ioctl_loop05
 ioctl_loop06 ioctl_loop06
 ioctl_loop07 ioctl_loop07
 ioctl_loop08 ioctl_loop08
+ioctl_loop09 ioctl_loop09
 
 ioctl_ns01 ioctl_ns01
 ioctl_ns02 ioctl_ns02
diff --git a/testcases/kernel/syscalls/ioctl/.gitignore b/testcases/kernel/syscalls/ioctl/.gitignore
index 97134aa0b..0decf6ea1 100644
--- a/testcases/kernel/syscalls/ioctl/.gitignore
+++ b/testcases/kernel/syscalls/ioctl/.gitignore
@@ -14,6 +14,7 @@
 /ioctl_loop06
 /ioctl_loop07
 /ioctl_loop08
+/ioctl_loop09
 /ioctl_ns01
 /ioctl_ns02
 /ioctl_ns03
diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop09.c b/testcases/kernel/syscalls/ioctl/ioctl_loop09.c
new file mode 100644
index 000000000..c291afeb6
--- /dev/null
+++ b/testcases/kernel/syscalls/ioctl/ioctl_loop09.c
@@ -0,0 +1,151 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2020 FUJITSU LIMITED. All rights reserved.
+ * Author: Yang Xu <xuyang2018.jy@cn.jujitsu.com>
+ *
+ * This is a basic ioctl test about loopdevice. It sets LO_FLAGS_DIRECT_IO
+ * in loop_config.info.lo_flags by using LOOP_CONFIGURE instead of using
+ * LOOP_SET_DIRECT_IO. It is similar with ioctl_loop05.c.
+ */
+
+#include <stdio.h>
+#include <unistd.h>
+#include <string.h>
+#include <stdlib.h>
+#include <sys/mount.h>
+#include "lapi/loop.h"
+#include "tst_test.h"
+#include "tst_safe_stdio.h"
+
+#define DIO_MESSAGE "In dio mode"
+#define NON_DIO_MESSAGE "In non dio mode"
+
+static char dev_path[1024], sys_loop_diopath[1024];
+static int dev_num, dev_fd, file_fd, block_devfd, logical_block_size;
+static struct loop_config loopconfig;
+static struct tcase {
+	int multi; /*logical_block_size / 2 as unit */
+	int dio_value;
+	char *message;
+} tcases[] = {
+	{0, 1, "Without setting lo_offset or sizelimit"},
+	{2, 1, "With offset equal to logical_block_size"},
+	{1, 0, "less than logical_block_size"},
+};
+
+static void check_dio_value(int flag)
+{
+	struct loop_info loopinfoget;
+
+	memset(&loopinfoget, 0, sizeof(loopinfoget));
+
+	SAFE_IOCTL(dev_fd, LOOP_GET_STATUS, &loopinfoget);
+	tst_res(TINFO, "%s", flag ? DIO_MESSAGE : NON_DIO_MESSAGE);
+
+	if (loopinfoget.lo_flags & LO_FLAGS_DIRECT_IO)
+		tst_res(flag ? TPASS : TFAIL, "lo_flags has LO_FLAGS_DIRECT_IO flag");
+	else
+		tst_res(flag ? TFAIL : TPASS, "lo_flags doesn't have LO_FLAGS_DIRECT_IO flag");
+
+	TST_ASSERT_INT(sys_loop_diopath, flag);
+}
+
+static void find_backing_bdpath(char *buf)
+{
+	const char *const df_cmd[] = {"df", "-T", ".", NULL};
+	char line[PATH_MAX];
+	FILE *file;
+
+	SAFE_CMD(df_cmd, "1.txt", NULL);
+	file = SAFE_FOPEN("1.txt", "r");
+
+	while (fgets(line, sizeof(line), file) != NULL) {
+		sscanf(line, "%s", buf);
+		if (strstr(buf, "/dev/") != NULL)
+			break;
+	}
+	SAFE_FCLOSE(file);
+}
+
+static void verify_ioctl_loop(unsigned int n)
+{
+	struct tcase *tc = &tcases[n];
+
+	loopconfig.info.lo_offset = tc->multi * logical_block_size / 2;
+
+	tst_res(TINFO, "%s", tc->message);
+	/*
+	 * When we call loop_configure ioctl successfully and detach it,
+	 * the subquent loo_configure without non-zero lo_offset or sizelimit
+	 * may trigger the blk_update_request I/O error. To avoid this, sleep
+	 * 1s to ensure last blk_update_request has completed.
+	 */
+	sleep(1);
+
+	/*
+	 * loop_cofigure calls loop_update_dio() function, it will ignore the result
+	 * of setting dio. It is different from loop_set_dio.
+	 */
+	TST_RETRY_FUNC(ioctl(dev_fd, LOOP_CONFIGURE, &loopconfig), TST_RETVAL_EQ0);
+	check_dio_value(tc->dio_value);
+	TST_RETRY_FUNC(ioctl(dev_fd, LOOP_CLR_FD, 0), TST_RETVAL_EQ0);
+}
+
+static void setup(void)
+{
+	int ret;
+	char buf[100];
+
+	if (tst_fs_type(".") == TST_TMPFS_MAGIC)
+		tst_brk(TCONF, "tmpfd doesn't support O_DIRECT flag");
+
+	dev_num = tst_find_free_loopdev(dev_path, sizeof(dev_path));
+	if (dev_num < 0)
+		tst_brk(TBROK, "Failed to find free loop device");
+
+	sprintf(sys_loop_diopath, "/sys/block/loop%d/loop/dio", dev_num);
+	tst_fill_file("test.img", 0, 1024, 1024);
+	dev_fd = SAFE_OPEN(dev_path, O_RDWR);
+	file_fd = SAFE_OPEN("test.img", O_RDWR);
+
+	find_backing_bdpath(buf);
+	block_devfd = SAFE_OPEN(buf, O_RDWR);
+
+	SAFE_IOCTL(block_devfd, BLKSSZGET, &logical_block_size);
+	tst_res(TINFO, "backing dev logical_block_size is %d", logical_block_size);
+	SAFE_CLOSE(block_devfd);
+	loopconfig.fd = -1;
+	ret = ioctl(dev_fd, LOOP_CONFIGURE, &loopconfig);
+
+	if (ret && errno != EBADF)
+		tst_brk(TCONF | TERRNO, "LOOP_CONFIGURE is not supported");
+	loopconfig.fd = file_fd;
+	loopconfig.info.lo_flags = LO_FLAGS_DIRECT_IO;
+}
+
+static void cleanup(void)
+{
+	if (dev_fd > 0)
+		SAFE_CLOSE(dev_fd);
+	if (block_devfd > 0)
+		SAFE_CLOSE(block_devfd);
+	if (file_fd > 0)
+		SAFE_CLOSE(file_fd);
+}
+
+static struct tst_test test = {
+	.setup = setup,
+	.cleanup = cleanup,
+	.tcnt = ARRAY_SIZE(tcases),
+	.test = verify_ioctl_loop,
+	.needs_root = 1,
+	.needs_tmpdir = 1,
+	.needs_drivers = (const char *const []) {
+		"loop",
+		NULL
+	},
+	.needs_cmds = (const char *const []) {
+		"df",
+		NULL
+	}
+};
-- 
2.23.0




^ permalink raw reply related	[flat|nested] 29+ messages in thread
* [LTP] [PATCH v1 0/4] *** Add LOOP_CONFIGURE ioctl test ***
  2020-06-11 10:35 [LTP] [PATCH v1 0/4] *** Add LOOP_CONFIGURE ioctl test *** Yang Xu
                   ` (3 preceding siblings ...)
  2020-06-11 10:35 ` [LTP] [PATCH v1 4/4] syscalls/ioctl_loop09: Add LOOP_CONFIGURE ioctl test with direct I/O flag Yang Xu
@ 2020-07-06  1:45 ` Yang Xu
  4 siblings, 0 replies; 29+ messages in thread
From: Yang Xu @ 2020-07-06  1:45 UTC (permalink / raw)
  To: ltp

Hi
ping.

> Since kernel commit[1], it has added LOOP_CONFIGURE ioctl test.
>  From this commit, loop_set_fd calls loop_configure with zero
> loop_config.
> 
> struct loop_config {
> 	__u32			fd;
> 	__u32                   block_size;
> 	struct loop_info64	info;
> 	__u64			__reserved[8];
> }
> 
> In addition to doing what LOOP_SET_STATUS can do, LOOP_CONFIGURE can
> also be used to:
> - Set the correct block size immediately by setting
>    loop_config.block_size (I test this in ioctl_loop08.c, like old
> ioctl_loop06.c)
> - Explicitly request direct I/O mode by setting LO_FLAGS_DIRECT_IO
>    in loop_config.info.lo_flags (I test this in ioctl_loop09.c, like old
> ioctl_loop05.c)
> - Explicitly request read-only mode by setting LO_FLAGS_READ_ONLY
>    in loop_config.info.lo_flags (I test this in old ioctl_loop02.c)
> 
> 
> [1]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3448914e8cc550ba792d4ccc74471d1ca4293a
> 
> Yang Xu (4):
>    lapi: Add fallback for LOOP_CONFIGURE ioctl and struct loop_config
>    syscalls/ioctl_loop02: Using LOOP_CONFIGURE to set read_only
>    syscalls/ioctl_loop08: Add LOOP_CONFIGURE error test with invalid
>      block size
>    syscalls/ioctl_loop09: Add LOOP_CONFIGURE ioctl test with direct I/O
>      flag
> 
>   configure.ac                                  |   1 +
>   include/lapi/loop.h                           |  23 +++
>   runtest/syscalls                              |   2 +
>   testcases/kernel/syscalls/ioctl/.gitignore    |   2 +
>   .../kernel/syscalls/ioctl/ioctl_loop02.c      |  53 ++++--
>   .../kernel/syscalls/ioctl/ioctl_loop08.c      | 101 ++++++++++++
>   .../kernel/syscalls/ioctl/ioctl_loop09.c      | 151 ++++++++++++++++++
>   7 files changed, 324 insertions(+), 9 deletions(-)
>   create mode 100644 testcases/kernel/syscalls/ioctl/ioctl_loop08.c
>   create mode 100644 testcases/kernel/syscalls/ioctl/ioctl_loop09.c
> 



^ permalink raw reply	[flat|nested] 29+ messages in thread
* [LTP] [PATCH v1 2/4] syscalls/ioctl_loop02: Using LOOP_CONFIGURE to set read_only
  2020-06-11 10:35 ` [LTP] [PATCH v1 2/4] syscalls/ioctl_loop02: Using LOOP_CONFIGURE to set read_only Yang Xu
@ 2020-07-08 13:15   ` Cyril Hrubis
  0 siblings, 0 replies; 29+ messages in thread
From: Cyril Hrubis @ 2020-07-08 13:15 UTC (permalink / raw)
  To: ltp

Hi!
Pushed with a few fixed, thanks.

* Shortened some overly long messages

* Fixed the code not to leak open file descriptors
  - removed useless open of file_fd in setup
  - moved TCONF check in run before fds are opened
  - added close for file_fd at the end of the run

 (this fixes runs with -i N especially on older kernels)

diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop02.c b/testcases/kernel/syscalls/ioctl/ioctl_loop02.c
index 549154fa1..3a03d052a 100644
--- a/testcases/kernel/syscalls/ioctl/ioctl_loop02.c
+++ b/testcases/kernel/syscalls/ioctl/ioctl_loop02.c
@@ -35,7 +35,7 @@ static struct tcase {
 	char *message;
 } tcases[] = {
 	{O_RDONLY, LOOP_SET_FD, "Using LOOP_SET_FD to setup loopdevice"},
-	{O_RDWR, LOOP_CONFIGURE, "Using LOOP_CONFIGURE with read_only flag to setup loopdevice"},
+	{O_RDWR, LOOP_CONFIGURE, "Using LOOP_CONFIGURE with read_only flag"},
 };
 
 static void verify_ioctl_loop(unsigned int n)
@@ -43,6 +43,11 @@ static void verify_ioctl_loop(unsigned int n)
 	struct tcase *tc = &tcases[n];
 	struct loop_info loopinfoget;
 
+	if (tc->ioctl == LOOP_CONFIGURE && !loop_configure_sup) {
+		tst_res(TCONF, "LOOP_CONFIGURE ioctl not supported");
+		return;
+	}
+
 	tst_res(TINFO, "%s", tc->message);
 	file_fd = SAFE_OPEN("test.img", tc->mode);
 	dev_fd = SAFE_OPEN(dev_path, O_RDWR);
@@ -50,11 +55,6 @@ static void verify_ioctl_loop(unsigned int n)
 	if (tc->ioctl == LOOP_SET_FD) {
 		SAFE_IOCTL(dev_fd, LOOP_SET_FD, file_fd);
 	} else {
-		if (!loop_configure_sup) {
-			tst_res(TCONF,
-				"Current environmet doesn't support LOOP_CONFIGURE ioctl, skip this");
-			return;
-		}
 		loopconfig.fd = file_fd;
 		SAFE_IOCTL(dev_fd, LOOP_CONFIGURE, &loopconfig);
 	}
@@ -98,6 +98,7 @@ static void verify_ioctl_loop(unsigned int n)
 	}
 
 	SAFE_CLOSE(dev_fd);
+	SAFE_CLOSE(file_fd);
 	tst_detach_device(dev_path);
 	attach_flag = 0;
 }
@@ -123,7 +124,6 @@ static void setup(void)
 
 	free(tmpdir);
 
-	file_fd = SAFE_OPEN("test.img", O_RDONLY);
 	file_change_fd = SAFE_OPEN("test1.img", O_RDWR);
 	file_fd_invalid = SAFE_OPEN("test2.img", O_RDWR);
 

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply related	[flat|nested] 29+ messages in thread
* [LTP] [PATCH v1 1/4] lapi: Add fallback for LOOP_CONFIGURE ioctl and struct loop_config
  2020-06-11 10:35 ` [LTP] [PATCH v1 1/4] lapi: Add fallback for LOOP_CONFIGURE ioctl and struct loop_config Yang Xu
@ 2020-07-08 13:18   ` Cyril Hrubis
  0 siblings, 0 replies; 29+ messages in thread
From: Cyril Hrubis @ 2020-07-08 13:18 UTC (permalink / raw)
  To: ltp

Hi!
Pushed, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 29+ messages in thread
* [LTP] [PATCH v1 3/4] syscalls/ioctl_loop08: Add LOOP_CONFIGURE error test with invalid block size
  2020-06-11 10:35 ` [LTP] [PATCH v1 3/4] syscalls/ioctl_loop08: Add LOOP_CONFIGURE error test with invalid block size Yang Xu
@ 2020-07-08 14:00   ` Cyril Hrubis
  0 siblings, 0 replies; 29+ messages in thread
From: Cyril Hrubis @ 2020-07-08 14:00 UTC (permalink / raw)
  To: ltp

Hi!
> + * This is a basic ioctl test about loopdevice.
> + * It is designed to test LOOP_CONFIGURE ioctl with invalid block size config.
> + * It is similar with ioctl_loop06.c.
> + */

Actually wouldn't it be easier to just add this to the ioctl_loop06.c?

I guess that we can have a run() function that would execute the
verify_ioctl_loop() with n and also pass flag down which ioctl to use.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 29+ messages in thread
* [LTP] [PATCH v1 4/4] syscalls/ioctl_loop09: Add LOOP_CONFIGURE ioctl test with direct I/O flag
  2020-06-11 10:35 ` [LTP] [PATCH v1 4/4] syscalls/ioctl_loop09: Add LOOP_CONFIGURE ioctl test with direct I/O flag Yang Xu
@ 2020-07-08 14:00   ` Cyril Hrubis
  2020-07-10  6:39     ` [LTP] [PATCH v2 1/2] syscalls/ioctl_loop06: Using LOOP_CONFIGURE to test invalid block size Yang Xu
  0 siblings, 1 reply; 29+ messages in thread
From: Cyril Hrubis @ 2020-07-08 14:00 UTC (permalink / raw)
  To: ltp

Hi!
And here as well.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 29+ messages in thread
* [LTP] [PATCH v2 1/2] syscalls/ioctl_loop06: Using LOOP_CONFIGURE to test invalid block size
  2020-07-08 14:00   ` Cyril Hrubis
@ 2020-07-10  6:39     ` Yang Xu
  2020-07-10  6:39       ` [LTP] [PATCH v2 2/2] syscalls/ioctl_loop05: Using LOOP_CONFIGURE to set direct io Yang Xu
  2020-07-22  9:45       ` [LTP] [PATCH v2 1/2] syscalls/ioctl_loop06: Using LOOP_CONFIGURE to test invalid block size Cyril Hrubis
  0 siblings, 2 replies; 29+ messages in thread
From: Yang Xu @ 2020-07-10  6:39 UTC (permalink / raw)
  To: ltp

Since kernel commit 3448914e8cc5("loop: Add LOOP_CONFIGURE ioctl"),
it can set the correct block size immediately by setting loop_config.block_size.

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
 .../kernel/syscalls/ioctl/ioctl_loop06.c      | 89 +++++++++++++++----
 1 file changed, 73 insertions(+), 16 deletions(-)

diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop06.c b/testcases/kernel/syscalls/ioctl/ioctl_loop06.c
index 096ec9363..2f172a09d 100644
--- a/testcases/kernel/syscalls/ioctl/ioctl_loop06.c
+++ b/testcases/kernel/syscalls/ioctl/ioctl_loop06.c
@@ -3,8 +3,8 @@
  * Copyright (c) 2020 FUJITSU LIMITED. All rights reserved.
  * Author: Yang Xu <xuyang2018.jy@cn.jujitsu.com>
  *
- * This is a basic ioctl error test about loopdevice
- * LOOP_SET_BLOCK_SIZE.
+ * This is a basic error test about the invalid block size of loopdevice
+ * by using LOOP_SET_BLOCK_SIZE or LOOP_CONFIGURE ioctl.
  */
 
 #include <stdio.h>
@@ -15,41 +15,86 @@
 #include "tst_test.h"
 
 static char dev_path[1024];
-static int dev_num, dev_fd, attach_flag;
+static int dev_num, dev_fd, file_fd, attach_flag, loop_configure_sup = 1;
 static unsigned int invalid_value, half_value, unalign_value;
+static struct loop_config loopconfig;
 
 static struct tcase {
 	unsigned int *setvalue;
-	int exp_err;
+	int ioctl_flag;
 	char *message;
 } tcases[] = {
-	{&half_value, EINVAL, "arg < 512"},
-	{&invalid_value, EINVAL, "arg > PAGE_SIZE"},
-	{&unalign_value, EINVAL, "arg != power_of_2"},
+	{&half_value, LOOP_SET_BLOCK_SIZE,
+	"Using LOOP_SET_BLOCK_SIZE with arg < 512"},
+
+	{&invalid_value, LOOP_SET_BLOCK_SIZE,
+	"Using LOOP_SET_BLOCK_SIZE with arg > PAGE_SIZE"},
+
+	{&unalign_value, LOOP_SET_BLOCK_SIZE,
+	"Using LOOP_SET_BLOCK_SIZE with arg != power_of_2"},
+
+	{&half_value, LOOP_CONFIGURE,
+	"Using LOOP_CONFIGURE with block_size < 512"},
+
+	{&invalid_value, LOOP_CONFIGURE,
+	"Using LOOP_CONFIGURE with block_size > PAGE_SIZE"},
+
+	{&unalign_value, LOOP_CONFIGURE,
+	"Using LOOP_CONFIGURE with block_size != power_of_2"},
 };
 
 static void verify_ioctl_loop(unsigned int n)
+{
+	if (tcases[n].ioctl_flag == LOOP_CONFIGURE)
+		TEST(ioctl(dev_fd, LOOP_CONFIGURE, &loopconfig));
+	else
+		TEST(ioctl(dev_fd, LOOP_SET_BLOCK_SIZE, *(tcases[n].setvalue)));
+
+	if (TST_RET == 0) {
+		tst_res(TFAIL, "Set block size succeed unexpectedly");
+		if (tcases[n].ioctl_flag == LOOP_CONFIGURE)
+			TST_RETRY_FUNC(ioctl(dev_fd, LOOP_CLR_FD, 0), TST_RETVAL_EQ0);
+		return;
+	}
+	if (TST_ERR == EINVAL)
+		tst_res(TPASS | TTERRNO, "Set block size failed as expected");
+	else
+		tst_res(TFAIL | TTERRNO, "Set block size failed expected EINVAL got");
+}
+
+static void run(unsigned int n)
 {
 	struct tcase *tc = &tcases[n];
 
 	tst_res(TINFO, "%s", tc->message);
-	TEST(ioctl(dev_fd, LOOP_SET_BLOCK_SIZE, *(tc->setvalue)));
-	if (TST_RET == 0) {
-		tst_res(TFAIL, "LOOP_SET_BLOCK_SIZE succeed unexpectedly");
+	if (tc->ioctl_flag == LOOP_SET_BLOCK_SIZE) {
+		if (!attach_flag) {
+			tst_attach_device(dev_path, "test.img");
+			attach_flag = 1;
+		}
+		verify_ioctl_loop(n);
 		return;
 	}
 
-	if (TST_ERR == tc->exp_err) {
-		tst_res(TPASS | TTERRNO, "LOOP_SET_BLOCK_SIZE failed as expected");
-	} else {
-		tst_res(TFAIL | TTERRNO, "LOOP_SET_BLOCK_SIZE failed expected %s got",
-				tst_strerrno(tc->exp_err));
+	if (tc->ioctl_flag == LOOP_CONFIGURE && !loop_configure_sup) {
+		tst_res(TCONF, "LOOP_CONFIGURE ioctl not supported");
+		return;
+	}
+	if (attach_flag) {
+		SAFE_CLOSE(dev_fd);
+		tst_detach_device(dev_path);
+		attach_flag = 0;
 	}
+	if (dev_fd < 0)
+		dev_fd = SAFE_OPEN(dev_path, O_RDWR);
+	loopconfig.block_size = *(tc->setvalue);
+	verify_ioctl_loop(n);
 }
 
 static void setup(void)
 {
 	unsigned int pg_size;
+	int ret;
 
 	dev_num = tst_find_free_loopdev(dev_path, sizeof(dev_path));
 	if (dev_num < 0)
@@ -67,12 +112,24 @@ static void setup(void)
 
 	if (ioctl(dev_fd, LOOP_SET_BLOCK_SIZE, 512) && errno == EINVAL)
 		tst_brk(TCONF, "LOOP_SET_BLOCK_SIZE is not supported");
+
+	file_fd = SAFE_OPEN("test.img", O_RDWR);
+	loopconfig.fd = -1;
+	ret = ioctl(dev_fd, LOOP_CONFIGURE, &loopconfig);
+	if (ret && errno != EBADF) {
+		tst_res(TINFO | TERRNO, "LOOP_CONFIGURE is not supported");
+		loop_configure_sup = 0;
+		return;
+	}
+	loopconfig.fd = file_fd;
 }
 
 static void cleanup(void)
 {
 	if (dev_fd > 0)
 		SAFE_CLOSE(dev_fd);
+	if (file_fd > 0)
+		SAFE_CLOSE(file_fd);
 	if (attach_flag)
 		tst_detach_device(dev_path);
 }
@@ -80,7 +137,7 @@ static void cleanup(void)
 static struct tst_test test = {
 	.setup = setup,
 	.cleanup = cleanup,
-	.test = verify_ioctl_loop,
+	.test = run,
 	.tcnt = ARRAY_SIZE(tcases),
 	.needs_root = 1,
 	.needs_tmpdir = 1,
-- 
2.23.0




^ permalink raw reply related	[flat|nested] 29+ messages in thread
* [LTP] [PATCH v2 2/2] syscalls/ioctl_loop05: Using LOOP_CONFIGURE to set direct io
  2020-07-10  6:39     ` [LTP] [PATCH v2 1/2] syscalls/ioctl_loop06: Using LOOP_CONFIGURE to test invalid block size Yang Xu
@ 2020-07-10  6:39       ` Yang Xu
  2020-07-30  7:28           ` [LTP] " Yang Xu
  2020-07-22  9:45       ` [LTP] [PATCH v2 1/2] syscalls/ioctl_loop06: Using LOOP_CONFIGURE to test invalid block size Cyril Hrubis
  1 sibling, 1 reply; 29+ messages in thread
From: Yang Xu @ 2020-07-10  6:39 UTC (permalink / raw)
  To: ltp

Since kernel commit 3448914e8cc5("loop: Add LOOP_CONFIGURE ioctl"),
it can explicitly request direct I/O mode by setting LO_FLAGS_DIRECT_IO
in loop_config.info.lo_flags.

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
 .../kernel/syscalls/ioctl/ioctl_loop05.c      | 154 +++++++++++++-----
 1 file changed, 117 insertions(+), 37 deletions(-)

diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop05.c b/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
index e3c14faab..6abb27998 100644
--- a/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
+++ b/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
@@ -19,6 +19,9 @@
  * enabled but falls back to buffered I/O later on. This is the case at least
  * for Btrfs. Because of that the test passes both with failure as well as
  * success with non-zero offset.
+ *
+ * Also use LOOP_CONFIGURE to test this by setting LO_FLAGS_DIRECT_IO
+ * in loop_config.info.lo_flags.
  */
 
 #include <stdio.h>
@@ -32,8 +35,36 @@
 #define DIO_MESSAGE "In dio mode"
 #define NON_DIO_MESSAGE "In non dio mode"
 
-static char dev_path[1024], sys_loop_diopath[1024], backing_file_path[1024];;
+static char dev_path[1024], sys_loop_diopath[1024], backing_file_path[1024];
 static int dev_num, dev_fd, block_devfd, attach_flag, logical_block_size;
+static int file_fd, loop_configure_sup = 1;
+static struct loop_config loopconfig;
+static struct loop_info loopinfo;
+
+static struct tcase {
+	int multi; /*logical_block_size / 2 as unit */
+	int dio_value;
+	int ioctl_flag;
+	char *message;
+} tcases[] = {
+	{0, 1, LOOP_SET_DIRECT_IO,
+	"Using LOOP_SET_DIRET_IO without setting lo_offset or sizelimit"},
+
+	{2, 1, LOOP_SET_DIRECT_IO,
+	"Using LOOP_SET_DIRECT_IO With offset equal to logical_block_size"},
+
+	{1, 0, LOOP_SET_DIRECT_IO,
+	"Using LOOP_SET_DIRECT_IO with offset less than logical_block_size"},
+
+	{0, 1, LOOP_CONFIGURE,
+	"Using LOOP_CONFIGURE without setting lo_offset or sizelimit"},
+
+	{2, 1, LOOP_CONFIGURE,
+	"Using LOOP_CONFIGURE With offset equal to logical_block_size"},
+
+	{1, 0, LOOP_CONFIGURE,
+	"Using LOOP_CONFIGURE witg offset less than logical_block_size"},
+};
 
 static void check_dio_value(int flag)
 {
@@ -42,61 +73,94 @@ static void check_dio_value(int flag)
 	memset(&loopinfoget, 0, sizeof(loopinfoget));
 
 	SAFE_IOCTL(dev_fd, LOOP_GET_STATUS, &loopinfoget);
-	tst_res(TINFO, "%s", flag ? DIO_MESSAGE : NON_DIO_MESSAGE);
 
 	if (loopinfoget.lo_flags & LO_FLAGS_DIRECT_IO)
-		tst_res(flag ? TPASS : TFAIL, "lo_flags has LO_FLAGS_DIRECT_IO flag");
+		tst_res(flag ? TPASS : TFAIL,
+			"%s, lo_flags has LO_FLAGS_DIRECT_IO flag",
+			flag ? DIO_MESSAGE : NON_DIO_MESSAGE);
 	else
-		tst_res(flag ? TFAIL : TPASS, "lo_flags doesn't have LO_FLAGS_DIRECT_IO flag");
+		tst_res(flag ? TFAIL : TPASS,
+			"%s, lo_flags doesn't have LO_FLAGS_DIRECT_IO flag",
+			flag ? DIO_MESSAGE : NON_DIO_MESSAGE);
 
 	TST_ASSERT_INT(sys_loop_diopath, flag);
 }
 
-static void verify_ioctl_loop(void)
+static void verify_ioctl_loop(unsigned int n)
 {
-	struct loop_info loopinfo;
-
-	memset(&loopinfo, 0, sizeof(loopinfo));
-	TST_RETRY_FUNC(ioctl(dev_fd, LOOP_SET_STATUS, &loopinfo), TST_RETVAL_EQ0);
+	if (tcases[n].ioctl_flag == LOOP_SET_DIRECT_IO) {
+		TST_RETRY_FUNC(ioctl(dev_fd, LOOP_SET_STATUS, &loopinfo), TST_RETVAL_EQ0);
+
+		TEST(ioctl(dev_fd, LOOP_SET_DIRECT_IO, 1));
+		if (TST_RET == 0) {
+			if (tcases[n].dio_value)
+				tst_res(TPASS, "set direct io succeeded");
+			else
+				tst_res(TPASS, "set direct io succeeded, offset is ignored");
+			check_dio_value(1);
+			SAFE_IOCTL(dev_fd, LOOP_SET_DIRECT_IO, 0);
+			return;
+		}
+		if (TST_ERR == EINVAL && !tcases[n].dio_value)
+			tst_res(TPASS | TTERRNO,
+				"set direct io failed as expected");
+		else
+			tst_res(TFAIL | TTERRNO, "set direct io failed");
+		return;
+	}
+	/*
+	 * When we call loop_configure ioctl successfully and detach it,
+	 * the subquent loop_configure without non-zero lo_offset or
+	 * sizelimit may trigger the blk_update_request I/O error.
+	 * To avoid this, sleep 1s to ensure last blk_update_request has
+	 * completed.
+	 */
+	sleep(1);
+	/*
+	 * loop_cofigure calls loop_update_dio() function, it will ignore
+	 * the result of setting dio. It is different from loop_set_dio.
+	 */
+	TST_RETRY_FUNC(ioctl(dev_fd, LOOP_CONFIGURE, &loopconfig), TST_RETVAL_EQ0);
+	check_dio_value(tcases[n].dio_value);
+	TST_RETRY_FUNC(ioctl(dev_fd, LOOP_CLR_FD, 0), TST_RETVAL_EQ0);
+}
 
-	tst_res(TINFO, "Without setting lo_offset or sizelimit");
-	SAFE_IOCTL(dev_fd, LOOP_SET_DIRECT_IO, 1);
-	check_dio_value(1);
+static void run(unsigned int n)
+{
+	struct tcase *tc = &tcases[n];
 
-	SAFE_IOCTL(dev_fd, LOOP_SET_DIRECT_IO, 0);
-	check_dio_value(0);
+	tst_res(TINFO, "%s", tc->message);
 
-	tst_res(TINFO, "With offset equal to logical_block_size");
-	loopinfo.lo_offset = logical_block_size;
-	TST_RETRY_FUNC(ioctl(dev_fd, LOOP_SET_STATUS, &loopinfo), TST_RETVAL_EQ0);
-	TEST(ioctl(dev_fd, LOOP_SET_DIRECT_IO, 1));
-	if (TST_RET == 0) {
-		tst_res(TPASS, "LOOP_SET_DIRECT_IO succeeded");
-		check_dio_value(1);
+	if (tc->ioctl_flag == LOOP_SET_DIRECT_IO) {
+		if (!attach_flag) {
+			tst_attach_device(dev_path, "test.img");
+			attach_flag = 1;
+		}
 		SAFE_IOCTL(dev_fd, LOOP_SET_DIRECT_IO, 0);
-	} else {
-		tst_res(TFAIL | TTERRNO, "LOOP_SET_DIRECT_IO failed");
+		check_dio_value(0);
+		loopinfo.lo_offset = logical_block_size * tc->multi / 2;
+		verify_ioctl_loop(n);
+		return;
 	}
-
-	tst_res(TINFO, "With nonzero offset less than logical_block_size");
-	loopinfo.lo_offset = logical_block_size / 2;
-	TST_RETRY_FUNC(ioctl(dev_fd, LOOP_SET_STATUS, &loopinfo), TST_RETVAL_EQ0);
-
-	TEST(ioctl(dev_fd, LOOP_SET_DIRECT_IO, 1));
-	if (TST_RET == 0) {
-		tst_res(TPASS, "LOOP_SET_DIRECT_IO succeeded, offset is ignored");
-		SAFE_IOCTL(dev_fd, LOOP_SET_DIRECT_IO, 0);
+	if (tc->ioctl_flag == LOOP_CONFIGURE && !loop_configure_sup) {
+		tst_res(TCONF, "LOOP_CONFIGURE ioctl not supported");
 		return;
 	}
-	if (TST_ERR == EINVAL)
-		tst_res(TPASS | TTERRNO, "LOOP_SET_DIRECT_IO failed as expected");
-	else
-		tst_res(TFAIL | TTERRNO, "LOOP_SET_DIRECT_IO failed expected EINVAL got");
+	if (attach_flag) {
+		SAFE_CLOSE(dev_fd);
+		tst_detach_device(dev_path);
+		attach_flag = 0;
+	}
+	if (dev_fd < 0)
+		dev_fd = SAFE_OPEN(dev_path, O_RDWR);
+	loopconfig.info.lo_offset = logical_block_size * tc->multi / 2;
+	verify_ioctl_loop(n);
 }
 
 static void setup(void)
 {
 	char bd_path[100];
+	int ret;
 
 	if (tst_fs_type(".") == TST_TMPFS_MAGIC)
 		tst_brk(TCONF, "tmpfd doesn't support O_DIRECT flag");
@@ -128,8 +192,21 @@ static void setup(void)
 	SAFE_IOCTL(block_devfd, BLKSSZGET, &logical_block_size);
 	tst_res(TINFO, "backing dev(%s) logical_block_size is %d", bd_path, logical_block_size);
 	SAFE_CLOSE(block_devfd);
+
 	if (logical_block_size > 512)
 		TST_RETRY_FUNC(ioctl(dev_fd, LOOP_SET_BLOCK_SIZE, logical_block_size), TST_RETVAL_EQ0);
+
+	file_fd = SAFE_OPEN("test.img", O_RDWR);
+	loopconfig.fd = -1;
+	ret = ioctl(dev_fd, LOOP_CONFIGURE, &loopconfig);
+	if (ret && errno != EBADF) {
+		tst_res(TINFO | TERRNO, "LOOP_CONFIGURE is not supported");
+		loop_configure_sup = 0;
+		return;
+	}
+	loopconfig.block_size = logical_block_size;
+	loopconfig.fd = file_fd;
+	loopconfig.info.lo_flags = LO_FLAGS_DIRECT_IO;
 }
 
 static void cleanup(void)
@@ -138,6 +215,8 @@ static void cleanup(void)
 		SAFE_CLOSE(dev_fd);
 	if (block_devfd > 0)
 		SAFE_CLOSE(block_devfd);
+	if (file_fd > 0)
+		SAFE_CLOSE(file_fd);
 	if (attach_flag)
 		tst_detach_device(dev_path);
 }
@@ -145,7 +224,8 @@ static void cleanup(void)
 static struct tst_test test = {
 	.setup = setup,
 	.cleanup = cleanup,
-	.test_all = verify_ioctl_loop,
+	.test = run,
+	.tcnt = ARRAY_SIZE(tcases),
 	.needs_root = 1,
 	.needs_tmpdir = 1,
 	.needs_drivers = (const char *const []) {
-- 
2.23.0




^ permalink raw reply related	[flat|nested] 29+ messages in thread
* [LTP] [PATCH v2 1/2] syscalls/ioctl_loop06: Using LOOP_CONFIGURE to test invalid block size
  2020-07-10  6:39     ` [LTP] [PATCH v2 1/2] syscalls/ioctl_loop06: Using LOOP_CONFIGURE to test invalid block size Yang Xu
  2020-07-10  6:39       ` [LTP] [PATCH v2 2/2] syscalls/ioctl_loop05: Using LOOP_CONFIGURE to set direct io Yang Xu
@ 2020-07-22  9:45       ` Cyril Hrubis
  2020-07-22 10:15         ` Yang Xu
  1 sibling, 1 reply; 29+ messages in thread
From: Cyril Hrubis @ 2020-07-22  9:45 UTC (permalink / raw)
  To: ltp

Hi!
Do we really need to close and open the dev_fd repeatedly and also we
don't have to attach the device in the test setup?

I.e. it should work the same with:

diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop06.c b/testcases/kernel/syscalls/ioctl/ioctl_loop06.c
index 2f172a09d..7936af4ac 100644
--- a/testcases/kernel/syscalls/ioctl/ioctl_loop06.c
+++ b/testcases/kernel/syscalls/ioctl/ioctl_loop06.c
@@ -81,12 +81,9 @@ static void run(unsigned int n)
                return;
        }
        if (attach_flag) {
-               SAFE_CLOSE(dev_fd);
                tst_detach_device(dev_path);
                attach_flag = 0;
        }
-       if (dev_fd < 0)
-               dev_fd = SAFE_OPEN(dev_path, O_RDWR);
        loopconfig.block_size = *(tc->setvalue);
        verify_ioctl_loop(n);
 }
@@ -101,8 +98,6 @@ static void setup(void)
                tst_brk(TBROK, "Failed to find free loop device");

        tst_fill_file("test.img", 0, 1024, 1024);
-       tst_attach_device(dev_path, "test.img");
-       attach_flag = 1;
        half_value = 256;
        pg_size = getpagesize();
        invalid_value = pg_size * 2;

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply related	[flat|nested] 29+ messages in thread
* [LTP] [PATCH v2 1/2] syscalls/ioctl_loop06: Using LOOP_CONFIGURE to test invalid block size
  2020-07-22  9:45       ` [LTP] [PATCH v2 1/2] syscalls/ioctl_loop06: Using LOOP_CONFIGURE to test invalid block size Cyril Hrubis
@ 2020-07-22 10:15         ` Yang Xu
  2020-07-22 12:59           ` Cyril Hrubis
  0 siblings, 1 reply; 29+ messages in thread
From: Yang Xu @ 2020-07-22 10:15 UTC (permalink / raw)
  To: ltp

Hi Cyril

> Hi!
> Do we really need to close and open the dev_fd repeatedly and also we
> don't have to attach the device in the test setup?
YES, we don't need to attach the device in the setup because 
LOOP_SET_BLOCK_SIZE checks works well(return ENXIO if supports, return 
EINVAL if not supports) when not attaching device.

But for close and open the dev_fd repeatedly, I think it is necessary 
because when we detach device firstly without closing dev fd, it will 
report the warnging as below:

tst_device.c:89: INFO: Found free device 0 '/dev/loop0'
ioctl_loop06.c:69: INFO: Using LOOP_SET_BLOCK_SIZE with arg < 512
ioctl_loop06.c:60: PASS: Set block size failed as expected: EINVAL (22)
ioctl_loop06.c:69: INFO: Using LOOP_SET_BLOCK_SIZE with arg > PAGE_SIZE
ioctl_loop06.c:60: PASS: Set block size failed as expected: EINVAL (22)
ioctl_loop06.c:69: INFO: Using LOOP_SET_BLOCK_SIZE with arg != power_of_2
ioctl_loop06.c:60: PASS: Set block size failed as expected: EINVAL (22)
ioctl_loop06.c:69: INFO: Using LOOP_CONFIGURE with block_size < 512
tst_device.c:223: WARN: ioctl(/dev/loop0, LOOP_CLR_FD, 0) no ENXIO for 
too long
ioctl_loop06.c:62: FAIL: Set block size failed expected EINVAL got: 
EBUSY (16)

That is why I close dev_fd firstly and then detach device in cleanup 
function.

> 
> I.e. it should work the same with:
> 
> diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop06.c b/testcases/kernel/syscalls/ioctl/ioctl_loop06.c
> index 2f172a09d..7936af4ac 100644
> --- a/testcases/kernel/syscalls/ioctl/ioctl_loop06.c
> +++ b/testcases/kernel/syscalls/ioctl/ioctl_loop06.c
> @@ -81,12 +81,9 @@ static void run(unsigned int n)
>                  return;
>          }
>          if (attach_flag) {
> -               SAFE_CLOSE(dev_fd);
>                  tst_detach_device(dev_path);
>                  attach_flag = 0;
>          }
> -       if (dev_fd < 0)
> -               dev_fd = SAFE_OPEN(dev_path, O_RDWR);
>          loopconfig.block_size = *(tc->setvalue);
>          verify_ioctl_loop(n);
>   }
> @@ -101,8 +98,6 @@ static void setup(void)
>                  tst_brk(TBROK, "Failed to find free loop device");
> 
>          tst_fill_file("test.img", 0, 1024, 1024);
> -       tst_attach_device(dev_path, "test.img");
> -       attach_flag = 1;
>          half_value = 256;
>          pg_size = getpagesize();
>          invalid_value = pg_size * 2;
> 



^ permalink raw reply	[flat|nested] 29+ messages in thread
* [LTP] [PATCH v2 1/2] syscalls/ioctl_loop06: Using LOOP_CONFIGURE to test invalid block size
  2020-07-22 10:15         ` Yang Xu
@ 2020-07-22 12:59           ` Cyril Hrubis
  2020-07-23  9:41             ` Yang Xu
  2020-07-24  2:05             ` [LTP] [PATCH v3 " Yang Xu
  0 siblings, 2 replies; 29+ messages in thread
From: Cyril Hrubis @ 2020-07-22 12:59 UTC (permalink / raw)
  To: ltp

Hi!
> > Do we really need to close and open the dev_fd repeatedly and also we
> > don't have to attach the device in the test setup?
> YES, we don't need to attach the device in the setup because 
> LOOP_SET_BLOCK_SIZE checks works well(return ENXIO if supports, return 
> EINVAL if not supports) when not attaching device.
> 
> But for close and open the dev_fd repeatedly, I think it is necessary 
> because when we detach device firstly without closing dev fd, it will 
> report the warnging as below:
> 
> tst_device.c:89: INFO: Found free device 0 '/dev/loop0'
> ioctl_loop06.c:69: INFO: Using LOOP_SET_BLOCK_SIZE with arg < 512
> ioctl_loop06.c:60: PASS: Set block size failed as expected: EINVAL (22)
> ioctl_loop06.c:69: INFO: Using LOOP_SET_BLOCK_SIZE with arg > PAGE_SIZE
> ioctl_loop06.c:60: PASS: Set block size failed as expected: EINVAL (22)
> ioctl_loop06.c:69: INFO: Using LOOP_SET_BLOCK_SIZE with arg != power_of_2
> ioctl_loop06.c:60: PASS: Set block size failed as expected: EINVAL (22)
> ioctl_loop06.c:69: INFO: Using LOOP_CONFIGURE with block_size < 512
> tst_device.c:223: WARN: ioctl(/dev/loop0, LOOP_CLR_FD, 0) no ENXIO for 
> too long
> ioctl_loop06.c:62: FAIL: Set block size failed expected EINVAL got: 
> EBUSY (16)
> 
> That is why I close dev_fd firstly and then detach device in cleanup 
> function.

Ah, right, the tst_detach_device() opens the dev_path so the function
fails to destroy it because the device is opened twice at that point.

I guess that proper solution would be to add a tst_detach_device_by_fd()
and change the library so that tst_detach_device() opens the dev_path
and calls tst_detach_device_by_fd() internally.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 29+ messages in thread
* [LTP] [PATCH v2 1/2] syscalls/ioctl_loop06: Using LOOP_CONFIGURE to test invalid block size
  2020-07-22 12:59           ` Cyril Hrubis
@ 2020-07-23  9:41             ` Yang Xu
  2020-07-24  2:05             ` [LTP] [PATCH v3 " Yang Xu
  1 sibling, 0 replies; 29+ messages in thread
From: Yang Xu @ 2020-07-23  9:41 UTC (permalink / raw)
  To: ltp

HI Cyril


> Hi!
>>> Do we really need to close and open the dev_fd repeatedly and also we
>>> don't have to attach the device in the test setup?
>> YES, we don't need to attach the device in the setup because
>> LOOP_SET_BLOCK_SIZE checks works well(return ENXIO if supports, return
>> EINVAL if not supports) when not attaching device.
>>
>> But for close and open the dev_fd repeatedly, I think it is necessary
>> because when we detach device firstly without closing dev fd, it will
>> report the warnging as below:
>>
>> tst_device.c:89: INFO: Found free device 0 '/dev/loop0'
>> ioctl_loop06.c:69: INFO: Using LOOP_SET_BLOCK_SIZE with arg < 512
>> ioctl_loop06.c:60: PASS: Set block size failed as expected: EINVAL (22)
>> ioctl_loop06.c:69: INFO: Using LOOP_SET_BLOCK_SIZE with arg > PAGE_SIZE
>> ioctl_loop06.c:60: PASS: Set block size failed as expected: EINVAL (22)
>> ioctl_loop06.c:69: INFO: Using LOOP_SET_BLOCK_SIZE with arg != power_of_2
>> ioctl_loop06.c:60: PASS: Set block size failed as expected: EINVAL (22)
>> ioctl_loop06.c:69: INFO: Using LOOP_CONFIGURE with block_size < 512
>> tst_device.c:223: WARN: ioctl(/dev/loop0, LOOP_CLR_FD, 0) no ENXIO for
>> too long
>> ioctl_loop06.c:62: FAIL: Set block size failed expected EINVAL got:
>> EBUSY (16)
>>
>> That is why I close dev_fd firstly and then detach device in cleanup
>> function.
> 
> Ah, right, the tst_detach_device() opens the dev_path so the function
> fails to destroy it because the device is opened twice at that point.
> 
> I guess that proper solution would be to add a tst_detach_device_by_fd()
> and change the library so that tst_detach_device() opens the dev_path
> and calls tst_detach_device_by_fd() internally.
Should I send a v3 patch or you directly use the new api for this patch?
> 



^ permalink raw reply	[flat|nested] 29+ messages in thread
* [LTP] [PATCH v3 1/2] syscalls/ioctl_loop06: Using LOOP_CONFIGURE to test invalid block size
  2020-07-22 12:59           ` Cyril Hrubis
  2020-07-23  9:41             ` Yang Xu
@ 2020-07-24  2:05             ` Yang Xu
  2020-07-24  2:05               ` [LTP] [PATCH v3 2/2] syscalls/ioctl_loop05: Using LOOP_CONFIGURE to set direct io Yang Xu
  2020-07-29 10:07               ` [LTP] [PATCH v3 1/2] syscalls/ioctl_loop06: Using LOOP_CONFIGURE to test invalid block size Cyril Hrubis
  1 sibling, 2 replies; 29+ messages in thread
From: Yang Xu @ 2020-07-24  2:05 UTC (permalink / raw)
  To: ltp

Since kernel commit 3448914e8cc5("loop: Add LOOP_CONFIGURE ioctl"),
it can set the correct block size immediately by setting loop_config.block_size.

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
v2-v3:
1.remove tst_attch_device in setup
2.use tst_detach_device_by_fd api
 .../kernel/syscalls/ioctl/ioctl_loop06.c      | 88 +++++++++++++++----
 1 file changed, 70 insertions(+), 18 deletions(-)

diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop06.c b/testcases/kernel/syscalls/ioctl/ioctl_loop06.c
index 096ec9363..d073c120b 100644
--- a/testcases/kernel/syscalls/ioctl/ioctl_loop06.c
+++ b/testcases/kernel/syscalls/ioctl/ioctl_loop06.c
@@ -3,8 +3,8 @@
  * Copyright (c) 2020 FUJITSU LIMITED. All rights reserved.
  * Author: Yang Xu <xuyang2018.jy@cn.jujitsu.com>
  *
- * This is a basic ioctl error test about loopdevice
- * LOOP_SET_BLOCK_SIZE.
+ * This is a basic error test about the invalid block size of loopdevice
+ * by using LOOP_SET_BLOCK_SIZE or LOOP_CONFIGURE ioctl.
  */
 
 #include <stdio.h>
@@ -15,49 +15,89 @@
 #include "tst_test.h"
 
 static char dev_path[1024];
-static int dev_num, dev_fd, attach_flag;
+static int dev_num, dev_fd, file_fd, attach_flag, loop_configure_sup = 1;
 static unsigned int invalid_value, half_value, unalign_value;
+static struct loop_config loopconfig;
 
 static struct tcase {
 	unsigned int *setvalue;
-	int exp_err;
+	int ioctl_flag;
 	char *message;
 } tcases[] = {
-	{&half_value, EINVAL, "arg < 512"},
-	{&invalid_value, EINVAL, "arg > PAGE_SIZE"},
-	{&unalign_value, EINVAL, "arg != power_of_2"},
+	{&half_value, LOOP_SET_BLOCK_SIZE,
+	"Using LOOP_SET_BLOCK_SIZE with arg < 512"},
+
+	{&invalid_value, LOOP_SET_BLOCK_SIZE,
+	"Using LOOP_SET_BLOCK_SIZE with arg > PAGE_SIZE"},
+
+	{&unalign_value, LOOP_SET_BLOCK_SIZE,
+	"Using LOOP_SET_BLOCK_SIZE with arg != power_of_2"},
+
+	{&half_value, LOOP_CONFIGURE,
+	"Using LOOP_CONFIGURE with block_size < 512"},
+
+	{&invalid_value, LOOP_CONFIGURE,
+	"Using LOOP_CONFIGURE with block_size > PAGE_SIZE"},
+
+	{&unalign_value, LOOP_CONFIGURE,
+	"Using LOOP_CONFIGURE with block_size != power_of_2"},
 };
 
 static void verify_ioctl_loop(unsigned int n)
+{
+	if (tcases[n].ioctl_flag == LOOP_CONFIGURE)
+		TEST(ioctl(dev_fd, LOOP_CONFIGURE, &loopconfig));
+	else
+		TEST(ioctl(dev_fd, LOOP_SET_BLOCK_SIZE, *(tcases[n].setvalue)));
+
+	if (TST_RET == 0) {
+		tst_res(TFAIL, "Set block size succeed unexpectedly");
+		if (tcases[n].ioctl_flag == LOOP_CONFIGURE)
+			TST_RETRY_FUNC(ioctl(dev_fd, LOOP_CLR_FD, 0), TST_RETVAL_EQ0);
+		return;
+	}
+	if (TST_ERR == EINVAL)
+		tst_res(TPASS | TTERRNO, "Set block size failed as expected");
+	else
+		tst_res(TFAIL | TTERRNO, "Set block size failed expected EINVAL got");
+}
+
+static void run(unsigned int n)
 {
 	struct tcase *tc = &tcases[n];
 
 	tst_res(TINFO, "%s", tc->message);
-	TEST(ioctl(dev_fd, LOOP_SET_BLOCK_SIZE, *(tc->setvalue)));
-	if (TST_RET == 0) {
-		tst_res(TFAIL, "LOOP_SET_BLOCK_SIZE succeed unexpectedly");
+	if (tc->ioctl_flag == LOOP_SET_BLOCK_SIZE) {
+		if (!attach_flag) {
+			tst_attach_device(dev_path, "test.img");
+			attach_flag = 1;
+		}
+		verify_ioctl_loop(n);
 		return;
 	}
 
-	if (TST_ERR == tc->exp_err) {
-		tst_res(TPASS | TTERRNO, "LOOP_SET_BLOCK_SIZE failed as expected");
-	} else {
-		tst_res(TFAIL | TTERRNO, "LOOP_SET_BLOCK_SIZE failed expected %s got",
-				tst_strerrno(tc->exp_err));
+	if (tc->ioctl_flag == LOOP_CONFIGURE && !loop_configure_sup) {
+		tst_res(TCONF, "LOOP_CONFIGURE ioctl not supported");
+		return;
 	}
+	if (attach_flag) {
+		tst_detach_device_by_fd(dev_path, dev_fd);
+		attach_flag = 0;
+	}
+	loopconfig.block_size = *(tc->setvalue);
+	verify_ioctl_loop(n);
 }
 
 static void setup(void)
 {
 	unsigned int pg_size;
+	int ret;
 
 	dev_num = tst_find_free_loopdev(dev_path, sizeof(dev_path));
 	if (dev_num < 0)
 		tst_brk(TBROK, "Failed to find free loop device");
 
 	tst_fill_file("test.img", 0, 1024, 1024);
-	tst_attach_device(dev_path, "test.img");
-	attach_flag = 1;
 	half_value = 256;
 	pg_size = getpagesize();
 	invalid_value = pg_size * 2 ;
@@ -67,12 +107,24 @@ static void setup(void)
 
 	if (ioctl(dev_fd, LOOP_SET_BLOCK_SIZE, 512) && errno == EINVAL)
 		tst_brk(TCONF, "LOOP_SET_BLOCK_SIZE is not supported");
+
+	file_fd = SAFE_OPEN("test.img", O_RDWR);
+	loopconfig.fd = -1;
+	ret = ioctl(dev_fd, LOOP_CONFIGURE, &loopconfig);
+	if (ret && errno != EBADF) {
+		tst_res(TINFO | TERRNO, "LOOP_CONFIGURE is not supported");
+		loop_configure_sup = 0;
+		return;
+	}
+	loopconfig.fd = file_fd;
 }
 
 static void cleanup(void)
 {
 	if (dev_fd > 0)
 		SAFE_CLOSE(dev_fd);
+	if (file_fd > 0)
+		SAFE_CLOSE(file_fd);
 	if (attach_flag)
 		tst_detach_device(dev_path);
 }
@@ -80,7 +132,7 @@ static void cleanup(void)
 static struct tst_test test = {
 	.setup = setup,
 	.cleanup = cleanup,
-	.test = verify_ioctl_loop,
+	.test = run,
 	.tcnt = ARRAY_SIZE(tcases),
 	.needs_root = 1,
 	.needs_tmpdir = 1,
-- 
2.23.0




^ permalink raw reply related	[flat|nested] 29+ messages in thread
* [LTP] [PATCH v3 2/2] syscalls/ioctl_loop05: Using LOOP_CONFIGURE to set direct io
  2020-07-24  2:05             ` [LTP] [PATCH v3 " Yang Xu
@ 2020-07-24  2:05               ` Yang Xu
  2020-07-29 11:39                 ` Cyril Hrubis
  2020-07-29 12:58                 ` Cyril Hrubis
  2020-07-29 10:07               ` [LTP] [PATCH v3 1/2] syscalls/ioctl_loop06: Using LOOP_CONFIGURE to test invalid block size Cyril Hrubis
  1 sibling, 2 replies; 29+ messages in thread
From: Yang Xu @ 2020-07-24  2:05 UTC (permalink / raw)
  To: ltp

Since kernel commit 3448914e8cc5("loop: Add LOOP_CONFIGURE ioctl"),
it can explicitly request direct I/O mode by setting LO_FLAGS_DIRECT_IO
in loop_config.info.lo_flags.

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
v2->v3:
1.Use tst_detach_device_by_fd api
 .../kernel/syscalls/ioctl/ioctl_loop05.c      | 151 +++++++++++++-----
 1 file changed, 114 insertions(+), 37 deletions(-)

diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop05.c b/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
index e3c14faab..7491e62fa 100644
--- a/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
+++ b/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
@@ -19,6 +19,9 @@
  * enabled but falls back to buffered I/O later on. This is the case at least
  * for Btrfs. Because of that the test passes both with failure as well as
  * success with non-zero offset.
+ *
+ * Also use LOOP_CONFIGURE to test this by setting LO_FLAGS_DIRECT_IO
+ * in loop_config.info.lo_flags.
  */
 
 #include <stdio.h>
@@ -32,8 +35,36 @@
 #define DIO_MESSAGE "In dio mode"
 #define NON_DIO_MESSAGE "In non dio mode"
 
-static char dev_path[1024], sys_loop_diopath[1024], backing_file_path[1024];;
+static char dev_path[1024], sys_loop_diopath[1024], backing_file_path[1024];
 static int dev_num, dev_fd, block_devfd, attach_flag, logical_block_size;
+static int file_fd, loop_configure_sup = 1;
+static struct loop_config loopconfig;
+static struct loop_info loopinfo;
+
+static struct tcase {
+	int multi; /*logical_block_size / 2 as unit */
+	int dio_value;
+	int ioctl_flag;
+	char *message;
+} tcases[] = {
+	{0, 1, LOOP_SET_DIRECT_IO,
+	"Using LOOP_SET_DIRET_IO without setting lo_offset or sizelimit"},
+
+	{2, 1, LOOP_SET_DIRECT_IO,
+	"Using LOOP_SET_DIRECT_IO With offset equal to logical_block_size"},
+
+	{1, 0, LOOP_SET_DIRECT_IO,
+	"Using LOOP_SET_DIRECT_IO with offset less than logical_block_size"},
+
+	{0, 1, LOOP_CONFIGURE,
+	"Using LOOP_CONFIGURE without setting lo_offset or sizelimit"},
+
+	{2, 1, LOOP_CONFIGURE,
+	"Using LOOP_CONFIGURE With offset equal to logical_block_size"},
+
+	{1, 0, LOOP_CONFIGURE,
+	"Using LOOP_CONFIGURE witg offset less than logical_block_size"},
+};
 
 static void check_dio_value(int flag)
 {
@@ -42,61 +73,91 @@ static void check_dio_value(int flag)
 	memset(&loopinfoget, 0, sizeof(loopinfoget));
 
 	SAFE_IOCTL(dev_fd, LOOP_GET_STATUS, &loopinfoget);
-	tst_res(TINFO, "%s", flag ? DIO_MESSAGE : NON_DIO_MESSAGE);
 
 	if (loopinfoget.lo_flags & LO_FLAGS_DIRECT_IO)
-		tst_res(flag ? TPASS : TFAIL, "lo_flags has LO_FLAGS_DIRECT_IO flag");
+		tst_res(flag ? TPASS : TFAIL,
+			"%s, lo_flags has LO_FLAGS_DIRECT_IO flag",
+			flag ? DIO_MESSAGE : NON_DIO_MESSAGE);
 	else
-		tst_res(flag ? TFAIL : TPASS, "lo_flags doesn't have LO_FLAGS_DIRECT_IO flag");
+		tst_res(flag ? TFAIL : TPASS,
+			"%s, lo_flags doesn't have LO_FLAGS_DIRECT_IO flag",
+			flag ? DIO_MESSAGE : NON_DIO_MESSAGE);
 
 	TST_ASSERT_INT(sys_loop_diopath, flag);
 }
 
-static void verify_ioctl_loop(void)
+static void verify_ioctl_loop(unsigned int n)
 {
-	struct loop_info loopinfo;
-
-	memset(&loopinfo, 0, sizeof(loopinfo));
-	TST_RETRY_FUNC(ioctl(dev_fd, LOOP_SET_STATUS, &loopinfo), TST_RETVAL_EQ0);
+	if (tcases[n].ioctl_flag == LOOP_SET_DIRECT_IO) {
+		TST_RETRY_FUNC(ioctl(dev_fd, LOOP_SET_STATUS, &loopinfo), TST_RETVAL_EQ0);
+
+		TEST(ioctl(dev_fd, LOOP_SET_DIRECT_IO, 1));
+		if (TST_RET == 0) {
+			if (tcases[n].dio_value)
+				tst_res(TPASS, "set direct io succeeded");
+			else
+				tst_res(TPASS, "set direct io succeeded, offset is ignored");
+			check_dio_value(1);
+			SAFE_IOCTL(dev_fd, LOOP_SET_DIRECT_IO, 0);
+			return;
+		}
+		if (TST_ERR == EINVAL && !tcases[n].dio_value)
+			tst_res(TPASS | TTERRNO,
+				"set direct io failed as expected");
+		else
+			tst_res(TFAIL | TTERRNO, "set direct io failed");
+		return;
+	}
+	/*
+	 * When we call loop_configure ioctl successfully and detach it,
+	 * the subquent loop_configure without non-zero lo_offset or
+	 * sizelimit may trigger the blk_update_request I/O error.
+	 * To avoid this, sleep 1s to ensure last blk_update_request has
+	 * completed.
+	 */
+	sleep(1);
+	/*
+	 * loop_cofigure calls loop_update_dio() function, it will ignore
+	 * the result of setting dio. It is different from loop_set_dio.
+	 */
+	TST_RETRY_FUNC(ioctl(dev_fd, LOOP_CONFIGURE, &loopconfig), TST_RETVAL_EQ0);
+	check_dio_value(tcases[n].dio_value);
+	TST_RETRY_FUNC(ioctl(dev_fd, LOOP_CLR_FD, 0), TST_RETVAL_EQ0);
+}
 
-	tst_res(TINFO, "Without setting lo_offset or sizelimit");
-	SAFE_IOCTL(dev_fd, LOOP_SET_DIRECT_IO, 1);
-	check_dio_value(1);
+static void run(unsigned int n)
+{
+	struct tcase *tc = &tcases[n];
 
-	SAFE_IOCTL(dev_fd, LOOP_SET_DIRECT_IO, 0);
-	check_dio_value(0);
+	tst_res(TINFO, "%s", tc->message);
 
-	tst_res(TINFO, "With offset equal to logical_block_size");
-	loopinfo.lo_offset = logical_block_size;
-	TST_RETRY_FUNC(ioctl(dev_fd, LOOP_SET_STATUS, &loopinfo), TST_RETVAL_EQ0);
-	TEST(ioctl(dev_fd, LOOP_SET_DIRECT_IO, 1));
-	if (TST_RET == 0) {
-		tst_res(TPASS, "LOOP_SET_DIRECT_IO succeeded");
-		check_dio_value(1);
+	if (tc->ioctl_flag == LOOP_SET_DIRECT_IO) {
+		if (!attach_flag) {
+			tst_attach_device(dev_path, "test.img");
+			attach_flag = 1;
+		}
 		SAFE_IOCTL(dev_fd, LOOP_SET_DIRECT_IO, 0);
-	} else {
-		tst_res(TFAIL | TTERRNO, "LOOP_SET_DIRECT_IO failed");
+		check_dio_value(0);
+		loopinfo.lo_offset = logical_block_size * tc->multi / 2;
+		verify_ioctl_loop(n);
+		return;
 	}
-
-	tst_res(TINFO, "With nonzero offset less than logical_block_size");
-	loopinfo.lo_offset = logical_block_size / 2;
-	TST_RETRY_FUNC(ioctl(dev_fd, LOOP_SET_STATUS, &loopinfo), TST_RETVAL_EQ0);
-
-	TEST(ioctl(dev_fd, LOOP_SET_DIRECT_IO, 1));
-	if (TST_RET == 0) {
-		tst_res(TPASS, "LOOP_SET_DIRECT_IO succeeded, offset is ignored");
-		SAFE_IOCTL(dev_fd, LOOP_SET_DIRECT_IO, 0);
+	if (tc->ioctl_flag == LOOP_CONFIGURE && !loop_configure_sup) {
+		tst_res(TCONF, "LOOP_CONFIGURE ioctl not supported");
 		return;
 	}
-	if (TST_ERR == EINVAL)
-		tst_res(TPASS | TTERRNO, "LOOP_SET_DIRECT_IO failed as expected");
-	else
-		tst_res(TFAIL | TTERRNO, "LOOP_SET_DIRECT_IO failed expected EINVAL got");
+	if (attach_flag) {
+		tst_detach_device_by_fd(dev_path, dev_fd);
+		attach_flag = 0;
+	}
+	loopconfig.info.lo_offset = logical_block_size * tc->multi / 2;
+	verify_ioctl_loop(n);
 }
 
 static void setup(void)
 {
 	char bd_path[100];
+	int ret;
 
 	if (tst_fs_type(".") == TST_TMPFS_MAGIC)
 		tst_brk(TCONF, "tmpfd doesn't support O_DIRECT flag");
@@ -128,8 +189,21 @@ static void setup(void)
 	SAFE_IOCTL(block_devfd, BLKSSZGET, &logical_block_size);
 	tst_res(TINFO, "backing dev(%s) logical_block_size is %d", bd_path, logical_block_size);
 	SAFE_CLOSE(block_devfd);
+
 	if (logical_block_size > 512)
 		TST_RETRY_FUNC(ioctl(dev_fd, LOOP_SET_BLOCK_SIZE, logical_block_size), TST_RETVAL_EQ0);
+
+	file_fd = SAFE_OPEN("test.img", O_RDWR);
+	loopconfig.fd = -1;
+	ret = ioctl(dev_fd, LOOP_CONFIGURE, &loopconfig);
+	if (ret && errno != EBADF) {
+		tst_res(TINFO | TERRNO, "LOOP_CONFIGURE is not supported");
+		loop_configure_sup = 0;
+		return;
+	}
+	loopconfig.block_size = logical_block_size;
+	loopconfig.fd = file_fd;
+	loopconfig.info.lo_flags = LO_FLAGS_DIRECT_IO;
 }
 
 static void cleanup(void)
@@ -138,6 +212,8 @@ static void cleanup(void)
 		SAFE_CLOSE(dev_fd);
 	if (block_devfd > 0)
 		SAFE_CLOSE(block_devfd);
+	if (file_fd > 0)
+		SAFE_CLOSE(file_fd);
 	if (attach_flag)
 		tst_detach_device(dev_path);
 }
@@ -145,7 +221,8 @@ static void cleanup(void)
 static struct tst_test test = {
 	.setup = setup,
 	.cleanup = cleanup,
-	.test_all = verify_ioctl_loop,
+	.test = run,
+	.tcnt = ARRAY_SIZE(tcases),
 	.needs_root = 1,
 	.needs_tmpdir = 1,
 	.needs_drivers = (const char *const []) {
-- 
2.23.0




^ permalink raw reply related	[flat|nested] 29+ messages in thread
* [LTP] [PATCH v3 1/2] syscalls/ioctl_loop06: Using LOOP_CONFIGURE to test invalid block size
  2020-07-24  2:05             ` [LTP] [PATCH v3 " Yang Xu
  2020-07-24  2:05               ` [LTP] [PATCH v3 2/2] syscalls/ioctl_loop05: Using LOOP_CONFIGURE to set direct io Yang Xu
@ 2020-07-29 10:07               ` Cyril Hrubis
  2020-07-29 10:43                 ` Yang Xu
  1 sibling, 1 reply; 29+ messages in thread
From: Cyril Hrubis @ 2020-07-29 10:07 UTC (permalink / raw)
  To: ltp

Hi!
>  static void verify_ioctl_loop(unsigned int n)
> +{
> +	if (tcases[n].ioctl_flag == LOOP_CONFIGURE)
> +		TEST(ioctl(dev_fd, LOOP_CONFIGURE, &loopconfig));
> +	else
> +		TEST(ioctl(dev_fd, LOOP_SET_BLOCK_SIZE, *(tcases[n].setvalue)));
> +
> +	if (TST_RET == 0) {
> +		tst_res(TFAIL, "Set block size succeed unexpectedly");
> +		if (tcases[n].ioctl_flag == LOOP_CONFIGURE)
> +			TST_RETRY_FUNC(ioctl(dev_fd, LOOP_CLR_FD, 0), TST_RETVAL_EQ0);

I guess that we can use the tst_detach_device_by_fd() here as well
right?

If you agree with that change I will change this and push the patch.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 29+ messages in thread
* [LTP] [PATCH v3 1/2] syscalls/ioctl_loop06: Using LOOP_CONFIGURE to test invalid block size
  2020-07-29 10:07               ` [LTP] [PATCH v3 1/2] syscalls/ioctl_loop06: Using LOOP_CONFIGURE to test invalid block size Cyril Hrubis
@ 2020-07-29 10:43                 ` Yang Xu
  2020-07-31 14:15                   ` Cyril Hrubis
  0 siblings, 1 reply; 29+ messages in thread
From: Yang Xu @ 2020-07-29 10:43 UTC (permalink / raw)
  To: ltp

Hi Cyril
> Hi!
>>   static void verify_ioctl_loop(unsigned int n)
>> +{
>> +	if (tcases[n].ioctl_flag == LOOP_CONFIGURE)
>> +		TEST(ioctl(dev_fd, LOOP_CONFIGURE, &loopconfig));
>> +	else
>> +		TEST(ioctl(dev_fd, LOOP_SET_BLOCK_SIZE, *(tcases[n].setvalue)));
>> +
>> +	if (TST_RET == 0) {
>> +		tst_res(TFAIL, "Set block size succeed unexpectedly");
>> +		if (tcases[n].ioctl_flag == LOOP_CONFIGURE)
>> +			TST_RETRY_FUNC(ioctl(dev_fd, LOOP_CLR_FD, 0), TST_RETVAL_EQ0);
> 
> I guess that we can use the tst_detach_device_by_fd() here as well
> right?
> 
> If you agree with that change I will change this and push the patch.
Agree.
> 



^ permalink raw reply	[flat|nested] 29+ messages in thread
* [LTP] [PATCH v3 2/2] syscalls/ioctl_loop05: Using LOOP_CONFIGURE to set direct io
  2020-07-24  2:05               ` [LTP] [PATCH v3 2/2] syscalls/ioctl_loop05: Using LOOP_CONFIGURE to set direct io Yang Xu
@ 2020-07-29 11:39                 ` Cyril Hrubis
  2020-07-30  8:49                   ` Yang Xu
  2020-07-29 12:58                 ` Cyril Hrubis
  1 sibling, 1 reply; 29+ messages in thread
From: Cyril Hrubis @ 2020-07-29 11:39 UTC (permalink / raw)
  To: ltp

Hi!
I started look at this patch however the first thing I've found out is that
our mountinfo parser is wrong. If you look at man 5 proc it says:

36 35 98:0 /mnt1 /mnt2 rw,noatime master:1 - ext3 /dev/root rw,errors=continue
(1)(2)(3)   (4)   (5)      (6)      (7)   (8) (9)   (10)         (11)


(7)  optional fields: zero or more fields of the form
     "tag[:value]"; see below.

So we cannot really parse the information with a static scanf() string,
since the number of elements in the line is not constant.

And it does fail on some of the machines I do have here since there is
no optional fields present.

So I guess that we will have to write a parser that reads that
information line by line after all.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 29+ messages in thread
* [LTP] [PATCH v3 2/2] syscalls/ioctl_loop05: Using LOOP_CONFIGURE to set direct io
  2020-07-24  2:05               ` [LTP] [PATCH v3 2/2] syscalls/ioctl_loop05: Using LOOP_CONFIGURE to set direct io Yang Xu
  2020-07-29 11:39                 ` Cyril Hrubis
@ 2020-07-29 12:58                 ` Cyril Hrubis
  1 sibling, 0 replies; 29+ messages in thread
From: Cyril Hrubis @ 2020-07-29 12:58 UTC (permalink / raw)
  To: ltp

Hi!
> +	/*
> +	 * When we call loop_configure ioctl successfully and detach it,
> +	 * the subquent loop_configure without non-zero lo_offset or
> +	 * sizelimit may trigger the blk_update_request I/O error.
> +	 * To avoid this, sleep 1s to ensure last blk_update_request has
> +	 * completed.
> +	 */
> +	sleep(1);

This sounds to me like a kernel bug. Have you tried asking kernel
developers if this is expected behavior before we attempt work around
the problem in tests?

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 29+ messages in thread
* Re: [PATCH v2 2/2] syscalls/ioctl_loop05: Using LOOP_CONFIGURE to set direct io
  2020-07-10  6:39       ` [LTP] [PATCH v2 2/2] syscalls/ioctl_loop05: Using LOOP_CONFIGURE to set direct io Yang Xu
@ 2020-07-30  7:28           ` Yang Xu
  0 siblings, 0 replies; 29+ messages in thread
From: Yang Xu @ 2020-07-30  7:28 UTC (permalink / raw)
  To: chrubis, Martijn Coenen, linux-block; +Cc: ltp

Hi Martijn
CC block kernel guys

I have a question when using loop_configure ioctl to set direct io flag.
In ltp testcase ioctl_loop05, I modify this case as the follow (Using 
loop_configure ioctl to set direct io mode with different 
logical_block_size).  But sometimes I met a problem that loop_configure 
ioctl succeed but the flag doesn't take effect.

the test (need to merge this patch[1] and remove sleep)
ioctl_loop05.c:132: INFO: Using LOOP_SET_DIRECT_IO with offset less than 
logical_block_size
ioctl_loop05.c:84: PASS: In non dio mode, lo_flags doesn't have 
LO_FLAGS_DIRECT_IO flag
ioctl_loop05.c:86: PASS: /sys/block/loop0/loop/dio = 0
ioctl_loop05.c:106: PASS: set direct io failed as expected: EINVAL (22)
ioctl_loop05.c:132: INFO: Using LOOP_CONFIGURE without setting lo_offset 
or sizelimit
ioctl_loop05.c:80: PASS: In dio mode, lo_flags has LO_FLAGS_DIRECT_IO flag
ioctl_loop05.c:86: PASS: /sys/block/loop0/loop/dio = 1
ioctl_loop05.c:132: INFO: Using LOOP_CONFIGURE With offset equal to 
logical_block_size
ioctl_loop05.c:80: PASS: In dio mode, lo_flags has LO_FLAGS_DIRECT_IO flag
ioctl_loop05.c:86: PASS: /sys/block/loop0/loop/dio = 1
ioctl_loop05.c:132: INFO: Using LOOP_CONFIGURE witg offset less than 
logical_block_size
ioctl_loop05.c:80: FAIL: In non dio mode, lo_flags has 
LO_FLAGS_DIRECT_IO flag
ioctl_loop05.c:86: FAIL: /sys/block/loop0/loop/dio != 0 got 1

Summary:
passed   17
failed   2
skipped  0
warnings 0

dmesg

[75103.201861] loop_set_status: loop0 () has still dirty pages (nrpages=3)
[75103.321850] blk_update_request: I/O error, dev loop0, sector 2047 op 
0x0:(READ) flags 0x80700 phys_seg 1 prio class 0
[75103.337105] blk_update_request: I/O error, dev loop0, sector 2047 op 
0x0:(READ) flags 0x0 phys_seg 1 prio class 0
[75103.337816] Buffer I/O error on dev loop0, logical block 255, async 
page read

It seems that the last blk_update_request has not completed but the 
subquent blk request (loop_configure ioctl with non zero size or logic 
block size triggers) has started.So kernel has this warning.Is it right? 
Is it a expected behaviour?

[1]https://patchwork.ozlabs.org/project/ltp/patch/1595556357-29932-2-git-send-email-xuyang2018.jy@cn.fujitsu.com/

Best Regards
Yang Xu

> Since kernel commit 3448914e8cc5("loop: Add LOOP_CONFIGURE ioctl"),
> it can explicitly request direct I/O mode by setting LO_FLAGS_DIRECT_IO
> in loop_config.info.lo_flags.
> 
> Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
> ---
>   .../kernel/syscalls/ioctl/ioctl_loop05.c      | 154 +++++++++++++-----
>   1 file changed, 117 insertions(+), 37 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop05.c b/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
> index e3c14faab..6abb27998 100644
> --- a/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
> +++ b/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
> @@ -19,6 +19,9 @@
>    * enabled but falls back to buffered I/O later on. This is the case at least
>    * for Btrfs. Because of that the test passes both with failure as well as
>    * success with non-zero offset.
> + *
> + * Also use LOOP_CONFIGURE to test this by setting LO_FLAGS_DIRECT_IO
> + * in loop_config.info.lo_flags.
>    */
>   
>   #include <stdio.h>
> @@ -32,8 +35,36 @@
>   #define DIO_MESSAGE "In dio mode"
>   #define NON_DIO_MESSAGE "In non dio mode"
>   
> -static char dev_path[1024], sys_loop_diopath[1024], backing_file_path[1024];;
> +static char dev_path[1024], sys_loop_diopath[1024], backing_file_path[1024];
>   static int dev_num, dev_fd, block_devfd, attach_flag, logical_block_size;
> +static int file_fd, loop_configure_sup = 1;
> +static struct loop_config loopconfig;
> +static struct loop_info loopinfo;
> +
> +static struct tcase {
> +	int multi; /*logical_block_size / 2 as unit */
> +	int dio_value;
> +	int ioctl_flag;
> +	char *message;
> +} tcases[] = {
> +	{0, 1, LOOP_SET_DIRECT_IO,
> +	"Using LOOP_SET_DIRET_IO without setting lo_offset or sizelimit"},
> +
> +	{2, 1, LOOP_SET_DIRECT_IO,
> +	"Using LOOP_SET_DIRECT_IO With offset equal to logical_block_size"},
> +
> +	{1, 0, LOOP_SET_DIRECT_IO,
> +	"Using LOOP_SET_DIRECT_IO with offset less than logical_block_size"},
> +
> +	{0, 1, LOOP_CONFIGURE,
> +	"Using LOOP_CONFIGURE without setting lo_offset or sizelimit"},
> +
> +	{2, 1, LOOP_CONFIGURE,
> +	"Using LOOP_CONFIGURE With offset equal to logical_block_size"},
> +
> +	{1, 0, LOOP_CONFIGURE,
> +	"Using LOOP_CONFIGURE witg offset less than logical_block_size"},
> +};
>   
>   static void check_dio_value(int flag)
>   {
> @@ -42,61 +73,94 @@ static void check_dio_value(int flag)
>   	memset(&loopinfoget, 0, sizeof(loopinfoget));
>   
>   	SAFE_IOCTL(dev_fd, LOOP_GET_STATUS, &loopinfoget);
> -	tst_res(TINFO, "%s", flag ? DIO_MESSAGE : NON_DIO_MESSAGE);
>   
>   	if (loopinfoget.lo_flags & LO_FLAGS_DIRECT_IO)
> -		tst_res(flag ? TPASS : TFAIL, "lo_flags has LO_FLAGS_DIRECT_IO flag");
> +		tst_res(flag ? TPASS : TFAIL,
> +			"%s, lo_flags has LO_FLAGS_DIRECT_IO flag",
> +			flag ? DIO_MESSAGE : NON_DIO_MESSAGE);
>   	else
> -		tst_res(flag ? TFAIL : TPASS, "lo_flags doesn't have LO_FLAGS_DIRECT_IO flag");
> +		tst_res(flag ? TFAIL : TPASS,
> +			"%s, lo_flags doesn't have LO_FLAGS_DIRECT_IO flag",
> +			flag ? DIO_MESSAGE : NON_DIO_MESSAGE);
>   
>   	TST_ASSERT_INT(sys_loop_diopath, flag);
>   }
>   
> -static void verify_ioctl_loop(void)
> +static void verify_ioctl_loop(unsigned int n)
>   {
> -	struct loop_info loopinfo;
> -
> -	memset(&loopinfo, 0, sizeof(loopinfo));
> -	TST_RETRY_FUNC(ioctl(dev_fd, LOOP_SET_STATUS, &loopinfo), TST_RETVAL_EQ0);
> +	if (tcases[n].ioctl_flag == LOOP_SET_DIRECT_IO) {
> +		TST_RETRY_FUNC(ioctl(dev_fd, LOOP_SET_STATUS, &loopinfo), TST_RETVAL_EQ0);
> +
> +		TEST(ioctl(dev_fd, LOOP_SET_DIRECT_IO, 1));
> +		if (TST_RET == 0) {
> +			if (tcases[n].dio_value)
> +				tst_res(TPASS, "set direct io succeeded");
> +			else
> +				tst_res(TPASS, "set direct io succeeded, offset is ignored");
> +			check_dio_value(1);
> +			SAFE_IOCTL(dev_fd, LOOP_SET_DIRECT_IO, 0);
> +			return;
> +		}
> +		if (TST_ERR == EINVAL && !tcases[n].dio_value)
> +			tst_res(TPASS | TTERRNO,
> +				"set direct io failed as expected");
> +		else
> +			tst_res(TFAIL | TTERRNO, "set direct io failed");
> +		return;
> +	}
> +	/*
> +	 * When we call loop_configure ioctl successfully and detach it,
> +	 * the subquent loop_configure without non-zero lo_offset or
> +	 * sizelimit may trigger the blk_update_request I/O error.
> +	 * To avoid this, sleep 1s to ensure last blk_update_request has
> +	 * completed.
> +	 */
> +	sleep(1);
> +	/*
> +	 * loop_cofigure calls loop_update_dio() function, it will ignore
> +	 * the result of setting dio. It is different from loop_set_dio.
> +	 */
> +	TST_RETRY_FUNC(ioctl(dev_fd, LOOP_CONFIGURE, &loopconfig), TST_RETVAL_EQ0);
> +	check_dio_value(tcases[n].dio_value);
> +	TST_RETRY_FUNC(ioctl(dev_fd, LOOP_CLR_FD, 0), TST_RETVAL_EQ0);
> +}
>   
> -	tst_res(TINFO, "Without setting lo_offset or sizelimit");
> -	SAFE_IOCTL(dev_fd, LOOP_SET_DIRECT_IO, 1);
> -	check_dio_value(1);
> +static void run(unsigned int n)
> +{
> +	struct tcase *tc = &tcases[n];
>   
> -	SAFE_IOCTL(dev_fd, LOOP_SET_DIRECT_IO, 0);
> -	check_dio_value(0);
> +	tst_res(TINFO, "%s", tc->message);
>   
> -	tst_res(TINFO, "With offset equal to logical_block_size");
> -	loopinfo.lo_offset = logical_block_size;
> -	TST_RETRY_FUNC(ioctl(dev_fd, LOOP_SET_STATUS, &loopinfo), TST_RETVAL_EQ0);
> -	TEST(ioctl(dev_fd, LOOP_SET_DIRECT_IO, 1));
> -	if (TST_RET == 0) {
> -		tst_res(TPASS, "LOOP_SET_DIRECT_IO succeeded");
> -		check_dio_value(1);
> +	if (tc->ioctl_flag == LOOP_SET_DIRECT_IO) {
> +		if (!attach_flag) {
> +			tst_attach_device(dev_path, "test.img");
> +			attach_flag = 1;
> +		}
>   		SAFE_IOCTL(dev_fd, LOOP_SET_DIRECT_IO, 0);
> -	} else {
> -		tst_res(TFAIL | TTERRNO, "LOOP_SET_DIRECT_IO failed");
> +		check_dio_value(0);
> +		loopinfo.lo_offset = logical_block_size * tc->multi / 2;
> +		verify_ioctl_loop(n);
> +		return;
>   	}
> -
> -	tst_res(TINFO, "With nonzero offset less than logical_block_size");
> -	loopinfo.lo_offset = logical_block_size / 2;
> -	TST_RETRY_FUNC(ioctl(dev_fd, LOOP_SET_STATUS, &loopinfo), TST_RETVAL_EQ0);
> -
> -	TEST(ioctl(dev_fd, LOOP_SET_DIRECT_IO, 1));
> -	if (TST_RET == 0) {
> -		tst_res(TPASS, "LOOP_SET_DIRECT_IO succeeded, offset is ignored");
> -		SAFE_IOCTL(dev_fd, LOOP_SET_DIRECT_IO, 0);
> +	if (tc->ioctl_flag == LOOP_CONFIGURE && !loop_configure_sup) {
> +		tst_res(TCONF, "LOOP_CONFIGURE ioctl not supported");
>   		return;
>   	}
> -	if (TST_ERR == EINVAL)
> -		tst_res(TPASS | TTERRNO, "LOOP_SET_DIRECT_IO failed as expected");
> -	else
> -		tst_res(TFAIL | TTERRNO, "LOOP_SET_DIRECT_IO failed expected EINVAL got");
> +	if (attach_flag) {
> +		SAFE_CLOSE(dev_fd);
> +		tst_detach_device(dev_path);
> +		attach_flag = 0;
> +	}
> +	if (dev_fd < 0)
> +		dev_fd = SAFE_OPEN(dev_path, O_RDWR);
> +	loopconfig.info.lo_offset = logical_block_size * tc->multi / 2;
> +	verify_ioctl_loop(n);
>   }
>   
>   static void setup(void)
>   {
>   	char bd_path[100];
> +	int ret;
>   
>   	if (tst_fs_type(".") == TST_TMPFS_MAGIC)
>   		tst_brk(TCONF, "tmpfd doesn't support O_DIRECT flag");
> @@ -128,8 +192,21 @@ static void setup(void)
>   	SAFE_IOCTL(block_devfd, BLKSSZGET, &logical_block_size);
>   	tst_res(TINFO, "backing dev(%s) logical_block_size is %d", bd_path, logical_block_size);
>   	SAFE_CLOSE(block_devfd);
> +
>   	if (logical_block_size > 512)
>   		TST_RETRY_FUNC(ioctl(dev_fd, LOOP_SET_BLOCK_SIZE, logical_block_size), TST_RETVAL_EQ0);
> +
> +	file_fd = SAFE_OPEN("test.img", O_RDWR);
> +	loopconfig.fd = -1;
> +	ret = ioctl(dev_fd, LOOP_CONFIGURE, &loopconfig);
> +	if (ret && errno != EBADF) {
> +		tst_res(TINFO | TERRNO, "LOOP_CONFIGURE is not supported");
> +		loop_configure_sup = 0;
> +		return;
> +	}
> +	loopconfig.block_size = logical_block_size;
> +	loopconfig.fd = file_fd;
> +	loopconfig.info.lo_flags = LO_FLAGS_DIRECT_IO;
>   }
>   
>   static void cleanup(void)
> @@ -138,6 +215,8 @@ static void cleanup(void)
>   		SAFE_CLOSE(dev_fd);
>   	if (block_devfd > 0)
>   		SAFE_CLOSE(block_devfd);
> +	if (file_fd > 0)
> +		SAFE_CLOSE(file_fd);
>   	if (attach_flag)
>   		tst_detach_device(dev_path);
>   }
> @@ -145,7 +224,8 @@ static void cleanup(void)
>   static struct tst_test test = {
>   	.setup = setup,
>   	.cleanup = cleanup,
> -	.test_all = verify_ioctl_loop,
> +	.test = run,
> +	.tcnt = ARRAY_SIZE(tcases),
>   	.needs_root = 1,
>   	.needs_tmpdir = 1,
>   	.needs_drivers = (const char *const []) {
> 



^ permalink raw reply	[flat|nested] 29+ messages in thread
* [LTP] [PATCH v2 2/2] syscalls/ioctl_loop05: Using LOOP_CONFIGURE to set direct io
@ 2020-07-30  7:28           ` Yang Xu
  0 siblings, 0 replies; 29+ messages in thread
From: Yang Xu @ 2020-07-30  7:28 UTC (permalink / raw)
  To: ltp

Hi Martijn
CC block kernel guys

I have a question when using loop_configure ioctl to set direct io flag.
In ltp testcase ioctl_loop05, I modify this case as the follow (Using 
loop_configure ioctl to set direct io mode with different 
logical_block_size).  But sometimes I met a problem that loop_configure 
ioctl succeed but the flag doesn't take effect.

the test (need to merge this patch[1] and remove sleep)
ioctl_loop05.c:132: INFO: Using LOOP_SET_DIRECT_IO with offset less than 
logical_block_size
ioctl_loop05.c:84: PASS: In non dio mode, lo_flags doesn't have 
LO_FLAGS_DIRECT_IO flag
ioctl_loop05.c:86: PASS: /sys/block/loop0/loop/dio = 0
ioctl_loop05.c:106: PASS: set direct io failed as expected: EINVAL (22)
ioctl_loop05.c:132: INFO: Using LOOP_CONFIGURE without setting lo_offset 
or sizelimit
ioctl_loop05.c:80: PASS: In dio mode, lo_flags has LO_FLAGS_DIRECT_IO flag
ioctl_loop05.c:86: PASS: /sys/block/loop0/loop/dio = 1
ioctl_loop05.c:132: INFO: Using LOOP_CONFIGURE With offset equal to 
logical_block_size
ioctl_loop05.c:80: PASS: In dio mode, lo_flags has LO_FLAGS_DIRECT_IO flag
ioctl_loop05.c:86: PASS: /sys/block/loop0/loop/dio = 1
ioctl_loop05.c:132: INFO: Using LOOP_CONFIGURE witg offset less than 
logical_block_size
ioctl_loop05.c:80: FAIL: In non dio mode, lo_flags has 
LO_FLAGS_DIRECT_IO flag
ioctl_loop05.c:86: FAIL: /sys/block/loop0/loop/dio != 0 got 1

Summary:
passed   17
failed   2
skipped  0
warnings 0

dmesg

[75103.201861] loop_set_status: loop0 () has still dirty pages (nrpages=3)
[75103.321850] blk_update_request: I/O error, dev loop0, sector 2047 op 
0x0:(READ) flags 0x80700 phys_seg 1 prio class 0
[75103.337105] blk_update_request: I/O error, dev loop0, sector 2047 op 
0x0:(READ) flags 0x0 phys_seg 1 prio class 0
[75103.337816] Buffer I/O error on dev loop0, logical block 255, async 
page read

It seems that the last blk_update_request has not completed but the 
subquent blk request (loop_configure ioctl with non zero size or logic 
block size triggers) has started.So kernel has this warning.Is it right? 
Is it a expected behaviour?

[1]https://patchwork.ozlabs.org/project/ltp/patch/1595556357-29932-2-git-send-email-xuyang2018.jy@cn.fujitsu.com/

Best Regards
Yang Xu

> Since kernel commit 3448914e8cc5("loop: Add LOOP_CONFIGURE ioctl"),
> it can explicitly request direct I/O mode by setting LO_FLAGS_DIRECT_IO
> in loop_config.info.lo_flags.
> 
> Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
> ---
>   .../kernel/syscalls/ioctl/ioctl_loop05.c      | 154 +++++++++++++-----
>   1 file changed, 117 insertions(+), 37 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop05.c b/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
> index e3c14faab..6abb27998 100644
> --- a/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
> +++ b/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
> @@ -19,6 +19,9 @@
>    * enabled but falls back to buffered I/O later on. This is the case at least
>    * for Btrfs. Because of that the test passes both with failure as well as
>    * success with non-zero offset.
> + *
> + * Also use LOOP_CONFIGURE to test this by setting LO_FLAGS_DIRECT_IO
> + * in loop_config.info.lo_flags.
>    */
>   
>   #include <stdio.h>
> @@ -32,8 +35,36 @@
>   #define DIO_MESSAGE "In dio mode"
>   #define NON_DIO_MESSAGE "In non dio mode"
>   
> -static char dev_path[1024], sys_loop_diopath[1024], backing_file_path[1024];;
> +static char dev_path[1024], sys_loop_diopath[1024], backing_file_path[1024];
>   static int dev_num, dev_fd, block_devfd, attach_flag, logical_block_size;
> +static int file_fd, loop_configure_sup = 1;
> +static struct loop_config loopconfig;
> +static struct loop_info loopinfo;
> +
> +static struct tcase {
> +	int multi; /*logical_block_size / 2 as unit */
> +	int dio_value;
> +	int ioctl_flag;
> +	char *message;
> +} tcases[] = {
> +	{0, 1, LOOP_SET_DIRECT_IO,
> +	"Using LOOP_SET_DIRET_IO without setting lo_offset or sizelimit"},
> +
> +	{2, 1, LOOP_SET_DIRECT_IO,
> +	"Using LOOP_SET_DIRECT_IO With offset equal to logical_block_size"},
> +
> +	{1, 0, LOOP_SET_DIRECT_IO,
> +	"Using LOOP_SET_DIRECT_IO with offset less than logical_block_size"},
> +
> +	{0, 1, LOOP_CONFIGURE,
> +	"Using LOOP_CONFIGURE without setting lo_offset or sizelimit"},
> +
> +	{2, 1, LOOP_CONFIGURE,
> +	"Using LOOP_CONFIGURE With offset equal to logical_block_size"},
> +
> +	{1, 0, LOOP_CONFIGURE,
> +	"Using LOOP_CONFIGURE witg offset less than logical_block_size"},
> +};
>   
>   static void check_dio_value(int flag)
>   {
> @@ -42,61 +73,94 @@ static void check_dio_value(int flag)
>   	memset(&loopinfoget, 0, sizeof(loopinfoget));
>   
>   	SAFE_IOCTL(dev_fd, LOOP_GET_STATUS, &loopinfoget);
> -	tst_res(TINFO, "%s", flag ? DIO_MESSAGE : NON_DIO_MESSAGE);
>   
>   	if (loopinfoget.lo_flags & LO_FLAGS_DIRECT_IO)
> -		tst_res(flag ? TPASS : TFAIL, "lo_flags has LO_FLAGS_DIRECT_IO flag");
> +		tst_res(flag ? TPASS : TFAIL,
> +			"%s, lo_flags has LO_FLAGS_DIRECT_IO flag",
> +			flag ? DIO_MESSAGE : NON_DIO_MESSAGE);
>   	else
> -		tst_res(flag ? TFAIL : TPASS, "lo_flags doesn't have LO_FLAGS_DIRECT_IO flag");
> +		tst_res(flag ? TFAIL : TPASS,
> +			"%s, lo_flags doesn't have LO_FLAGS_DIRECT_IO flag",
> +			flag ? DIO_MESSAGE : NON_DIO_MESSAGE);
>   
>   	TST_ASSERT_INT(sys_loop_diopath, flag);
>   }
>   
> -static void verify_ioctl_loop(void)
> +static void verify_ioctl_loop(unsigned int n)
>   {
> -	struct loop_info loopinfo;
> -
> -	memset(&loopinfo, 0, sizeof(loopinfo));
> -	TST_RETRY_FUNC(ioctl(dev_fd, LOOP_SET_STATUS, &loopinfo), TST_RETVAL_EQ0);
> +	if (tcases[n].ioctl_flag == LOOP_SET_DIRECT_IO) {
> +		TST_RETRY_FUNC(ioctl(dev_fd, LOOP_SET_STATUS, &loopinfo), TST_RETVAL_EQ0);
> +
> +		TEST(ioctl(dev_fd, LOOP_SET_DIRECT_IO, 1));
> +		if (TST_RET == 0) {
> +			if (tcases[n].dio_value)
> +				tst_res(TPASS, "set direct io succeeded");
> +			else
> +				tst_res(TPASS, "set direct io succeeded, offset is ignored");
> +			check_dio_value(1);
> +			SAFE_IOCTL(dev_fd, LOOP_SET_DIRECT_IO, 0);
> +			return;
> +		}
> +		if (TST_ERR == EINVAL && !tcases[n].dio_value)
> +			tst_res(TPASS | TTERRNO,
> +				"set direct io failed as expected");
> +		else
> +			tst_res(TFAIL | TTERRNO, "set direct io failed");
> +		return;
> +	}
> +	/*
> +	 * When we call loop_configure ioctl successfully and detach it,
> +	 * the subquent loop_configure without non-zero lo_offset or
> +	 * sizelimit may trigger the blk_update_request I/O error.
> +	 * To avoid this, sleep 1s to ensure last blk_update_request has
> +	 * completed.
> +	 */
> +	sleep(1);
> +	/*
> +	 * loop_cofigure calls loop_update_dio() function, it will ignore
> +	 * the result of setting dio. It is different from loop_set_dio.
> +	 */
> +	TST_RETRY_FUNC(ioctl(dev_fd, LOOP_CONFIGURE, &loopconfig), TST_RETVAL_EQ0);
> +	check_dio_value(tcases[n].dio_value);
> +	TST_RETRY_FUNC(ioctl(dev_fd, LOOP_CLR_FD, 0), TST_RETVAL_EQ0);
> +}
>   
> -	tst_res(TINFO, "Without setting lo_offset or sizelimit");
> -	SAFE_IOCTL(dev_fd, LOOP_SET_DIRECT_IO, 1);
> -	check_dio_value(1);
> +static void run(unsigned int n)
> +{
> +	struct tcase *tc = &tcases[n];
>   
> -	SAFE_IOCTL(dev_fd, LOOP_SET_DIRECT_IO, 0);
> -	check_dio_value(0);
> +	tst_res(TINFO, "%s", tc->message);
>   
> -	tst_res(TINFO, "With offset equal to logical_block_size");
> -	loopinfo.lo_offset = logical_block_size;
> -	TST_RETRY_FUNC(ioctl(dev_fd, LOOP_SET_STATUS, &loopinfo), TST_RETVAL_EQ0);
> -	TEST(ioctl(dev_fd, LOOP_SET_DIRECT_IO, 1));
> -	if (TST_RET == 0) {
> -		tst_res(TPASS, "LOOP_SET_DIRECT_IO succeeded");
> -		check_dio_value(1);
> +	if (tc->ioctl_flag == LOOP_SET_DIRECT_IO) {
> +		if (!attach_flag) {
> +			tst_attach_device(dev_path, "test.img");
> +			attach_flag = 1;
> +		}
>   		SAFE_IOCTL(dev_fd, LOOP_SET_DIRECT_IO, 0);
> -	} else {
> -		tst_res(TFAIL | TTERRNO, "LOOP_SET_DIRECT_IO failed");
> +		check_dio_value(0);
> +		loopinfo.lo_offset = logical_block_size * tc->multi / 2;
> +		verify_ioctl_loop(n);
> +		return;
>   	}
> -
> -	tst_res(TINFO, "With nonzero offset less than logical_block_size");
> -	loopinfo.lo_offset = logical_block_size / 2;
> -	TST_RETRY_FUNC(ioctl(dev_fd, LOOP_SET_STATUS, &loopinfo), TST_RETVAL_EQ0);
> -
> -	TEST(ioctl(dev_fd, LOOP_SET_DIRECT_IO, 1));
> -	if (TST_RET == 0) {
> -		tst_res(TPASS, "LOOP_SET_DIRECT_IO succeeded, offset is ignored");
> -		SAFE_IOCTL(dev_fd, LOOP_SET_DIRECT_IO, 0);
> +	if (tc->ioctl_flag == LOOP_CONFIGURE && !loop_configure_sup) {
> +		tst_res(TCONF, "LOOP_CONFIGURE ioctl not supported");
>   		return;
>   	}
> -	if (TST_ERR == EINVAL)
> -		tst_res(TPASS | TTERRNO, "LOOP_SET_DIRECT_IO failed as expected");
> -	else
> -		tst_res(TFAIL | TTERRNO, "LOOP_SET_DIRECT_IO failed expected EINVAL got");
> +	if (attach_flag) {
> +		SAFE_CLOSE(dev_fd);
> +		tst_detach_device(dev_path);
> +		attach_flag = 0;
> +	}
> +	if (dev_fd < 0)
> +		dev_fd = SAFE_OPEN(dev_path, O_RDWR);
> +	loopconfig.info.lo_offset = logical_block_size * tc->multi / 2;
> +	verify_ioctl_loop(n);
>   }
>   
>   static void setup(void)
>   {
>   	char bd_path[100];
> +	int ret;
>   
>   	if (tst_fs_type(".") == TST_TMPFS_MAGIC)
>   		tst_brk(TCONF, "tmpfd doesn't support O_DIRECT flag");
> @@ -128,8 +192,21 @@ static void setup(void)
>   	SAFE_IOCTL(block_devfd, BLKSSZGET, &logical_block_size);
>   	tst_res(TINFO, "backing dev(%s) logical_block_size is %d", bd_path, logical_block_size);
>   	SAFE_CLOSE(block_devfd);
> +
>   	if (logical_block_size > 512)
>   		TST_RETRY_FUNC(ioctl(dev_fd, LOOP_SET_BLOCK_SIZE, logical_block_size), TST_RETVAL_EQ0);
> +
> +	file_fd = SAFE_OPEN("test.img", O_RDWR);
> +	loopconfig.fd = -1;
> +	ret = ioctl(dev_fd, LOOP_CONFIGURE, &loopconfig);
> +	if (ret && errno != EBADF) {
> +		tst_res(TINFO | TERRNO, "LOOP_CONFIGURE is not supported");
> +		loop_configure_sup = 0;
> +		return;
> +	}
> +	loopconfig.block_size = logical_block_size;
> +	loopconfig.fd = file_fd;
> +	loopconfig.info.lo_flags = LO_FLAGS_DIRECT_IO;
>   }
>   
>   static void cleanup(void)
> @@ -138,6 +215,8 @@ static void cleanup(void)
>   		SAFE_CLOSE(dev_fd);
>   	if (block_devfd > 0)
>   		SAFE_CLOSE(block_devfd);
> +	if (file_fd > 0)
> +		SAFE_CLOSE(file_fd);
>   	if (attach_flag)
>   		tst_detach_device(dev_path);
>   }
> @@ -145,7 +224,8 @@ static void cleanup(void)
>   static struct tst_test test = {
>   	.setup = setup,
>   	.cleanup = cleanup,
> -	.test_all = verify_ioctl_loop,
> +	.test = run,
> +	.tcnt = ARRAY_SIZE(tcases),
>   	.needs_root = 1,
>   	.needs_tmpdir = 1,
>   	.needs_drivers = (const char *const []) {
> 



^ permalink raw reply	[flat|nested] 29+ messages in thread
* [LTP] [PATCH v3 2/2] syscalls/ioctl_loop05: Using LOOP_CONFIGURE to set direct io
  2020-07-29 11:39                 ` Cyril Hrubis
@ 2020-07-30  8:49                   ` Yang Xu
  2020-07-30  9:28                     ` Cyril Hrubis
  0 siblings, 1 reply; 29+ messages in thread
From: Yang Xu @ 2020-07-30  8:49 UTC (permalink / raw)
  To: ltp

HI Cyril

> Hi!
> I started look at this patch however the first thing I've found out is that
> our mountinfo parser is wrong. If you look at man 5 proc it says:
> 
> 36 35 98:0 /mnt1 /mnt2 rw,noatime master:1 - ext3 /dev/root rw,errors=continue
> (1)(2)(3)   (4)   (5)      (6)      (7)   (8) (9)   (10)         (11)
> 
> 
> (7)  optional fields: zero or more fields of the form
>       "tag[:value]"; see below.
> 
> So we cannot really parse the information with a static scanf() string,
> since the number of elements in the line is not constant.
> 
> And it does fail on some of the machines I do have here since there is
> no optional fields present.
> 
> So I guess that we will have to write a parser that reads that
> information line by line after all.
I doubt how machies will have more or zero fields in (7). But I think 
you are right,
How about using the (3) field and second to last field. Then we can 
avoid zero or more filed in (7). the code as below?

diff --git a/lib/tst_device.c b/lib/tst_device.c
index 8d8bc5b40..36d6666fe 100644
--- a/lib/tst_device.c
+++ b/lib/tst_device.c
@@ -497,16 +497,31 @@ unsigned long tst_dev_bytes_written(const char *dev)

  void tst_find_backing_dev(const char *path, char *dev)
  {
-       char fmt[1024];
+       char fmt[20];
         struct stat buf;
+       FILE *file;
+       char line[PATH_MAX];
+       char *pre = NULL;
+       char *next = NULL;

         if (stat(path, &buf) < 0)
                  tst_brkm(TWARN | TERRNO, NULL, "stat() failed");

-       snprintf(fmt, sizeof(fmt), "%%*i %%*i %u:%u %%*s %%*s %%*s %%*s 
%%*s %%*s %%s %%*s",
-                       major(buf.st_dev), minor(buf.st_dev));
+       snprintf(fmt, sizeof(fmt), "%u:%u", major(buf.st_dev), 
minor(buf.st_dev));
+       file = SAFE_FOPEN(NULL, "/proc/self/mountinfo", "r");

-       SAFE_FILE_LINES_SCANF(NULL, "/proc/self/mountinfo", fmt, dev);
+       while (fgets(line, sizeof(line), file)) {
+               if (strstr(line, fmt) != NULL) {
+                       pre = strtok_r(line, " ", &next);
+                       while(pre != NULL) {
+                               strcpy(dev, pre);
+                               pre = strtok_r(NULL, " ", &next);
+                               if (strlen(next) == 0)
+                                       break;
+                       }
+                       break;
+               }
+       }

         if (stat(dev, &buf) < 0)
                  tst_brkm(TWARN | TERRNO, NULL, "stat(%s) failed", dev);

> 



^ permalink raw reply related	[flat|nested] 29+ messages in thread
* [LTP] [PATCH v3 2/2] syscalls/ioctl_loop05: Using LOOP_CONFIGURE to set direct io
  2020-07-30  8:49                   ` Yang Xu
@ 2020-07-30  9:28                     ` Cyril Hrubis
  2020-07-30 10:08                       ` Yang Xu
  0 siblings, 1 reply; 29+ messages in thread
From: Cyril Hrubis @ 2020-07-30  9:28 UTC (permalink / raw)
  To: ltp

Hi!
> > So I guess that we will have to write a parser that reads that
> > information line by line after all.
> I doubt how machies will have more or zero fields in (7). But I think 
> you are right,

Well that's what I do have here.

> How about using the (3) field and second to last field. Then we can 
> avoid zero or more filed in (7). the code as below??

Actually looking into util-linux code it says that th the optional
fields are terminated with " - ", see:

https://git.kernel.org/pub/scm/utils/util-linux/util-linux.git/tree/libmount/src/tab_parse.c#n177

So I guess the safest option would be:

* Match the line by major:minor as you do
* Then strstr() for " - " should land us directly to field (8)

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 29+ messages in thread
* [LTP] [PATCH v3 2/2] syscalls/ioctl_loop05: Using LOOP_CONFIGURE to set direct io
  2020-07-30  9:28                     ` Cyril Hrubis
@ 2020-07-30 10:08                       ` Yang Xu
  2020-07-30 10:38                         ` Cyril Hrubis
  0 siblings, 1 reply; 29+ messages in thread
From: Yang Xu @ 2020-07-30 10:08 UTC (permalink / raw)
  To: ltp

HI Cyril
> Hi!
>>> So I guess that we will have to write a parser that reads that
>>> information line by line after all.
>> I doubt how machies will have more or zero fields in (7). But I think
>> you are right,
> 
> Well that's what I do have here.
> 
>> How about using the (3) field and second to last field. Then we can
>> avoid zero or more filed in (7). the code as below??
> 
> Actually looking into util-linux code it says that th the optional
> fields are terminated with " - ", see:
> 
> https://git.kernel.org/pub/scm/utils/util-linux/util-linux.git/tree/libmount/src/tab_parse.c#n177
> 
> So I guess the safest option would be:
> 
> * Match the line by major:minor as you do
> * Then strstr() for " - " should land us directly to field (8)
Yes, using " - " more easily and faster. code as below?

index 8d8bc5b40..bdd93f19e 100644
--- a/lib/tst_device.c
+++ b/lib/tst_device.c
@@ -497,17 +497,30 @@ unsigned long tst_dev_bytes_written(const char *dev)

  void tst_find_backing_dev(const char *path, char *dev)
  {
-       char fmt[1024];
+       char fmt[20];
         struct stat buf;
+       FILE *file;
+       char line[PATH_MAX];
+       char *pre = NULL;
+       char *next = NULL;

         if (stat(path, &buf) < 0)
                  tst_brkm(TWARN | TERRNO, NULL, "stat() failed");

-       snprintf(fmt, sizeof(fmt), "%%*i %%*i %u:%u %%*s %%*s %%*s %%*s 
%%*s %%*s %%s %%*s",
-                       major(buf.st_dev), minor(buf.st_dev));
+       snprintf(fmt, sizeof(fmt), "%u:%u", major(buf.st_dev), 
minor(buf.st_dev));
+       file = SAFE_FOPEN(NULL, "/proc/self/mountinfo", "r");

-       SAFE_FILE_LINES_SCANF(NULL, "/proc/self/mountinfo", fmt, dev);
+       while (fgets(line, sizeof(line), file)) {
+               if (strstr(line, fmt) != NULL) {
+                       pre = strstr(line, " - ");
+                       pre = strtok_r(pre, " ", &next);
+                       pre = strtok_r(NULL, " ", &next);
+                       pre = strtok_r(NULL, " ", &next);
+                       strcpy(dev, pre);
+               }
+       }

+       SAFE_FCLOSE(NULL, file);
         if (stat(dev, &buf) < 0)
                  tst_brkm(TWARN | TERRNO, NULL, "stat(%s) failed", dev);


> 



^ permalink raw reply related	[flat|nested] 29+ messages in thread
* [LTP] [PATCH v3 2/2] syscalls/ioctl_loop05: Using LOOP_CONFIGURE to set direct io
  2020-07-30 10:08                       ` Yang Xu
@ 2020-07-30 10:38                         ` Cyril Hrubis
  0 siblings, 0 replies; 29+ messages in thread
From: Cyril Hrubis @ 2020-07-30 10:38 UTC (permalink / raw)
  To: ltp

Hi!
> index 8d8bc5b40..bdd93f19e 100644
> --- a/lib/tst_device.c
> +++ b/lib/tst_device.c
> @@ -497,17 +497,30 @@ unsigned long tst_dev_bytes_written(const char *dev)
> 
>   void tst_find_backing_dev(const char *path, char *dev)
>   {
> -       char fmt[1024];
> +       char fmt[20];
>          struct stat buf;
> +       FILE *file;
> +       char line[PATH_MAX];
> +       char *pre = NULL;
> +       char *next = NULL;
> 
>          if (stat(path, &buf) < 0)
>                   tst_brkm(TWARN | TERRNO, NULL, "stat() failed");
> 
> -       snprintf(fmt, sizeof(fmt), "%%*i %%*i %u:%u %%*s %%*s %%*s %%*s 
> %%*s %%*s %%s %%*s",
> -                       major(buf.st_dev), minor(buf.st_dev));
> +       snprintf(fmt, sizeof(fmt), "%u:%u", major(buf.st_dev), 
> minor(buf.st_dev));
> +       file = SAFE_FOPEN(NULL, "/proc/self/mountinfo", "r");
> 
> -       SAFE_FILE_LINES_SCANF(NULL, "/proc/self/mountinfo", fmt, dev);
> +       while (fgets(line, sizeof(line), file)) {
> +               if (strstr(line, fmt) != NULL) {
> +                       pre = strstr(line, " - ");
> +                       pre = strtok_r(pre, " ", &next);
> +                       pre = strtok_r(NULL, " ", &next);
> +                       pre = strtok_r(NULL, " ", &next);
> +                       strcpy(dev, pre);

We should break; here as well, since we already found the result.

> +               }
> +       }
> 
> +       SAFE_FCLOSE(NULL, file);
>          if (stat(dev, &buf) < 0)
>                   tst_brkm(TWARN | TERRNO, NULL, "stat(%s) failed", dev);

Otherwise it looks good.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 29+ messages in thread
* [LTP] [PATCH v3 1/2] syscalls/ioctl_loop06: Using LOOP_CONFIGURE to test invalid block size
  2020-07-29 10:43                 ` Yang Xu
@ 2020-07-31 14:15                   ` Cyril Hrubis
  0 siblings, 0 replies; 29+ messages in thread
From: Cyril Hrubis @ 2020-07-31 14:15 UTC (permalink / raw)
  To: ltp

Hi!
Pushed, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 29+ messages in thread
end of thread, other threads:[~2020-07-31 14:15 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-11 10:35 [LTP] [PATCH v1 0/4] *** Add LOOP_CONFIGURE ioctl test *** Yang Xu
2020-06-11 10:35 ` [LTP] [PATCH v1 1/4] lapi: Add fallback for LOOP_CONFIGURE ioctl and struct loop_config Yang Xu
2020-07-08 13:18   ` Cyril Hrubis
2020-06-11 10:35 ` [LTP] [PATCH v1 2/4] syscalls/ioctl_loop02: Using LOOP_CONFIGURE to set read_only Yang Xu
2020-07-08 13:15   ` Cyril Hrubis
2020-06-11 10:35 ` [LTP] [PATCH v1 3/4] syscalls/ioctl_loop08: Add LOOP_CONFIGURE error test with invalid block size Yang Xu
2020-07-08 14:00   ` Cyril Hrubis
2020-06-11 10:35 ` [LTP] [PATCH v1 4/4] syscalls/ioctl_loop09: Add LOOP_CONFIGURE ioctl test with direct I/O flag Yang Xu
2020-07-08 14:00   ` Cyril Hrubis
2020-07-10  6:39     ` [LTP] [PATCH v2 1/2] syscalls/ioctl_loop06: Using LOOP_CONFIGURE to test invalid block size Yang Xu
2020-07-10  6:39       ` [LTP] [PATCH v2 2/2] syscalls/ioctl_loop05: Using LOOP_CONFIGURE to set direct io Yang Xu
2020-07-30  7:28         ` Yang Xu
2020-07-30  7:28           ` [LTP] " Yang Xu
2020-07-22  9:45       ` [LTP] [PATCH v2 1/2] syscalls/ioctl_loop06: Using LOOP_CONFIGURE to test invalid block size Cyril Hrubis
2020-07-22 10:15         ` Yang Xu
2020-07-22 12:59           ` Cyril Hrubis
2020-07-23  9:41             ` Yang Xu
2020-07-24  2:05             ` [LTP] [PATCH v3 " Yang Xu
2020-07-24  2:05               ` [LTP] [PATCH v3 2/2] syscalls/ioctl_loop05: Using LOOP_CONFIGURE to set direct io Yang Xu
2020-07-29 11:39                 ` Cyril Hrubis
2020-07-30  8:49                   ` Yang Xu
2020-07-30  9:28                     ` Cyril Hrubis
2020-07-30 10:08                       ` Yang Xu
2020-07-30 10:38                         ` Cyril Hrubis
2020-07-29 12:58                 ` Cyril Hrubis
2020-07-29 10:07               ` [LTP] [PATCH v3 1/2] syscalls/ioctl_loop06: Using LOOP_CONFIGURE to test invalid block size Cyril Hrubis
2020-07-29 10:43                 ` Yang Xu
2020-07-31 14:15                   ` Cyril Hrubis
2020-07-06  1:45 ` [LTP] [PATCH v1 0/4] *** Add LOOP_CONFIGURE ioctl test *** Yang Xu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.