Re: [PATCH v2 4/8] block: introduce bio_required_sector_alignment()

From: Eric Biggers <ebiggers@kernel.org>
To: Christoph Hellwig <hch@infradead.org>
Cc: Satya Tangirala <satyat@google.com>,
	linux-block@vger.kernel.org, linux-kernel@vger.kernel.org,
	Jens Axboe <axboe@kernel.dk>
Subject: Re: [PATCH v2 4/8] block: introduce bio_required_sector_alignment()
Date: Thu, 15 Apr 2021 12:37:44 -0700	[thread overview]
Message-ID: <YHiWCCliqPkA2o6h@gmail.com> (raw)
In-Reply-To: <20210330180653.GB811594@infradead.org>

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