Messages in this thread | | | Date | Thu, 3 Nov 2016 15:26:44 +0100 | From | Christoph Hellwig <> | Subject | Re: [PATCH 2/2] loop: support 4k physical blocksize |
| |
> -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; > @@ -233,6 +234,12 @@ figure_loop_size(struct loop_device *lo, loff_t offset, loff_t sizelimit) > lo->lo_offset = offset; > if (lo->lo_sizelimit != sizelimit) > lo->lo_sizelimit = sizelimit; > + if (lo->lo_flags & LO_FLAGS_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); > + }
I've looked how the existing code uses lo_blocksize and this whole thing confuses me to no end. Can we have all the blocksize assignments (i.e. including the existing lo_blocksize assignments) in one single place and documented? Especialy as it seems so far lo_blocksize seems to be the "fs" blocksize as set by set_blocksize, which seems totally wrong to be set by loop at all.
> + 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;
No need for the inner braces. Also please find a way to have a descriptive name for the field, be that an anonymous union, or a #define.
> + (lo->lo_logical_blocksize != info->lo_init[0])))
Same comment about the inner braces here.
> + if (figure_loop_size(lo, info->lo_offset, info->lo_sizelimit, > + info->lo_init[0])) > return -EFBIG; > > loop_config_discard(lo); > @@ -1303,7 +1328,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);
I'd say drop all the arguments but lo here (maybe in a prep patch) as passing them all seems pointless and confusing.
| |