[LTP] [PATCH] syscalls/ioctl_loop05: Get the logic_block_size dynamically

[LTP] [PATCH] syscalls/ioctl_loop05: Get the logic_block_size dynamically

[Yang Xu]

In loop driver code, the sb_bsize was calculated as below
sb_bsize = bdev_logical_block_size(inode->i_sb->s_bdev),

it is the super block's block size that the backing file's inode belongs to,
not by using the st_blksize member of stat struct(it uses inode->i_blkbits).

IMO, we don't have the direct ioctl to get this size, just try it from 512 to page_size.

Also, "offset is ignored" belongs to the last test(less than logical_block_size) but not
the second test(equal to logical_block_size).

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

diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop05.c b/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
index a96997823..09326042f 100644
--- a/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
+++ b/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
@@ -71,7 +71,7 @@ static void verify_ioctl_loop(void)
 	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");
+		tst_res(TPASS, "LOOP_SET_DIRECT_IO succeeded");
 		check_dio_value(1);
 		SAFE_IOCTL(dev_fd, LOOP_SET_DIRECT_IO, 0);
 	} else {
@@ -84,7 +84,7 @@ static void verify_ioctl_loop(void)
 
 	TEST(ioctl(dev_fd, LOOP_SET_DIRECT_IO, 1));
 	if (TST_RET == 0) {
-		tst_res(TPASS, "LOOP_SET_DIRECT_IO succeeded");
+		tst_res(TPASS, "LOOP_SET_DIRECT_IO succeeded, offset is ignored");
 		SAFE_IOCTL(dev_fd, LOOP_SET_DIRECT_IO, 0);
 		return;
 	}
@@ -96,8 +96,7 @@ static void verify_ioctl_loop(void)
 
 static void setup(void)
 {
-	int fd;
-	struct stat buf;
+	int pg_size = getpagesize();
 
 	if (tst_fs_type(".") == TST_TMPFS_MAGIC)
 		tst_brk(TCONF, "tmpfd doesn't support O_DIRECT flag");
@@ -109,13 +108,6 @@ 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);
@@ -130,7 +122,12 @@ static void setup(void)
 	 *   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);
+	for (logical_block_size = 512; logical_block_size < pg_size; logical_block_size += 512) {
+		TST_RETRY_FUNC(ioctl(dev_fd, LOOP_SET_BLOCK_SIZE, logical_block_size), TST_RETVAL_EQ0);
+		if (ioctl(dev_fd, LOOP_SET_DIRECT_IO, 1) == 0)
+			break;
+	}
+	tst_res(TINFO, "backing dev logical_block_size is %d", logical_block_size);
 }
 
 static void cleanup(void)
-- 
2.23.0

[LTP] [PATCH] syscalls/ioctl_loop05: Get the logic_block_size dynamically

[Jan Stancek]

----- Original Message -----
> In loop driver code, the sb_bsize was calculated as below
> sb_bsize = bdev_logical_block_size(inode->i_sb->s_bdev),
> 
> it is the super block's block size that the backing file's inode belongs to,
> not by using the st_blksize member of stat struct(it uses inode->i_blkbits).

I'm not sure I follow the above, are you saying the difference is bdev blksize
vs. filesystem blksize? Is the test failing in some scenarios or is this
fix based on code inspection?

> 
> IMO, we don't have the direct ioctl to get this size, just try it from 512 to
> page_size.

Would BLKSSZGET work? It returns bdev_logical_block_size().

[LTP] [PATCH] syscalls/ioctl_loop05: Get the logic_block_size dynamically

[Yang Xu]

Hi Jan

>
> ----- Original Message -----
>> In loop driver code, the sb_bsize was calculated as below
>> sb_bsize = bdev_logical_block_size(inode->i_sb->s_bdev),
>>
>> it is the super block's block size that the backing file's inode belongs to,
>> not by using the st_blksize member of stat struct(it uses inode->i_blkbits).
> I'm not sure I follow the above, are you saying the difference is bdev blksize
> vs. filesystem blksize?

I said the loop driver used  dev_logical_block_size(inode->i_sb->s_bdev) but not using
st_blksize. I don't see they have conversion formula so using st_blksize maybe wrong.

>   Is the test failing in some scenarios or is this
> fix based on code inspection?

It affects the result of? ("With nonzero offset less than logical_block_size").
When we can get sb_bdev on other machine(not s390), it should report EINVAL error.

But if we use stat.st_blksize, it passed.

not using st_blksize? result as below:
ioctl_loop05.c:81: INFO: With nonzero offset less than logical_block_size
ioctl_loop05.c:92: PASS: LOOP_SET_DIRECT_IO failed as expected: EINVAL (22)

using st_blksize? result as below:
ioctl_loop05.c:81: INFO: With nonzero offset less than logical_block_size
ioctl_loop05.c:87: PASS: LOOP_SET_DIRECT_IO succeeded


>
>> IMO, we don't have the direct ioctl to get this size, just try it from 512 to
>> page_size.
> Would BLKSSZGET work? It returns bdev_logical_block_size().

But it needs a blockdev, in user space, we can specify bdev, but how can we figure out this inode->i_sb->s_bdev block dev.

>
>
>


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20200609/970d8e37/attachment.htm>

[LTP] [PATCH] syscalls/ioctl_loop05: Get the logic_block_size dynamically

[Jan Stancek]

----- Original Message -----
> Hi Jan
> 
> >
> > ----- Original Message -----
> >> In loop driver code, the sb_bsize was calculated as below
> >> sb_bsize = bdev_logical_block_size(inode->i_sb->s_bdev),
> >>
> >> it is the super block's block size that the backing file's inode belongs
> >> to,
> >> not by using the st_blksize member of stat struct(it uses
> >> inode->i_blkbits).
> > I'm not sure I follow the above, are you saying the difference is bdev
> > blksize
> > vs. filesystem blksize?
> 
> I said the loop driver used  dev_logical_block_size(inode->i_sb->s_bdev) but
> not using
> st_blksize.

I know, but I'm trying to understand what the difference is between those two.

> > Would BLKSSZGET work? It returns bdev_logical_block_size().
> 
> But it needs a blockdev, in user space, we can specify bdev, but how can we
> figure out this inode->i_sb->s_bdev block dev.

Isn't that the block device "test.img" is on?

[LTP] [PATCH] syscalls/ioctl_loop05: Get the logic_block_size dynamically

[Yang Xu]

Hi Jan

> 
> 
> ----- Original Message -----
>> Hi Jan
>>
>>>
>>> ----- Original Message -----
>>>> In loop driver code, the sb_bsize was calculated as below
>>>> sb_bsize = bdev_logical_block_size(inode->i_sb->s_bdev),
>>>>
>>>> it is the super block's block size that the backing file's inode belongs
>>>> to,
>>>> not by using the st_blksize member of stat struct(it uses
>>>> inode->i_blkbits).
>>> I'm not sure I follow the above, are you saying the difference is bdev
>>> blksize
>>> vs. filesystem blksize?
>>
>> I said the loop driver used  dev_logical_block_size(inode->i_sb->s_bdev) but
>> not using
>> st_blksize.
> 
> I know, but I'm trying to understand what the difference is between those two.
AFAIK, st_blksize is used to standard io in user space as man-page said 
"/* Block size for filesystem I/O */", it maybe a buffer write used. It 
is a  block size that block in inode used.
But dev_logical_block_size is a min unit thatrequest queue used on block 
layer.

ps? this is my understanding. If it's wrong, please correct me.
> 
>>> Would BLKSSZGET work? It returns bdev_logical_block_size().
>>
>> But it needs a blockdev, in user space, we can specify bdev, but how can we
>> figure out this inode->i_sb->s_bdev block dev.
> 
> Isn't that the block device "test.img" is on?
Do you mean the test.img belong to some block dev, such as /dev/sda1 or 
our mounted block_dev? If so, I think it is.
> 
> 
> 

[LTP] [PATCH] syscalls/ioctl_loop05: Get the logic_block_size dynamically

[Jan Stancek]

----- Original Message -----
> >>> Would BLKSSZGET work? It returns bdev_logical_block_size().
> >>
> >> But it needs a blockdev, in user space, we can specify bdev, but how can
> >> we
> >> figure out this inode->i_sb->s_bdev block dev.
> > 
> > Isn't that the block device "test.img" is on?
> Do you mean the test.img belong to some block dev, such as /dev/sda1 or
> our mounted block_dev? If so, I think it is.

The former. Say if test.img is in /tmp, then I'd assume "s_bdev" is
/dev/mapper/rhel-root (/dev/dm-0)

$ df -T /tmp/test.img
Filesystem            Type 1K-blocks     Used Available Use% Mounted on
/dev/mapper/rhel-root xfs   66789516 33211340  33578176  50% /

[LTP] [PATCH] syscalls/ioctl_loop05: Get the logic_block_size dynamically

[Yang Xu]

Hi Jan


> 
> 
> ----- Original Message -----
>>>>> Would BLKSSZGET work? It returns bdev_logical_block_size().
>>>>
>>>> But it needs a blockdev, in user space, we can specify bdev, but how can
>>>> we
>>>> figure out this inode->i_sb->s_bdev block dev.
>>>
>>> Isn't that the block device "test.img" is on?
>> Do you mean the test.img belong to some block dev, such as /dev/sda1 or
>> our mounted block_dev? If so, I think it is.
> 
> The former. Say if test.img is in /tmp, then I'd assume "s_bdev" is
> /dev/mapper/rhel-root (/dev/dm-0)
> 
> $ df -T /tmp/test.img
> Filesystem            Type 1K-blocks     Used Available Use% Mounted on
> /dev/mapper/rhel-root xfs   66789516 33211340  33578176  50% /
I try this, it is ok on my machine. I will send a v2 patch by using 
BLKSSZGET.
> 
> 
> 

[LTP] [PATCH v2] syscalls/ioctl_loop05: Use correct blockdev to get logical_block_size

[Yang Xu]

At the first, we use BLKSSZGET ioctl to get this size, but using wrong
block dev(/dev/loopN) intead of correct backing file block dev(such as /dev/sdaN).

kernel code(driver/block/loop.c  __loop_update_dio function) as below:
---------------------------------------
if (inode->i_sb->s_bdev) {
	sb_bsize = bdev_logical_block_size(inode->i_sb->s_bdev);
	dio_align = sb_bsize - 1;
}
if (dio) {
	if (queue_logical_block_size(lo->lo_queue) >= sb_bsize &&
		!(lo->lo_offset & dio_align) &&
		mapping->a_ops->direct_IO &&!lo->transfer)
		use_dio = true;
	else
		use_dio = false;
} else {
        use_dio = false;
}
---------------------------------------

Using inode block is wrong because it is for filesystem io(such as we formart
filesystem can specify block size for data or log or metadata), it is not suitable
for logical block size.

Using df cmd (df -T /tmp/xxxxx/test.img)to get the correct block dev.

Also, "offset is ignored" belongs to the last test(less than logical_block_size) but not
the second test(equal to logical_block_size).

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

diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop05.c b/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
index a96997823..643892fff 100644
--- a/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
+++ b/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
@@ -28,12 +28,13 @@
 #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, attach_flag, logical_block_size;
+static int dev_num, dev_fd, block_devfd, attach_flag, logical_block_size;
 
 static void check_dio_value(int flag)
 {
@@ -71,7 +72,7 @@ static void verify_ioctl_loop(void)
 	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");
+		tst_res(TPASS, "LOOP_SET_DIRECT_IO succeeded");
 		check_dio_value(1);
 		SAFE_IOCTL(dev_fd, LOOP_SET_DIRECT_IO, 0);
 	} else {
@@ -84,7 +85,7 @@ static void verify_ioctl_loop(void)
 
 	TEST(ioctl(dev_fd, LOOP_SET_DIRECT_IO, 1));
 	if (TST_RET == 0) {
-		tst_res(TPASS, "LOOP_SET_DIRECT_IO succeeded");
+		tst_res(TPASS, "LOOP_SET_DIRECT_IO succeeded, offset is ignored");
 		SAFE_IOCTL(dev_fd, LOOP_SET_DIRECT_IO, 0);
 		return;
 	}
@@ -94,10 +95,22 @@ static void verify_ioctl_loop(void)
 		tst_res(TFAIL | TTERRNO, "LOOP_SET_DIRECT_IO failed expected EINVAL got");
 }
 
+static void find_backing_bdpath(char *buf)
+{
+	char line[PATH_MAX];
+	FILE *file;
+
+	file = SAFE_FOPEN("1.txt", "r");
+
+	while (fgets(line, sizeof(line), file) != NULL)
+		sscanf(line, "%s", buf);
+	SAFE_FCLOSE(file);
+}
+
 static void setup(void)
 {
-	int fd;
-	struct stat buf;
+	char buf[100];
+	const char *const df_cmd[] = {"df", "-T", ".", NULL};
 
 	if (tst_fs_type(".") == TST_TMPFS_MAGIC)
 		tst_brk(TCONF, "tmpfd doesn't support O_DIRECT flag");
@@ -109,13 +122,6 @@ 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);
@@ -130,13 +136,24 @@ static void setup(void)
 	 *   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);
+	SAFE_CMD(df_cmd, "1.txt", NULL);
+	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);
+
+	if (logical_block_size > 512)
+		TST_RETRY_FUNC(ioctl(dev_fd, LOOP_SET_BLOCK_SIZE, logical_block_size), TST_RETVAL_EQ0);
 }
 
 static void cleanup(void)
 {
 	if (dev_fd > 0)
 		SAFE_CLOSE(dev_fd);
+	if (block_devfd > 0)
+		SAFE_CLOSE(block_devfd);
 	if (attach_flag)
 		tst_detach_device(dev_path);
 }
@@ -150,5 +167,9 @@ static struct tst_test test = {
 	.needs_drivers = (const char *const []) {
 		"loop",
 		NULL
+	},
+	.needs_cmds = (const char *const []) {
+		"df",
+		NULL
 	}
 };
-- 
2.23.0

[LTP] [PATCH v2] syscalls/ioctl_loop05: Use correct blockdev to get logical_block_size

[Jan Stancek]

----- Original Message -----
> 
> Using df cmd (df -T /tmp/xxxxx/test.img)to get the correct block dev.

What I had in mind was to take "df -T" as inspiration, not call it directly,
but that could work too. See notes below.

> +static void find_backing_bdpath(char *buf)
> +{
> +	char line[PATH_MAX];
> +	FILE *file;
> +
> +	file = SAFE_FOPEN("1.txt", "r");
> +
> +	while (fgets(line, sizeof(line), file) != NULL)
> +		sscanf(line, "%s", buf);

This will take the last line of output, which can be a problem as some
version align output differently. For example:

# df -T .
Filesystem           Type 1K-blocks    Used Available Use% Mounted on
/dev/mapper/vg_dhcp13579-lv_root
                     ext4  46967160 3102232  41472456   7% /

can break output into two lines.

> +	SAFE_FCLOSE(file);
> +}
> +
>  static void setup(void)
>  {
> -	int fd;
> -	struct stat buf;
> +	char buf[100];
> +	const char *const df_cmd[] = {"df", "-T", ".", NULL};
>  
>  	if (tst_fs_type(".") == TST_TMPFS_MAGIC)
>  		tst_brk(TCONF, "tmpfd doesn't support O_DIRECT flag");
> @@ -109,13 +122,6 @@ 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);
> @@ -130,13 +136,24 @@ static void setup(void)
>  	 *   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);
> +	SAFE_CMD(df_cmd, "1.txt", NULL);

This could be part of find_backing_bdpath() function.

What I had in mind when I referred to df was something like:
  stat("test.img", &statbuf);
  SAFE_MKNOD("blkdev", S_IFBLK | S_IRWXU, statbuf.st_dev);
  block_devfd = SAFE_OPEN("blkdev", O_RDWR);
What do you think?

[LTP] [PATCH v2] syscalls/ioctl_loop05: Use correct blockdev to get logical_block_size

[Yang Xu]

Hi Jan


> 
> 
> ----- Original Message -----
>>
>> Using df cmd (df -T /tmp/xxxxx/test.img)to get the correct block dev.
> 
> What I had in mind was to take "df -T" as inspiration, not call it directly,
> but that could work too. See notes below.
> 
>> +static void find_backing_bdpath(char *buf)
>> +{
>> +	char line[PATH_MAX];
>> +	FILE *file;
>> +
>> +	file = SAFE_FOPEN("1.txt", "r");
>> +
>> +	while (fgets(line, sizeof(line), file) != NULL)
>> +		sscanf(line, "%s", buf);
> 
> This will take the last line of output, which can be a problem as some
> version align output differently. For example:
> 
> # df -T .
> Filesystem           Type 1K-blocks    Used Available Use% Mounted on
> /dev/mapper/vg_dhcp13579-lv_root
>                       ext4  46967160 3102232  41472456   7% /
> 
> can break output into two lines.
Yes.
> 
>> +	SAFE_FCLOSE(file);
>> +}
>> +
>>   static void setup(void)
>>   {
>> -	int fd;
>> -	struct stat buf;
>> +	char buf[100];
>> +	const char *const df_cmd[] = {"df", "-T", ".", NULL};
>>   
>>   	if (tst_fs_type(".") == TST_TMPFS_MAGIC)
>>   		tst_brk(TCONF, "tmpfd doesn't support O_DIRECT flag");
>> @@ -109,13 +122,6 @@ 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);
>> @@ -130,13 +136,24 @@ static void setup(void)
>>   	 *   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);
>> +	SAFE_CMD(df_cmd, "1.txt", NULL);
> 
> This could be part of find_backing_bdpath() function.
Yes.
> 
> What I had in mind when I referred to df was something like:
>    stat("test.img", &statbuf);
>    SAFE_MKNOD("blkdev", S_IFBLK | S_IRWXU, statbuf.st_dev);
>    block_devfd = SAFE_OPEN("blkdev", O_RDWR);
> What do you think?
> 
Oh, yes, it is more easier (I have tried this). I will send a v3 for this.

ps: I think I can use this in my other loop patches for loop_configure 
ioctl.
> 
> 

[LTP] [PATCH v2] syscalls/ioctl_loop05: Use correct blockdev to get logical_block_size

[Yang Xu]

Hi Jan


> Hi Jan
> 
> 
>>
>>
>> ----- Original Message -----
>>>
>>> Using df cmd (df -T /tmp/xxxxx/test.img)to get the correct block dev.
>>
>> What I had in mind was to take "df -T" as inspiration, not call it 
>> directly,
>> but that could work too. See notes below.
>>
>>> +static void find_backing_bdpath(char *buf)
>>> +{
>>> +??? char line[PATH_MAX];
>>> +??? FILE *file;
>>> +
>>> +??? file = SAFE_FOPEN("1.txt", "r");
>>> +
>>> +??? while (fgets(line, sizeof(line), file) != NULL)
>>> +??????? sscanf(line, "%s", buf);
>>
>> This will take the last line of output, which can be a problem as some
>> version align output differently. For example:
>>
>> # df -T .
>> Filesystem?????????? Type 1K-blocks??? Used Available Use% Mounted on
>> /dev/mapper/vg_dhcp13579-lv_root
>> ????????????????????? ext4? 46967160 3102232? 41472456?? 7% /
>>
>> can break output into two lines.
> Yes.
>>
>>> +??? SAFE_FCLOSE(file);
>>> +}
>>> +
>>> ? static void setup(void)
>>> ? {
>>> -??? int fd;
>>> -??? struct stat buf;
>>> +??? char buf[100];
>>> +??? const char *const df_cmd[] = {"df", "-T", ".", NULL};
>>> ????? if (tst_fs_type(".") == TST_TMPFS_MAGIC)
>>> ????????? tst_brk(TCONF, "tmpfd doesn't support O_DIRECT flag");
>>> @@ -109,13 +122,6 @@ 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);
>>> @@ -130,13 +136,24 @@ static void setup(void)
>>> ?????? *?? 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);
>>> +??? SAFE_CMD(df_cmd, "1.txt", NULL);
>>
>> This could be part of find_backing_bdpath() function.
> Yes.
>>
>> What I had in mind when I referred to df was something like:
>> ?? stat("test.img", &statbuf);
>> ?? SAFE_MKNOD("blkdev", S_IFBLK | S_IRWXU, statbuf.st_dev);
>> ?? block_devfd = SAFE_OPEN("blkdev", O_RDWR);
>> What do you think?
>>
It works well on ext4 or xfs filesystem(user may mount wanted filesystem 
on tmpdir). But if we use btrfs, this
BLKSSZGET will fail because major dev numer is 0. When we meet this 
situation, we don't need to call this ioctl and we can directly test 
becuase it doesn' t have backing file block device align limit.
What do you thin about it?



> Oh, yes, it is more easier (I have tried this). I will send a v3 for this.
> 
> ps: I think I can use this in my other loop patches for loop_configure 
> ioctl.
>>
>>
> 
> 
> 

[LTP] [PATCH v2] syscalls/ioctl_loop05: Use correct blockdev to get logical_block_size

[Jan Stancek]

----- Original Message -----
> >>
> >> What I had in mind when I referred to df was something like:
> >> ?? stat("test.img", &statbuf);
> >> ?? SAFE_MKNOD("blkdev", S_IFBLK | S_IRWXU, statbuf.st_dev);
> >> ?? block_devfd = SAFE_OPEN("blkdev", O_RDWR);
> >> What do you think?
> >>
> It works well on ext4 or xfs filesystem(user may mount wanted filesystem
> on tmpdir). But if we use btrfs, this
> BLKSSZGET will fail because major dev numer is 0. When we meet this
> situation, we don't need to call this ioctl and we can directly test
> becuase it doesn' t have backing file block device align limit.
> What do you thin about it?

This I didn't expect. If it's not reliable then perhaps your method
in v1 that incrementally increases it until it works is perhaps most
universal approach. Sorry for the detour to get there.

[LTP] [PATCH v2] syscalls/ioctl_loop05: Use correct blockdev to get logical_block_size

[Yang Xu]

Hi Jan


> 
> 
> ----- Original Message -----
>>>>
>>>> What I had in mind when I referred to df was something like:
>>>>  ?? stat("test.img", &statbuf);
>>>>  ?? SAFE_MKNOD("blkdev", S_IFBLK | S_IRWXU, statbuf.st_dev);
>>>>  ?? block_devfd = SAFE_OPEN("blkdev", O_RDWR);
>>>> What do you think?
>>>>
>> It works well on ext4 or xfs filesystem(user may mount wanted filesystem
>> on tmpdir). But if we use btrfs, this
>> BLKSSZGET will fail because major dev numer is 0. When we meet this
>> situation, we don't need to call this ioctl and we can directly test
>> becuase it doesn' t have backing file block device align limit.
>> What do you thin about it?
> 
> This I didn't expect. If it's not reliable then perhaps your method
> in v1 that incrementally increases it until it works is perhaps most
> universal approach. Sorry for the detour to get there.
> 
After I trace the stat syscall, btrfs uses virtual block dev, so major 
dev number is 0 since kernel commit[1].

Originally, I want to use that major dev number is 0 to judge whether 
loop driver has align limit on some fileystems. But it sees that they 
don't have direct connection between inode->i_sb->s_bdev (loop used)and 
inode->st_dev(stat used). So using the major dev number to judge whether 
loop driver code has align limits sounds unreasonable.

To aovid this, I will use v1 method with some improvement.


[1]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/fs/btrfs/inode.c?id=3394e1607eaf870ebba37d303fbd590a4c569908 

> 
> 

[LTP] [PATCH v3] syscalls/ioctl_loop05: Use correct blockdev to get logical_block_size

[Yang Xu]

At the first, we use BLKSSZGET ioctl to get this size, but using wrong
block dev(/dev/loopN) intead of correct backing file block dev(such as /dev/sdaN).

kernel code(driver/block/loop.c  __loop_update_dio function) as below:
---------------------------------------
if (inode->i_sb->s_bdev) {
	sb_bsize = bdev_logical_block_size(inode->i_sb->s_bdev);
	dio_align = sb_bsize - 1;
}
if (dio) {
	if (queue_logical_block_size(lo->lo_queue) >= sb_bsize &&
		!(lo->lo_offset & dio_align) &&
		mapping->a_ops->direct_IO &&!lo->transfer)
		use_dio = true;
	else
		use_dio = false;
} else {
        use_dio = false;
}
-------------------------------------

Using inode block size is also wrong because it is for filesystem io(such as we format
filesystem can specify block size for data or log or metadata), it is not suitable
for logical block size.

Using df cmd (df -T /tmp/xxxxx/test.img)to get the correct block dev.

Also, "offset is ignored" belongs to the third test(less than logical_block_size) but not
the second test(equal to logical_block_size).

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
v2-v3:
1. move SAFE_CMD into find_backing_bdpath function
2. change parsing strategy, comparing with "/dev/" string
 .../kernel/syscalls/ioctl/ioctl_loop05.c      | 50 ++++++++++++++-----
 1 file changed, 37 insertions(+), 13 deletions(-)

diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop05.c b/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
index a96997823..0e29b484c 100644
--- a/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
+++ b/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
@@ -28,12 +28,13 @@
 #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, attach_flag, logical_block_size;
+static int dev_num, dev_fd, block_devfd, attach_flag, logical_block_size;
 
 static void check_dio_value(int flag)
 {
@@ -71,7 +72,7 @@ static void verify_ioctl_loop(void)
 	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");
+		tst_res(TPASS, "LOOP_SET_DIRECT_IO succeeded");
 		check_dio_value(1);
 		SAFE_IOCTL(dev_fd, LOOP_SET_DIRECT_IO, 0);
 	} else {
@@ -84,7 +85,7 @@ static void verify_ioctl_loop(void)
 
 	TEST(ioctl(dev_fd, LOOP_SET_DIRECT_IO, 1));
 	if (TST_RET == 0) {
-		tst_res(TPASS, "LOOP_SET_DIRECT_IO succeeded");
+		tst_res(TPASS, "LOOP_SET_DIRECT_IO succeeded, offset is ignored");
 		SAFE_IOCTL(dev_fd, LOOP_SET_DIRECT_IO, 0);
 		return;
 	}
@@ -94,10 +95,26 @@ static void verify_ioctl_loop(void)
 		tst_res(TFAIL | TTERRNO, "LOOP_SET_DIRECT_IO failed expected EINVAL got");
 }
 
+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 setup(void)
 {
-	int fd;
-	struct stat buf;
+	char buf[100];
 
 	if (tst_fs_type(".") == TST_TMPFS_MAGIC)
 		tst_brk(TCONF, "tmpfd doesn't support O_DIRECT flag");
@@ -109,13 +126,6 @@ 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);
@@ -130,13 +140,23 @@ static void setup(void)
 	 *   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);
+	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);
+
+	if (logical_block_size > 512)
+		TST_RETRY_FUNC(ioctl(dev_fd, LOOP_SET_BLOCK_SIZE, logical_block_size), TST_RETVAL_EQ0);
 }
 
 static void cleanup(void)
 {
 	if (dev_fd > 0)
 		SAFE_CLOSE(dev_fd);
+	if (block_devfd > 0)
+		SAFE_CLOSE(block_devfd);
 	if (attach_flag)
 		tst_detach_device(dev_path);
 }
@@ -150,5 +170,9 @@ static struct tst_test test = {
 	.needs_drivers = (const char *const []) {
 		"loop",
 		NULL
+	},
+	.needs_cmds = (const char *const []) {
+		"df",
+		NULL
 	}
 };
-- 
2.23.0

[LTP] [PATCH v3] syscalls/ioctl_loop05: Use correct blockdev to get logical_block_size

[Jan Stancek]

----- Original Message -----
> Using inode block size is also wrong because it is for filesystem io(such as
> we format
> filesystem can specify block size for data or log or metadata), it is not
> suitable
> for logical block size.

If this copes correctly with btrfs too, I don't have objections.
I retested on failing s390 setup and v3 works there OK.

[LTP] [PATCH v3] syscalls/ioctl_loop05: Use correct blockdev to get logical_block_size

[Yang Xu]

Hi Jan

> 
> 
> ----- Original Message -----
>> Using inode block size is also wrong because it is for filesystem io(such as
>> we format
>> filesystem can specify block size for data or log or metadata), it is not
>> suitable
>> for logical block size.
> 
> If this copes correctly with btrfs too, I don't have objections.
For btrfs, I think it is also right.
> I retested on failing s390 setup and v3 works there OK.
Thanks for your retest.

Best Regards
Yang Xu
> 
> 
> 

[LTP] [PATCH v3] syscalls/ioctl_loop05: Use correct blockdev to get logical_block_size

[Yang Xu]

Hi
Does anyone have comment on this patch?

Best Regards
Yang Xu

> Hi Jan
> 
>>
>>
>> ----- Original Message -----
>>> Using inode block size is also wrong because it is for filesystem 
>>> io(such as
>>> we format
>>> filesystem can specify block size for data or log or metadata), it is 
>>> not
>>> suitable
>>> for logical block size.
>>
>> If this copes correctly with btrfs too, I don't have objections.
> For btrfs, I think it is also right.
>> I retested on failing s390 setup and v3 works there OK.
> Thanks for your retest.
> 
> Best Regards
> Yang Xu
>>
>>
>>

[LTP] [PATCH v3] syscalls/ioctl_loop05: Use correct blockdev to get logical_block_size

[Cyril Hrubis]

Hi!
> +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);
> +}

I do not like that we are calling df for something like this.

Looking at what that command does it's not that complex. It does
statfs() to get minor and major number, then scans /proc/self/mountinfo
for these, since these are on third column and then just prints whatever
it's in the 10th column. This isn't more complex that what we have here
and avoids needs to execute binaries and parse the output.

Also this function could be in a test library probably in tst_device.h.

>  static void setup(void)
>  {
> -	int fd;
> -	struct stat buf;
> +	char buf[100];
>  
>  	if (tst_fs_type(".") == TST_TMPFS_MAGIC)
>  		tst_brk(TCONF, "tmpfd doesn't support O_DIRECT flag");
> @@ -109,13 +126,6 @@ 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);
> @@ -130,13 +140,23 @@ static void setup(void)
>  	 *   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);
> +	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);
> +
> +	if (logical_block_size > 512)
> +		TST_RETRY_FUNC(ioctl(dev_fd, LOOP_SET_BLOCK_SIZE, logical_block_size), TST_RETVAL_EQ0);
>  }
>  
>  static void cleanup(void)
>  {
>  	if (dev_fd > 0)
>  		SAFE_CLOSE(dev_fd);
> +	if (block_devfd > 0)
> +		SAFE_CLOSE(block_devfd);
>  	if (attach_flag)
>  		tst_detach_device(dev_path);
>  }
> @@ -150,5 +170,9 @@ static struct tst_test test = {
>  	.needs_drivers = (const char *const []) {
>  		"loop",
>  		NULL
> +	},
> +	.needs_cmds = (const char *const []) {
> +		"df",
> +		NULL
>  	}
>  };
> -- 
> 2.23.0
> 
> 
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v3] syscalls/ioctl_loop05: Use correct blockdev to get logical_block_size

[Jan Stancek]

----- Original Message -----
> Looking at what that command does it's not that complex. It does
> statfs() to get minor and major number, then scans /proc/self/mountinfo
> for these, since these are on third column and then just prints whatever
> it's in the 10th column. This isn't more complex that what we have here
> and avoids needs to execute binaries and parse the output.

Thanks for mountinfo pointer. We went down the stat() route already,
but hit issues with some filesystems. I tried it now with mountinfo
approach and that does seem to fix btrfs case too, so I agree that
parsing /proc/self/mountinfo rather than df output is nicer.

[LTP] [PATCH v3] syscalls/ioctl_loop05: Use correct blockdev to get logical_block_size

[Yang Xu]

Hi Cyril

Sorry for late reply.
> Hi!
>> +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);
>> +}
> 
> I do not like that we are calling df for something like this.
> 
> Looking at what that command does it's not that complex. It does
> statfs() to get minor and major number, then scans /proc/self/mountinfo
> for these, since these are on third column and then just prints whatever
> it's in the 10th column. This isn't more complex that what we have here
> and avoids needs to execute binaries and parse the output.
> 
I look statfs manpage, its buf doesn't have  dev_t type member, I think 
it maybe stat function. But stat function has problem when filsystem 
uses virtual block(such as btrfs,fuse, then major numer is 0).
I try to parse /proc/self/mountinfo(this format is not changed over 
10years), as below:

diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop05.c 
b/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
index a96997823..d6db9cc83 100644
--- a/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
+++ b/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
@@ -28,12 +28,13 @@
  #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, attach_flag, logical_block_size;
+static int dev_num, dev_fd, block_devfd, attach_flag, logical_block_size;

  static void check_dio_value(int flag)
  {
@@ -94,11 +95,34 @@ static void verify_ioctl_loop(void)
                 tst_res(TFAIL | TTERRNO, "LOOP_SET_DIRECT_IO failed 
expected EINVAL got");
  }

-static void setup(void)
+static void find_backing_bdpath(char *buf, char *bd_path)
  {
-       int fd;
-       struct stat buf;
+       char fmt1[1024];
+       char fmt2[1024];
+       char mnt_root[100];
+       char line[PATH_MAX];
+       FILE *file;
+
+       sprintf(fmt1, "%%*i %%*i %%*u:%%*u %%*s %s %%*s %%*s %%*s %%*s 
%%s %%*s", buf);
+       sprintf(fmt2, "%%*i %%*i %%*u:%%*u %%*s %%s %%*s %%*s %%*s %%*s 
%%s %%*s");
+       if (!FILE_LINES_SCANF("/proc/self/mountinfo", fmt1, bd_path)) {
+               tst_res(TINFO, "have %s mntpoint", buf);
+               return;
+       }
+       tst_res(TINFO, "Not have %s mntpoint, try /", buf);
+       file = SAFE_FOPEN("/proc/self/mountinfo", "r");
+       while (fgets(line, sizeof(line), file) != NULL) {
+               sscanf(line, fmt2, mnt_root, bd_path);
+               if(strcmp(mnt_root, "/"))
+                       continue;
+               break;
+       }
+       SAFE_FCLOSE(file);
+}

+static void setup(void)
+{
+       char bd_path[100];
         if (tst_fs_type(".") == TST_TMPFS_MAGIC)
                 tst_brk(TCONF, "tmpfd doesn't support O_DIRECT flag");

@@ -109,13 +133,6 @@ 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);
@@ -130,13 +147,21 @@ static void setup(void)
          *   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);
+       find_backing_bdpath("/tmp", bd_path);
+       block_devfd = SAFE_OPEN(bd_path, 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);
+       if (logical_block_size > 512)
+               TST_RETRY_FUNC(ioctl(dev_fd, LOOP_SET_BLOCK_SIZE, 
logical_block_size), TST_RETVAL_EQ0);
  }

  static void cleanup(void)
  {
         if (dev_fd > 0)
                 SAFE_CLOSE(dev_fd);
+       if (block_devfd > 0)
+               SAFE_CLOSE(block_devfd);
         if (attach_flag)
                 tst_detach_device(dev_path);
  }
> Also this function could be in a test library probably in tst_device.h.
> 
>>   static void setup(void)
>>   {
>> -	int fd;
>> -	struct stat buf;
>> +	char buf[100];
>>   
>>   	if (tst_fs_type(".") == TST_TMPFS_MAGIC)
>>   		tst_brk(TCONF, "tmpfd doesn't support O_DIRECT flag");
>> @@ -109,13 +126,6 @@ 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);
>> @@ -130,13 +140,23 @@ static void setup(void)
>>   	 *   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);
>> +	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);
>> +
>> +	if (logical_block_size > 512)
>> +		TST_RETRY_FUNC(ioctl(dev_fd, LOOP_SET_BLOCK_SIZE, logical_block_size), TST_RETVAL_EQ0);
>>   }
>>   
>>   static void cleanup(void)
>>   {
>>   	if (dev_fd > 0)
>>   		SAFE_CLOSE(dev_fd);
>> +	if (block_devfd > 0)
>> +		SAFE_CLOSE(block_devfd);
>>   	if (attach_flag)
>>   		tst_detach_device(dev_path);
>>   }
>> @@ -150,5 +170,9 @@ static struct tst_test test = {
>>   	.needs_drivers = (const char *const []) {
>>   		"loop",
>>   		NULL
>> +	},
>> +	.needs_cmds = (const char *const []) {
>> +		"df",
>> +		NULL
>>   	}
>>   };
>> -- 
>> 2.23.0
>>
>>
>>
>>
>> -- 
>> Mailing list info: https://lists.linux.it/listinfo/ltp
> 

[LTP] [PATCH v4 1/2] tst_device: Add new api tst_find_backing_dev(path, dev)

[Yang Xu]

This api reads the /proc/self/mountinfo and compare path with
the 5th column each row in this file, assign the 10th column
value to dev when match succeed.

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
 doc/test-writing-guidelines.txt | 11 ++++++++
 include/tst_device.h            |  7 +++++
 lib/newlib_tests/tst_device.c   |  8 ++++++
 lib/tst_device.c                | 47 +++++++++++++++++++++++++++++++++
 4 files changed, 73 insertions(+)

diff --git a/doc/test-writing-guidelines.txt b/doc/test-writing-guidelines.txt
index 6e466ed0f..1fe11abca 100644
--- a/doc/test-writing-guidelines.txt
+++ b/doc/test-writing-guidelines.txt
@@ -1089,6 +1089,17 @@ FS deferred IO metadata/cache interference, we suggest doing "syncfs" before the
 tst_dev_bytes_written first invocation. And an inline function named tst_dev_sync
 is created for that intention.
 
+[source,c]
+-------------------------------------------------------------------------------
+#include "tst_test.h"
+
+voud tst_find_backing_dev(const char *path, char *dev);
+-------------------------------------------------------------------------------
+
+This function finds the block dev that this path belongs to, it compares path buf
+with the fifth column of each row in "/proc/self/mountinfo" and list 10th column
+as its block dev.
+
 2.2.16 Formatting a device with a filesystem
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
diff --git a/include/tst_device.h b/include/tst_device.h
index 950cfe1ed..6a1fc5186 100644
--- a/include/tst_device.h
+++ b/include/tst_device.h
@@ -84,4 +84,11 @@ unsigned long tst_dev_bytes_written(const char *dev);
  */
 void tst_purge_dir(const char *path);
 
+/*
+ * Find the file or path belongs to which block dev
+ * @path  Path to find the backing dev
+ * @dev   The block dev
+ */
+void tst_find_backing_dev(const char *path, char *dev);
+
 #endif	/* TST_DEVICE_H__ */
diff --git a/lib/newlib_tests/tst_device.c b/lib/newlib_tests/tst_device.c
index 1344495b3..ad077affd 100644
--- a/lib/newlib_tests/tst_device.c
+++ b/lib/newlib_tests/tst_device.c
@@ -13,6 +13,7 @@ static void do_test(void)
 {
 	int fd;
 	const char *dev;
+	char block_dev[100];
 	uint64_t ltp_dev_size;
 
 	dev = tst_device->dev;
@@ -29,6 +30,13 @@ static void do_test(void)
 		tst_res(TPASS, "Got expected device size");
 	else
 		tst_res(TFAIL, "Got unexpected device size");
+
+	tst_find_backing_dev("/boot", block_dev);
+	tst_res(TPASS, "/boot belongs to %s block dev", block_dev);
+	tst_find_backing_dev("/", block_dev);
+	tst_res(TPASS, "/ belongs to %s block dev", block_dev);
+	tst_find_backing_dev("/tmp", block_dev);
+	tst_find_backing_dev("/boot/xuyang", block_dev);
 }
 
 static struct tst_test test = {
diff --git a/lib/tst_device.c b/lib/tst_device.c
index 67fe90ed6..97b42eb4f 100644
--- a/lib/tst_device.c
+++ b/lib/tst_device.c
@@ -488,3 +488,50 @@ unsigned long tst_dev_bytes_written(const char *dev)
 
 	return dev_bytes_written;
 }
+
+void tst_find_backing_dev(const char *path, char *dev)
+{
+	char fmt[100];
+	char mnt_root[100];
+	char bd_device[100];
+	char line[1024];
+	FILE *file;
+	int flag = 0;
+	int ret;
+	int fd;
+	struct stat st;
+
+	if (access(path, F_OK))
+		tst_brkm(TCONF, NULL, "path(%s) doesn't exist", path);
+
+	sprintf(fmt, "%%*i %%*i %%*u:%%*u %%*s %%s %%*s %%*s %%*s %%*s %%s %%*s");
+	file = SAFE_FOPEN(NULL, "/proc/self/mountinfo", "r");
+	while (fgets(line, sizeof(line), file) != NULL) {
+		ret = sscanf(line, fmt, mnt_root, bd_device);
+		if (ret != 2)
+			 tst_brkm(TCONF, NULL, "paring /proc/self/mountfinfo file failed, expected 2, got %d", ret);
+		if (!strcmp(mnt_root, path)) {
+			strcpy(dev, bd_device);
+			flag = 1;
+			break;
+		}
+		if (!strncmp(mnt_root, path, strlen(mnt_root))) {
+			strcpy(dev, bd_device);
+			flag = 1;
+		}
+	}
+	SAFE_FCLOSE(NULL, file);
+	if (!flag || access(dev, F_OK))
+		tst_brkm(TCONF, NULL, "Can not find backing dev(%s)", path);
+
+	fd = open(dev, O_RDONLY);
+	if (fd < 0)
+		tst_brkm(TWARN | TERRNO, NULL, "open(%s, O_RDONLY) failed", dev);
+	if (stat(dev, &st) < 0) {
+		close(fd);
+		tst_brkm(TWARN | TERRNO, NULL, "stat() failed");
+	}
+	close(fd);
+	if (S_ISBLK(st.st_mode) != 1)
+		tst_brkm(TCONF, NULL, "dev(%s) isn't a block dev", dev);
+}
-- 
2.23.0

[LTP] [PATCH v4 2/2] syscalls/ioctl_loop05: Use correct blockdev to get logical_block_size

[Yang Xu]

At the first, we use BLKSSZGET ioctl to get this size, but using wrong
block dev(/dev/loopN) intead of correct backing file block dev(such as /dev/sdaN).

kernel code(driver/block/loop.c  __loop_update_dio function) as below:
---------------------------------------
if (inode->i_sb->s_bdev) {
	sb_bsize = bdev_logical_block_size(inode->i_sb->s_bdev);
	dio_align = sb_bsize - 1;
}
if (dio) {
	if (queue_logical_block_size(lo->lo_queue) >= sb_bsize &&
		!(lo->lo_offset & dio_align) &&
		mapping->a_ops->direct_IO &&!lo->transfer)
		use_dio = true;
	else
		use_dio = false;
} else {
        use_dio = false;
}
-------------------------------------

Using inode block size is also wrong because it is for filesystem io(such as we format
filesystem can specify block size for data or log or metadata), it is not suitable
for logical block size.

Using tst_find_backing_dev(path, dev)to get the correct block dev.

Also, "offset is ignored" belongs to the third test(less than logical_block_size) but not
the second test(equal to logical_block_size)

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
v3->v4:
1. using tst_find_backing_dev instead of df command
2. also print block dev name
 .../kernel/syscalls/ioctl/ioctl_loop05.c      | 29 ++++++++++---------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop05.c b/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
index a96997823..e3c14faab 100644
--- a/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
+++ b/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
@@ -32,8 +32,8 @@
 #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, attach_flag, logical_block_size;
+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 void check_dio_value(int flag)
 {
@@ -71,7 +71,7 @@ static void verify_ioctl_loop(void)
 	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");
+		tst_res(TPASS, "LOOP_SET_DIRECT_IO succeeded");
 		check_dio_value(1);
 		SAFE_IOCTL(dev_fd, LOOP_SET_DIRECT_IO, 0);
 	} else {
@@ -84,7 +84,7 @@ static void verify_ioctl_loop(void)
 
 	TEST(ioctl(dev_fd, LOOP_SET_DIRECT_IO, 1));
 	if (TST_RET == 0) {
-		tst_res(TPASS, "LOOP_SET_DIRECT_IO succeeded");
+		tst_res(TPASS, "LOOP_SET_DIRECT_IO succeeded, offset is ignored");
 		SAFE_IOCTL(dev_fd, LOOP_SET_DIRECT_IO, 0);
 		return;
 	}
@@ -96,8 +96,7 @@ static void verify_ioctl_loop(void)
 
 static void setup(void)
 {
-	int fd;
-	struct stat buf;
+	char bd_path[100];
 
 	if (tst_fs_type(".") == TST_TMPFS_MAGIC)
 		tst_brk(TCONF, "tmpfd doesn't support O_DIRECT flag");
@@ -109,13 +108,6 @@ 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);
@@ -130,13 +122,22 @@ static void setup(void)
 	 *   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);
+	sprintf(backing_file_path, "%s/test.img", tst_get_tmpdir());
+	tst_find_backing_dev(backing_file_path, bd_path);
+	block_devfd = SAFE_OPEN(bd_path, O_RDWR);
+	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);
 }
 
 static void cleanup(void)
 {
 	if (dev_fd > 0)
 		SAFE_CLOSE(dev_fd);
+	if (block_devfd > 0)
+		SAFE_CLOSE(block_devfd);
 	if (attach_flag)
 		tst_detach_device(dev_path);
 }
-- 
2.23.0

[LTP] [PATCH v4 1/2] tst_device: Add new api tst_find_backing_dev(path, dev)

[Jan Stancek]

----- Original Message -----
> +This function finds the block dev that this path belongs to, it compares
> path buf
> +with the fifth column of each row in "/proc/self/mountinfo" and list 10th
> column
> +as its block dev.

Why not match major/minor numbers?

You said "But stat function has problem when filsystem uses virtual block
(such as btrfs,fuse, then major numer is 0)." - Why is that a problem
for comparison against /proc/self/mountinfo?

[LTP] [PATCH v4 1/2] tst_device: Add new api tst_find_backing_dev(path, dev)

[Yang Xu]

Hi Jan


> 
> 
> ----- Original Message -----
>> +This function finds the block dev that this path belongs to, it compares
>> path buf
>> +with the fifth column of each row in "/proc/self/mountinfo" and list 10th
>> column
>> +as its block dev.
> 
> Why not match major/minor numbers?
> 
> You said "But stat function has problem when filsystem uses virtual block
> (such as btrfs,fuse, then major numer is 0)." - Why is that a problem
> for comparison against /proc/self/mountinfo?
> 
Yes, you are right. I wrongly think btrfs filesystem affects the 10th 
columu value in /proc/self/mountinfo. In actually, it can show the 
correct backing block dev.
so this functon code as below:
void tst_find_backing_dev(const char *path, char *dev)
{
         char fmt[1024];
         struct stat buf;

         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));

         SAFE_FILE_LINES_SCANF(NULL, "/proc/self/mountinfo", fmt, dev);

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

         if (S_ISBLK(buf.st_mode) != 1)
                 tst_brkm(TCONF, NULL, "dev(%s) isn't a block dev", dev);
}


ps: If you think it is ok, I will send a v5 patch about this.
> 
> 

[LTP] [PATCH v4 1/2] tst_device: Add new api tst_find_backing_dev(path, dev)

[Jan Stancek]

> > You said "But stat function has problem when filsystem uses virtual block
> > (such as btrfs,fuse, then major numer is 0)." - Why is that a problem
> > for comparison against /proc/self/mountinfo?
> > 
> Yes, you are right. I wrongly think btrfs filesystem affects the 10th
> columu value in /proc/self/mountinfo. In actually, it can show the
> correct backing block dev.

I haven't tested your pasted version, but that approach seems better,
since that should work for any path, not just the mount point.

[LTP] [PATCH v5 1/2] tst_device: Add new api tst_find_backing_dev(path, dev)

[Yang Xu]

This api uses stat() to get major/minor devnumber of the path, assign the
10th column value to dev when match succeeds in "/proc/self/mountinfo".

Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
---
 doc/test-writing-guidelines.txt | 11 +++++++++++
 include/tst_device.h            |  7 +++++++
 lib/newlib_tests/tst_device.c   |  8 ++++++++
 lib/tst_device.c                | 21 +++++++++++++++++++++
 4 files changed, 47 insertions(+)

diff --git a/doc/test-writing-guidelines.txt b/doc/test-writing-guidelines.txt
index 6e466ed0f..58116a40e 100644
--- a/doc/test-writing-guidelines.txt
+++ b/doc/test-writing-guidelines.txt
@@ -1089,6 +1089,17 @@ FS deferred IO metadata/cache interference, we suggest doing "syncfs" before the
 tst_dev_bytes_written first invocation. And an inline function named tst_dev_sync
 is created for that intention.
 
+[source,c]
+-------------------------------------------------------------------------------
+#include "tst_test.h"
+
+voud tst_find_backing_dev(const char *path, char *dev);
+-------------------------------------------------------------------------------
+
+This function finds the block dev that this path belongs to, it uses stat function
+to get the major/minor number of the path. Then scan them in "/proc/self/mountinfo"
+and list 10th column value as its block dev if match succeeds.
+
 2.2.16 Formatting a device with a filesystem
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
diff --git a/include/tst_device.h b/include/tst_device.h
index 950cfe1ed..6a1fc5186 100644
--- a/include/tst_device.h
+++ b/include/tst_device.h
@@ -84,4 +84,11 @@ unsigned long tst_dev_bytes_written(const char *dev);
  */
 void tst_purge_dir(const char *path);
 
+/*
+ * Find the file or path belongs to which block dev
+ * @path  Path to find the backing dev
+ * @dev   The block dev
+ */
+void tst_find_backing_dev(const char *path, char *dev);
+
 #endif	/* TST_DEVICE_H__ */
diff --git a/lib/newlib_tests/tst_device.c b/lib/newlib_tests/tst_device.c
index 1344495b3..ad077affd 100644
--- a/lib/newlib_tests/tst_device.c
+++ b/lib/newlib_tests/tst_device.c
@@ -13,6 +13,7 @@ static void do_test(void)
 {
 	int fd;
 	const char *dev;
+	char block_dev[100];
 	uint64_t ltp_dev_size;
 
 	dev = tst_device->dev;
@@ -29,6 +30,13 @@ static void do_test(void)
 		tst_res(TPASS, "Got expected device size");
 	else
 		tst_res(TFAIL, "Got unexpected device size");
+
+	tst_find_backing_dev("/boot", block_dev);
+	tst_res(TPASS, "/boot belongs to %s block dev", block_dev);
+	tst_find_backing_dev("/", block_dev);
+	tst_res(TPASS, "/ belongs to %s block dev", block_dev);
+	tst_find_backing_dev("/tmp", block_dev);
+	tst_find_backing_dev("/boot/xuyang", block_dev);
 }
 
 static struct tst_test test = {
diff --git a/lib/tst_device.c b/lib/tst_device.c
index 67fe90ed6..842bb7ca7 100644
--- a/lib/tst_device.c
+++ b/lib/tst_device.c
@@ -31,6 +31,7 @@
 #include <linux/loop.h>
 #include <stdint.h>
 #include <inttypes.h>
+#include <sys/sysmacros.h>
 #include "lapi/syscalls.h"
 #include "test.h"
 #include "safe_macros.h"
@@ -488,3 +489,23 @@ unsigned long tst_dev_bytes_written(const char *dev)
 
 	return dev_bytes_written;
 }
+
+void tst_find_backing_dev(const char *path, char *dev)
+{
+	char fmt[1024];
+	struct stat buf;
+
+	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));
+
+	SAFE_FILE_LINES_SCANF(NULL, "/proc/self/mountinfo", fmt, dev);
+
+	if (stat(dev, &buf) < 0)
+		 tst_brkm(TWARN | TERRNO, NULL, "stat(%s) failed", dev);
+
+	if (S_ISBLK(buf.st_mode) != 1)
+		tst_brkm(TCONF, NULL, "dev(%s) isn't a block dev", dev);
+}
-- 
2.23.0

[LTP] [PATCH v5 2/2] syscalls/ioctl_loop05: Use correct blockdev to get logical_block_size

[Yang Xu]

At the first, we use BLKSSZGET ioctl to get this size, but using wrong
block dev(/dev/loopN) intead of correct backing file block dev(such as /dev/sdaN).

kernel code(driver/block/loop.c  __loop_update_dio function) as below:
---------------------------------------
if (inode->i_sb->s_bdev) {
	sb_bsize = bdev_logical_block_size(inode->i_sb->s_bdev);
	dio_align = sb_bsize - 1;
}
if (dio) {
	if (queue_logical_block_size(lo->lo_queue) >= sb_bsize &&
		!(lo->lo_offset & dio_align) &&
		mapping->a_ops->direct_IO &&!lo->transfer)
		use_dio = true;
	else
		use_dio = false;
} else {
        use_dio = false;
}
-------------------------------------

Using inode block size is also wrong because it is for filesystem io(such as we format
filesystem can specify block size for data or log or metadata), it is not suitable
for logical block size.

Using tst_find_backing_dev(path, dev)to get the correct block dev.

Also, "offset is ignored" belongs to the third test(less than logical_block_size) but not
the second test(equal to logical_block_size)

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

diff --git a/testcases/kernel/syscalls/ioctl/ioctl_loop05.c b/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
index a96997823..e3c14faab 100644
--- a/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
+++ b/testcases/kernel/syscalls/ioctl/ioctl_loop05.c
@@ -32,8 +32,8 @@
 #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, attach_flag, logical_block_size;
+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 void check_dio_value(int flag)
 {
@@ -71,7 +71,7 @@ static void verify_ioctl_loop(void)
 	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");
+		tst_res(TPASS, "LOOP_SET_DIRECT_IO succeeded");
 		check_dio_value(1);
 		SAFE_IOCTL(dev_fd, LOOP_SET_DIRECT_IO, 0);
 	} else {
@@ -84,7 +84,7 @@ static void verify_ioctl_loop(void)
 
 	TEST(ioctl(dev_fd, LOOP_SET_DIRECT_IO, 1));
 	if (TST_RET == 0) {
-		tst_res(TPASS, "LOOP_SET_DIRECT_IO succeeded");
+		tst_res(TPASS, "LOOP_SET_DIRECT_IO succeeded, offset is ignored");
 		SAFE_IOCTL(dev_fd, LOOP_SET_DIRECT_IO, 0);
 		return;
 	}
@@ -96,8 +96,7 @@ static void verify_ioctl_loop(void)
 
 static void setup(void)
 {
-	int fd;
-	struct stat buf;
+	char bd_path[100];
 
 	if (tst_fs_type(".") == TST_TMPFS_MAGIC)
 		tst_brk(TCONF, "tmpfd doesn't support O_DIRECT flag");
@@ -109,13 +108,6 @@ 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);
@@ -130,13 +122,22 @@ static void setup(void)
 	 *   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);
+	sprintf(backing_file_path, "%s/test.img", tst_get_tmpdir());
+	tst_find_backing_dev(backing_file_path, bd_path);
+	block_devfd = SAFE_OPEN(bd_path, O_RDWR);
+	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);
 }
 
 static void cleanup(void)
 {
 	if (dev_fd > 0)
 		SAFE_CLOSE(dev_fd);
+	if (block_devfd > 0)
+		SAFE_CLOSE(block_devfd);
 	if (attach_flag)
 		tst_detach_device(dev_path);
 }
-- 
2.23.0

[LTP] [PATCH v5 1/2] tst_device: Add new api tst_find_backing_dev(path, dev)

[Jan Stancek]

----- Original Message -----
> This api uses stat() to get major/minor devnumber of the path, assign the
> 10th column value to dev when match succeeds in "/proc/self/mountinfo".
> 
> Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>

Cyril, any objections to v5?

[LTP] [PATCH v5 1/2] tst_device: Add new api tst_find_backing_dev(path, dev)

[Cyril Hrubis]

Hi!
> > This api uses stat() to get major/minor devnumber of the path, assign the
> > 10th column value to dev when match succeeds in "/proc/self/mountinfo".
> > 
> > Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
> 
> Cyril, any objections to v5?

Looks good, Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v5 1/2] tst_device: Add new api tst_find_backing_dev(path, dev)

[Jan Stancek]

----- Original Message -----
> Hi!
> > > This api uses stat() to get major/minor devnumber of the path, assign the
> > > 10th column value to dev when match succeeds in "/proc/self/mountinfo".
> > > 
> > > Signed-off-by: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
> > 
> > Cyril, any objections to v5?
> 
> Looks good, Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

Thank you, pushed.