On Tue, Aug 22, 2017 at 10:33:00AM -0700, Omar Sandoval wrote:
> @@ -946,6 +938,9 @@ static int loop_set_fd(struct loop_device *lo, fmode_t mode,
>
> if (!(lo_flags & LO_FLAGS_READ_ONLY) && file->f_op->fsync)
> blk_queue_write_cache(lo->lo_queue, true, false);
> + blk_queue_logical_block_size(lo->lo_queue, 512);
> + blk_queue_physical_block_size(lo->lo_queue, 512);
> + blk_queue_io_min(lo->lo_queue, 512);
...
> @@ -1133,14 +1128,7 @@ loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
...
> + if (info->lo_flags & LO_FLAGS_BLOCKSIZE) {
> + blk_queue_logical_block_size(lo->lo_queue, LO_INFO_BLOCKSIZE(info));
> + blk_queue_physical_block_size(lo->lo_queue, LO_INFO_BLOCKSIZE(info));
> + blk_queue_io_min(lo->lo_queue, LO_INFO_BLOCKSIZE(info));
> + }
> +
I don't understand this.
* it seems the default is 512, but what about if the backing file
is not a regular file? For example "losetup -f /dev/sda" where
sda sector size is > 512.
* why the attributes in the /sys are affected by LO_FLAGS_BLOCKSIZE?
Would be better to have a real sizes there (independently on the flag)?
* I think kernel lies in /sys about I/O size now. The phy sector size
has never been 512, but PAGE_SIZE or real phy sector if backing file
is a block device.
Your patch improves it for LO_FLAGS_BLOCKSIZE, but it's still lies
about internally used I/O sizes.
Why we cannot use
blk_queue_physical_block_size(lo->lo_queue, lo->lo_blocksize);
blk_queue_logical_block_size(lo->lo_queue, lo->lo_logical_blocksize);
(as suggested by Hannes' original patch), but without
if (info->lo_flags & LO_FLAGS_BLOCKSIZE)
condition?
Yes, result will be backwardly incompatible, but the current
situation (all is 512) does not describe reality. It's legacy from
time where all in kernel was 512...
Karel
--
Karel Zak <kzak@xxxxxxxxxx>
http://karelzak.blogspot.com
