Hi Hannes, Hannes Reinecke <[email protected]> writes:
> The current LOOP_SET_STATUS64 ioctl has two unused fields > 'init[2]', which can be used in conjunction with the > LO_FLAGS_BLOCKSIZE flag to pass in the new logical blocksize. I don't see a reason to set LO_FLAGS_BLOCKSIZE inside of the loop_device->lo_flags. It's not a flag that gets toggled; rather, it's a flag that indicates that we're attempting to change the block size. The block size is the persistent state. To further clarify, I'm okay with passing it in info->lo_flags, I just don't like that it gets also set in the struct loop_device. I also think that you should spell out logical_block_size, instead of having physical_block_size and block_size. It's just easier to read, in my opinion. If you take my suggestion to set the physical block size automatically, I think that will clean things up some, too. I'm looking forward to v2. And just FYI, nothing looked horrible in patch 3/4, but I'm not going to ack it until I see the v2 posting, since I think it will change at least a little. Cheers, Jeff > Signed-off-by: Hannes Reinecke <[email protected]> > --- > drivers/block/loop.c | 25 ++++++++++++++++++++----- > 1 file changed, 20 insertions(+), 5 deletions(-) > > diff --git a/drivers/block/loop.c b/drivers/block/loop.c > index d0b8754..6723e5e 100644 > --- a/drivers/block/loop.c > +++ b/drivers/block/loop.c > @@ -221,7 +221,8 @@ static void __loop_update_dio(struct loop_device *lo, > bool dio) > } > > static int > -figure_loop_size(struct loop_device *lo, loff_t offset, loff_t sizelimit) > +figure_loop_size(struct loop_device *lo, loff_t offset, loff_t sizelimit, > + loff_t logical_blocksize) > { > loff_t size = get_size(offset, sizelimit, lo->lo_backing_file); > sector_t x = (sector_t)size; > @@ -234,6 +235,8 @@ figure_loop_size(struct loop_device *lo, loff_t offset, > loff_t sizelimit) > if (lo->lo_sizelimit != sizelimit) > lo->lo_sizelimit = sizelimit; > if (lo->lo_flags & LO_FLAGS_BLOCKSIZE) { > + if (lo->lo_logical_blocksize != logical_blocksize) > + lo->lo_logical_blocksize = logical_blocksize; > blk_queue_physical_block_size(lo->lo_queue, lo->lo_blocksize); > blk_queue_logical_block_size(lo->lo_queue, > lo->lo_logical_blocksize); > @@ -1129,13 +1132,24 @@ loop_set_status(struct loop_device *lo, const struct > loop_info64 *info) > if (err) > return err; > > - if (info->lo_flags & LO_FLAGS_BLOCKSIZE) > + if (info->lo_flags & LO_FLAGS_BLOCKSIZE) { > lo->lo_flags |= LO_FLAGS_BLOCKSIZE; > + if ((info->lo_init[0] != 512) && > + (info->lo_init[0] != 1024) && > + (info->lo_init[0] != 2048) && > + (info->lo_init[0] != 4096)) > + return -EINVAL; > + if (info->lo_init[0] > lo->lo_blocksize) > + return -EINVAL; > + } > > if (lo->lo_offset != info->lo_offset || > lo->lo_sizelimit != info->lo_sizelimit || > - lo->lo_flags != lo_flags) > - if (figure_loop_size(lo, info->lo_offset, info->lo_sizelimit)) > + lo->lo_flags != lo_flags || > + ((lo->lo_flags & LO_FLAGS_BLOCKSIZE) && > + (lo->lo_logical_blocksize != info->lo_init[0]))) > + if (figure_loop_size(lo, info->lo_offset, info->lo_sizelimit, > + info->lo_init[0])) > return -EFBIG; > > loop_config_discard(lo); > @@ -1320,7 +1334,8 @@ static int loop_set_capacity(struct loop_device *lo) > if (unlikely(lo->lo_state != Lo_bound)) > return -ENXIO; > > - return figure_loop_size(lo, lo->lo_offset, lo->lo_sizelimit); > + return figure_loop_size(lo, lo->lo_offset, lo->lo_sizelimit, > + lo->lo_logical_blocksize); > } > > static int loop_set_dio(struct loop_device *lo, unsigned long arg) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
