| Message ID | 20210325212609.492188-5-satyat@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ensure bios aren't split in middle of crypto data unit | expand |
On Thu, Mar 25, 2021 at 09:26:05PM +0000, Satya Tangirala wrote: > +/* > + * The required sector alignment for a bio. The number of sectors in any bio > + * that's constructed/split must be aligned to this value. > + */ > +static inline unsigned int bio_required_sector_alignment(struct bio *bio) > +{ > + struct request_queue *q = bio->bi_bdev->bd_disk->queue; > + > + return max(queue_logical_block_size(q) >> SECTOR_SHIFT, > + blk_crypto_bio_sectors_alignment(bio)); > +} It might make more sense to just have a field in the request queue for the max alignment so that the fast path just looks at one field. Then the various setup time functions would update it to the maximum required.
On Tue, Mar 30, 2021 at 07:06:53PM +0100, Christoph Hellwig wrote: > On Thu, Mar 25, 2021 at 09:26:05PM +0000, Satya Tangirala wrote: > > +/* > > + * The required sector alignment for a bio. The number of sectors in any bio > > + * that's constructed/split must be aligned to this value. > > + */ > > +static inline unsigned int bio_required_sector_alignment(struct bio *bio) > > +{ > > + struct request_queue *q = bio->bi_bdev->bd_disk->queue; > > + > > + return max(queue_logical_block_size(q) >> SECTOR_SHIFT, > > + blk_crypto_bio_sectors_alignment(bio)); > > +} > > It might make more sense to just have a field in the request queue > for the max alignment so that the fast path just looks at one field. > Then the various setup time functions would update it to the maximum > required. I don't see how that would work here, as the required alignment is a per-bio thing which depends on whether the bio has an encryption context or not, and (if it does) also the data_unit_size the submitter of the bio selected. We could just always assume the worst-case scenario, but that seems unnecessarily pessimistic? - Eric
On Thu, Mar 25, 2021 at 09:26:05PM +0000, Satya Tangirala wrote: > This function returns the required alignment for the number of sectors in > a bio. In particular, the number of sectors passed to bio_split() must be > aligned to this value. > > Signed-off-by: Satya Tangirala <satyat@google.com> > --- > block/blk.h | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/block/blk.h b/block/blk.h > index 3b53e44b967e..5ef207a6d34c 100644 > --- a/block/blk.h > +++ b/block/blk.h > @@ -261,6 +261,18 @@ static inline unsigned int bio_allowed_max_sectors(struct request_queue *q) > return round_down(UINT_MAX, queue_logical_block_size(q)) >> 9; > } > > +/* > + * The required sector alignment for a bio. The number of sectors in any bio > + * that's constructed/split must be aligned to this value. What does "constructed" mean in this context? > + */ > +static inline unsigned int bio_required_sector_alignment(struct bio *bio) > +{ > + struct request_queue *q = bio->bi_bdev->bd_disk->queue; > + > + return max(queue_logical_block_size(q) >> SECTOR_SHIFT, > + blk_crypto_bio_sectors_alignment(bio)); > +} It would be slightly simpler to use bdev_logical_block_size(bio->bi_bdev) instead of queue_logical_block_size(bio->bi_bdev->bd_disk->queue). Also, if there's interest in avoiding the branch implied by the max() here, it would be possible to take advantage of the values being powers of 2 as follows: static inline unsigned int bio_required_sector_alignment(struct bio *bio) { unsigned int alignmask = (bdev_logical_block_size(q) >> SECTOR_SHIFT) - 1; alignmask |= blk_crypto_bio_sectors_alignment(bio) - 1; return alignmask + 1; }
diff --git a/block/blk.h b/block/blk.h index 3b53e44b967e..5ef207a6d34c 100644 --- a/block/blk.h +++ b/block/blk.h @@ -261,6 +261,18 @@ static inline unsigned int bio_allowed_max_sectors(struct request_queue *q) return round_down(UINT_MAX, queue_logical_block_size(q)) >> 9; } +/* + * The required sector alignment for a bio. The number of sectors in any bio + * that's constructed/split must be aligned to this value. + */ +static inline unsigned int bio_required_sector_alignment(struct bio *bio) +{ + struct request_queue *q = bio->bi_bdev->bd_disk->queue; + + return max(queue_logical_block_size(q) >> SECTOR_SHIFT, + blk_crypto_bio_sectors_alignment(bio)); +} + /* * The max bio size which is aligned to q->limits.discard_granularity. This * is a hint to split large discard bio in generic block layer, then if device
This function returns the required alignment for the number of sectors in a bio. In particular, the number of sectors passed to bio_split() must be aligned to this value. Signed-off-by: Satya Tangirala <satyat@google.com> --- block/blk.h | 12 ++++++++++++ 1 file changed, 12 insertions(+)