All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH] syscalls/ioctl_loop05: set loop blksize to bdev blksize
@ 2020-06-01 13:01 Jan Stancek
  2020-06-02  4:53 ` Yang Xu
  2020-06-02  8:42 ` [LTP] [PATCH v2] " Jan Stancek
  0 siblings, 2 replies; 6+ messages in thread
From: Jan Stancek @ 2020-06-01 13:01 UTC (permalink / raw)
  To: ltp

Test is failing on s390, where default loop blksize is less than
backing dev's blksize (4096):
  tst_test.c:1247: INFO: Timeout per run is 0h 05m 00s
  tst_device.c:88: INFO: Found free device 0 '/dev/loop0'
  ioctl_loop05.c:116: INFO: /dev/loop0 default logical_block_size is 512
  ioctl_loop05.c:62: INFO: Without setting lo_offset or sizelimit
  ioctl_loop05.c:63: BROK: ioctl(3,LOOP_SET_DIRECT_IO,...) failed: EINVAL (22)

Per kernel comment at __loop_update_dio(), direct io is supported
when "logical block size of loop is bigger than the backing device's".

Set loop blksize to one of backing device. Retry is there to avoid
EAGAIN warning "loop0 (test.img) has still dirty pages".

Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
 .../kernel/syscalls/ioctl/ioctl_loop05.c      | 21 +++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop05.c b/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
index 6c9ea2802981..a969978239a5 100644
--- a/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
+++ b/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
@@ -96,6 +96,9 @@ static void verify_ioctl_loop(void)
 
 static void setup(void)
 {
+	int fd;
+	struct stat buf;
+
 	if (tst_fs_type(".") == TST_TMPFS_MAGIC)
 		tst_brk(TCONF, "tmpfd doesn't support O_DIRECT flag");
 
@@ -105,6 +108,14 @@ static void setup(void)
 
 	sprintf(sys_loop_diopath, "/sys/block/loop%d/loop/dio", dev_num);
 	tst_fill_file("test.img", 0, 1024, 1024);
+
+	fd = SAFE_OPEN("test.img", O_RDONLY);
+	SAFE_FSTAT(fd, &buf);
+	SAFE_CLOSE(fd);
+
+	logical_block_size = buf.st_blksize;
+	tst_res(TINFO, "backing dev logical_block_size is %d", logical_block_size);
+
 	tst_attach_device(dev_path, "test.img");
 	attach_flag = 1;
 	dev_fd = SAFE_OPEN(dev_path, O_RDWR);
@@ -112,8 +123,14 @@ static void setup(void)
 	if (ioctl(dev_fd, LOOP_SET_DIRECT_IO, 0) && errno == EINVAL)
 		tst_brk(TCONF, "LOOP_SET_DIRECT_IO is not supported");
 
-	SAFE_IOCTL(dev_fd, BLKSSZGET, &logical_block_size);
-	tst_res(TINFO, "%s default logical_block_size is %d", dev_path, logical_block_size);
+	/*
+	 * from __loop_update_dio():
+	 *   We support direct I/O only if lo_offset is aligned with the
+	 *   logical I/O size of backing device, and the logical block
+	 *   size of loop is bigger than the backing device's and the loop
+	 *   needn't transform transfer.
+	 */
+	TST_RETRY_FUNC(ioctl(dev_fd, LOOP_SET_BLOCK_SIZE, logical_block_size), TST_RETVAL_EQ0);
 }
 
 static void cleanup(void)
-- 
2.18.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [LTP] [PATCH] syscalls/ioctl_loop05: set loop blksize to bdev blksize
  2020-06-01 13:01 [LTP] [PATCH] syscalls/ioctl_loop05: set loop blksize to bdev blksize Jan Stancek
@ 2020-06-02  4:53 ` Yang Xu
  2020-06-02  6:16   ` Jan Stancek
  2020-06-02  8:42 ` [LTP] [PATCH v2] " Jan Stancek
  1 sibling, 1 reply; 6+ messages in thread
From: Yang Xu @ 2020-06-02  4:53 UTC (permalink / raw)
  To: ltp

Hi Jan


> Test is failing on s390, where default loop blksize is less than
> backing dev's blksize (4096):
>    tst_test.c:1247: INFO: Timeout per run is 0h 05m 00s
>    tst_device.c:88: INFO: Found free device 0 '/dev/loop0'
>    ioctl_loop05.c:116: INFO: /dev/loop0 default logical_block_size is 512
>    ioctl_loop05.c:62: INFO: Without setting lo_offset or sizelimit
>    ioctl_loop05.c:63: BROK: ioctl(3,LOOP_SET_DIRECT_IO,...) failed: EINVAL (22)
> 
After looking kernel code, I think removing BLKSSZGET ioctl is ok.
Also since kernel commit 85560117d00 ("loop: change queue block size to 
match when using DIO"), it will change logic block size automaticly when 
fd is opened with O_DIRECT. Can we use it?just a suggestion??
> Per kernel comment at __loop_update_dio(), direct io is supported
> when "logical block size of loop is bigger than the backing device's".
> 
> Set loop blksize to one of backing device. Retry is there to avoid
> EAGAIN warning "loop0 (test.img) has still dirty pages".
> 
> Signed-off-by: Jan Stancek <jstancek@redhat.com>
> ---
>   .../kernel/syscalls/ioctl/ioctl_loop05.c      | 21 +++++++++++++++++--
>   1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop05.c b/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
> index 6c9ea2802981..a969978239a5 100644
> --- a/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
> +++ b/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
> @@ -96,6 +96,9 @@ static void verify_ioctl_loop(void)
>   
>   static void setup(void)
>   {
> +	int fd;
> +	struct stat buf;
> +
>   	if (tst_fs_type(".") == TST_TMPFS_MAGIC)
>   		tst_brk(TCONF, "tmpfd doesn't support O_DIRECT flag");
>   
> @@ -105,6 +108,14 @@ static void setup(void)
>   
>   	sprintf(sys_loop_diopath, "/sys/block/loop%d/loop/dio", dev_num);
>   	tst_fill_file("test.img", 0, 1024, 1024);
> +
> +	fd = SAFE_OPEN("test.img", O_RDONLY);
> +	SAFE_FSTAT(fd, &buf);
> +	SAFE_CLOSE(fd);
> +
> +	logical_block_size = buf.st_blksize;
> +	tst_res(TINFO, "backing dev logical_block_size is %d", logical_block_size);
> +
>   	tst_attach_device(dev_path, "test.img");
>   	attach_flag = 1;
>   	dev_fd = SAFE_OPEN(dev_path, O_RDWR);
> @@ -112,8 +123,14 @@ static void setup(void)
>   	if (ioctl(dev_fd, LOOP_SET_DIRECT_IO, 0) && errno == EINVAL)
>   		tst_brk(TCONF, "LOOP_SET_DIRECT_IO is not supported");
>   
> -	SAFE_IOCTL(dev_fd, BLKSSZGET, &logical_block_size);
> -	tst_res(TINFO, "%s default logical_block_size is %d", dev_path, logical_block_size);
> +	/*
> +	 * from __loop_update_dio():
> +	 *   We support direct I/O only if lo_offset is aligned with the
> +	 *   logical I/O size of backing device, and the logical block
> +	 *   size of loop is bigger than the backing device's and the loop
> +	 *   needn't transform transfer.
> +	 */
> +	TST_RETRY_FUNC(ioctl(dev_fd, LOOP_SET_BLOCK_SIZE, logical_block_size), TST_RETVAL_EQ0);
LOOP_SET_DIRECT_IO was introduce since 4.10 and  LOOP_SET_BLOCK_SIZE 
ioctl was introduced since kernel 4.14, I guess we should add a check 
for LOOP_SET_BLOCK_SIZE in here.

Best Regards
Yang Xu
>   }
>   
>   static void cleanup(void)
> 



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [LTP] [PATCH] syscalls/ioctl_loop05: set loop blksize to bdev blksize
  2020-06-02  4:53 ` Yang Xu
@ 2020-06-02  6:16   ` Jan Stancek
  0 siblings, 0 replies; 6+ messages in thread
From: Jan Stancek @ 2020-06-02  6:16 UTC (permalink / raw)
  To: ltp



----- Original Message -----
> Hi Jan
> 
> 
> > Test is failing on s390, where default loop blksize is less than
> > backing dev's blksize (4096):
> >    tst_test.c:1247: INFO: Timeout per run is 0h 05m 00s
> >    tst_device.c:88: INFO: Found free device 0 '/dev/loop0'
> >    ioctl_loop05.c:116: INFO: /dev/loop0 default logical_block_size is 512
> >    ioctl_loop05.c:62: INFO: Without setting lo_offset or sizelimit
> >    ioctl_loop05.c:63: BROK: ioctl(3,LOOP_SET_DIRECT_IO,...) failed: EINVAL
> >    (22)
> > 
> After looking kernel code, I think removing BLKSSZGET ioctl is ok.
> Also since kernel commit 85560117d00 ("loop: change queue block size to
> match when using DIO"), it will change logic block size automaticly when
> fd is opened with O_DIRECT. Can we use it??ust a suggestion??

We could, but then we need to limit test to kernels 5.4 and later,
since we depend on behavior introduced by that commit.

> > Per kernel comment at __loop_update_dio(), direct io is supported
> > when "logical block size of loop is bigger than the backing device's".
> > 
> > Set loop blksize to one of backing device. Retry is there to avoid
> > EAGAIN warning "loop0 (test.img) has still dirty pages".
> > 
> > Signed-off-by: Jan Stancek <jstancek@redhat.com>
> > ---
> >   .../kernel/syscalls/ioctl/ioctl_loop05.c      | 21 +++++++++++++++++--
> >   1 file changed, 19 insertions(+), 2 deletions(-)
> > 
> > diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
> > b/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
> > index 6c9ea2802981..a969978239a5 100644
> > --- a/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
> > +++ b/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
> > @@ -96,6 +96,9 @@ static void verify_ioctl_loop(void)
> >   
> >   static void setup(void)
> >   {
> > +	int fd;
> > +	struct stat buf;
> > +
> >   	if (tst_fs_type(".") == TST_TMPFS_MAGIC)
> >   		tst_brk(TCONF, "tmpfd doesn't support O_DIRECT flag");
> >   
> > @@ -105,6 +108,14 @@ static void setup(void)
> >   
> >   	sprintf(sys_loop_diopath, "/sys/block/loop%d/loop/dio", dev_num);
> >   	tst_fill_file("test.img", 0, 1024, 1024);
> > +
> > +	fd = SAFE_OPEN("test.img", O_RDONLY);
> > +	SAFE_FSTAT(fd, &buf);
> > +	SAFE_CLOSE(fd);
> > +
> > +	logical_block_size = buf.st_blksize;
> > +	tst_res(TINFO, "backing dev logical_block_size is %d",
> > logical_block_size);
> > +
> >   	tst_attach_device(dev_path, "test.img");
> >   	attach_flag = 1;
> >   	dev_fd = SAFE_OPEN(dev_path, O_RDWR);
> > @@ -112,8 +123,14 @@ static void setup(void)
> >   	if (ioctl(dev_fd, LOOP_SET_DIRECT_IO, 0) && errno == EINVAL)
> >   		tst_brk(TCONF, "LOOP_SET_DIRECT_IO is not supported");
> >   
> > -	SAFE_IOCTL(dev_fd, BLKSSZGET, &logical_block_size);
> > -	tst_res(TINFO, "%s default logical_block_size is %d", dev_path,
> > logical_block_size);
> > +	/*
> > +	 * from __loop_update_dio():
> > +	 *   We support direct I/O only if lo_offset is aligned with the
> > +	 *   logical I/O size of backing device, and the logical block
> > +	 *   size of loop is bigger than the backing device's and the loop
> > +	 *   needn't transform transfer.
> > +	 */
> > +	TST_RETRY_FUNC(ioctl(dev_fd, LOOP_SET_BLOCK_SIZE, logical_block_size),
> > TST_RETVAL_EQ0);
> LOOP_SET_DIRECT_IO was introduce since 4.10 and  LOOP_SET_BLOCK_SIZE
> ioctl was introduced since kernel 4.14, I guess we should add a check
> for LOOP_SET_BLOCK_SIZE in here.

Good point. 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [LTP] [PATCH v2] syscalls/ioctl_loop05: set loop blksize to bdev blksize
  2020-06-01 13:01 [LTP] [PATCH] syscalls/ioctl_loop05: set loop blksize to bdev blksize Jan Stancek
  2020-06-02  4:53 ` Yang Xu
@ 2020-06-02  8:42 ` Jan Stancek
  2020-06-02  8:57   ` Yang Xu
  1 sibling, 1 reply; 6+ messages in thread
From: Jan Stancek @ 2020-06-02  8:42 UTC (permalink / raw)
  To: ltp

Test is failing on s390, where default loop blksize is less than
backing dev's blksize (4096):
  tst_test.c:1247: INFO: Timeout per run is 0h 05m 00s
  tst_device.c:88: INFO: Found free device 0 '/dev/loop0'
  ioctl_loop05.c:116: INFO: /dev/loop0 default logical_block_size is 512
  ioctl_loop05.c:62: INFO: Without setting lo_offset or sizelimit
  ioctl_loop05.c:63: BROK: ioctl(3,LOOP_SET_DIRECT_IO,...) failed: EINVAL (22)

Per kernel comment at __loop_update_dio(), direct io is supported
when ".. logical block size of loop is bigger than the backing device's".

Set loop blksize to one of backing device. Retry is there to avoid
EAGAIN warning "loop0 (test.img) has still dirty pages".

Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
 .../kernel/syscalls/ioctl/ioctl_loop05.c      | 26 +++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

Changes in v2:
- check if LOOP_SET_BLOCK_SIZE is supported in setup()

diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop05.c b/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
index 6c9ea2802981..2a3c127959aa 100644
--- a/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
+++ b/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
@@ -96,6 +96,9 @@ static void verify_ioctl_loop(void)
 
 static void setup(void)
 {
+	int fd;
+	struct stat buf;
+
 	if (tst_fs_type(".") == TST_TMPFS_MAGIC)
 		tst_brk(TCONF, "tmpfd doesn't support O_DIRECT flag");
 
@@ -105,6 +108,14 @@ static void setup(void)
 
 	sprintf(sys_loop_diopath, "/sys/block/loop%d/loop/dio", dev_num);
 	tst_fill_file("test.img", 0, 1024, 1024);
+
+	fd = SAFE_OPEN("test.img", O_RDONLY);
+	SAFE_FSTAT(fd, &buf);
+	SAFE_CLOSE(fd);
+
+	logical_block_size = buf.st_blksize;
+	tst_res(TINFO, "backing dev logical_block_size is %d", logical_block_size);
+
 	tst_attach_device(dev_path, "test.img");
 	attach_flag = 1;
 	dev_fd = SAFE_OPEN(dev_path, O_RDWR);
@@ -112,8 +123,19 @@ static void setup(void)
 	if (ioctl(dev_fd, LOOP_SET_DIRECT_IO, 0) && errno == EINVAL)
 		tst_brk(TCONF, "LOOP_SET_DIRECT_IO is not supported");
 
-	SAFE_IOCTL(dev_fd, BLKSSZGET, &logical_block_size);
-	tst_res(TINFO, "%s default logical_block_size is %d", dev_path, logical_block_size);
+	/*
+	 * from __loop_update_dio():
+	 *   We support direct I/O only if lo_offset is aligned with the
+	 *   logical I/O size of backing device, and the logical block
+	 *   size of loop is bigger than the backing device's and the loop
+	 *   needn't transform transfer.
+	 */
+	if (ioctl(dev_fd, LOOP_SET_BLOCK_SIZE, logical_block_size) != 0) {
+		if (errno == EINVAL)
+			tst_brk(TCONF, "LOOP_SET_BLOCK_SIZE is not supported");
+		TST_RETRY_FUNC(ioctl(dev_fd, LOOP_SET_BLOCK_SIZE,
+			logical_block_size), TST_RETVAL_EQ0);
+	}
 }
 
 static void cleanup(void)
-- 
2.18.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [LTP] [PATCH v2] syscalls/ioctl_loop05: set loop blksize to bdev blksize
  2020-06-02  8:42 ` [LTP] [PATCH v2] " Jan Stancek
@ 2020-06-02  8:57   ` Yang Xu
  2020-06-05  6:13     ` Jan Stancek
  0 siblings, 1 reply; 6+ messages in thread
From: Yang Xu @ 2020-06-02  8:57 UTC (permalink / raw)
  To: ltp

Hi Jan

Looks good to me, acked.
> Test is failing on s390, where default loop blksize is less than
> backing dev's blksize (4096):
>    tst_test.c:1247: INFO: Timeout per run is 0h 05m 00s
>    tst_device.c:88: INFO: Found free device 0 '/dev/loop0'
>    ioctl_loop05.c:116: INFO: /dev/loop0 default logical_block_size is 512
>    ioctl_loop05.c:62: INFO: Without setting lo_offset or sizelimit
>    ioctl_loop05.c:63: BROK: ioctl(3,LOOP_SET_DIRECT_IO,...) failed: EINVAL (22)
> 
> Per kernel comment at __loop_update_dio(), direct io is supported
> when ".. logical block size of loop is bigger than the backing device's".
> 
> Set loop blksize to one of backing device. Retry is there to avoid
> EAGAIN warning "loop0 (test.img) has still dirty pages".
> 
> Signed-off-by: Jan Stancek <jstancek@redhat.com>
> ---
>   .../kernel/syscalls/ioctl/ioctl_loop05.c      | 26 +++++++++++++++++--
>   1 file changed, 24 insertions(+), 2 deletions(-)
> 
> Changes in v2:
> - check if LOOP_SET_BLOCK_SIZE is supported in setup()
> 
> diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop05.c b/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
> index 6c9ea2802981..2a3c127959aa 100644
> --- a/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
> +++ b/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
> @@ -96,6 +96,9 @@ static void verify_ioctl_loop(void)
>   
>   static void setup(void)
>   {
> +	int fd;
> +	struct stat buf;
> +
>   	if (tst_fs_type(".") == TST_TMPFS_MAGIC)
>   		tst_brk(TCONF, "tmpfd doesn't support O_DIRECT flag");
>   
> @@ -105,6 +108,14 @@ static void setup(void)
>   
>   	sprintf(sys_loop_diopath, "/sys/block/loop%d/loop/dio", dev_num);
>   	tst_fill_file("test.img", 0, 1024, 1024);
> +
> +	fd = SAFE_OPEN("test.img", O_RDONLY);
> +	SAFE_FSTAT(fd, &buf);
> +	SAFE_CLOSE(fd);
> +
> +	logical_block_size = buf.st_blksize;
> +	tst_res(TINFO, "backing dev logical_block_size is %d", logical_block_size);
> +
>   	tst_attach_device(dev_path, "test.img");
>   	attach_flag = 1;
>   	dev_fd = SAFE_OPEN(dev_path, O_RDWR);
> @@ -112,8 +123,19 @@ static void setup(void)
>   	if (ioctl(dev_fd, LOOP_SET_DIRECT_IO, 0) && errno == EINVAL)
>   		tst_brk(TCONF, "LOOP_SET_DIRECT_IO is not supported");
>   
> -	SAFE_IOCTL(dev_fd, BLKSSZGET, &logical_block_size);
> -	tst_res(TINFO, "%s default logical_block_size is %d", dev_path, logical_block_size);
> +	/*
> +	 * from __loop_update_dio():
> +	 *   We support direct I/O only if lo_offset is aligned with the
> +	 *   logical I/O size of backing device, and the logical block
> +	 *   size of loop is bigger than the backing device's and the loop
> +	 *   needn't transform transfer.
> +	 */
> +	if (ioctl(dev_fd, LOOP_SET_BLOCK_SIZE, logical_block_size) != 0) {
> +		if (errno == EINVAL)
> +			tst_brk(TCONF, "LOOP_SET_BLOCK_SIZE is not supported");
> +		TST_RETRY_FUNC(ioctl(dev_fd, LOOP_SET_BLOCK_SIZE,
> +			logical_block_size), TST_RETVAL_EQ0);
> +	}
>   }
>   
>   static void cleanup(void)
> 



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [LTP] [PATCH v2] syscalls/ioctl_loop05: set loop blksize to bdev blksize
  2020-06-02  8:57   ` Yang Xu
@ 2020-06-05  6:13     ` Jan Stancek
  0 siblings, 0 replies; 6+ messages in thread
From: Jan Stancek @ 2020-06-05  6:13 UTC (permalink / raw)
  To: ltp



----- Original Message -----
> Hi Jan
> 
> Looks good to me, acked.

Pushed.


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-06-05  6:13 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-01 13:01 [LTP] [PATCH] syscalls/ioctl_loop05: set loop blksize to bdev blksize Jan Stancek
2020-06-02  4:53 ` Yang Xu
2020-06-02  6:16   ` Jan Stancek
2020-06-02  8:42 ` [LTP] [PATCH v2] " Jan Stancek
2020-06-02  8:57   ` Yang Xu
2020-06-05  6:13     ` Jan Stancek

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.