| Message ID | 1591691583-12442-1-git-send-email-xuyang2018.jy@cn.fujitsu.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | syscalls/ioctl_loop05: Get the logic_block_size dynamically | expand |
----- 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().
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. > > >
----- 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?
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. > > >
----- 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% /
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. > > >
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)
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(-)