On Tue, May 24, 2022 at 08:09:21AM +0200, Christoph Hellwig wrote:
> On Mon, May 23, 2022 at 02:01:18PM -0700, Keith Busch wrote:
> > From: Keith Busch <kbusch@xxxxxxxxxx>
> >
> > Individual bv_len's may not be a sector size.
> >
> > Signed-off-by: Keith Busch <kbusch@xxxxxxxxxx>
> > ---
> > block/bounce.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/block/bounce.c b/block/bounce.c
> > index 8f7b6fe3b4db..20a43c4dbdda 100644
> > --- a/block/bounce.c
> > +++ b/block/bounce.c
> > @@ -207,17 +207,18 @@ void __blk_queue_bounce(struct request_queue *q, struct bio **bio_orig)
> > struct bvec_iter iter;
> > unsigned i = 0;
> > bool bounce = false;
> > - int sectors = 0;
> > + int sectors = 0, bytes = 0;
> >
> > bio_for_each_segment(from, *bio_orig, iter) {
> > if (i++ < BIO_MAX_VECS)
> > - sectors += from.bv_len >> 9;
> > + bytes += from.bv_len;
> > if (PageHighMem(from.bv_page))
> > bounce = true;
> > }
> > if (!bounce)
> > return;
> >
> > + sectors = ALIGN_DOWN(bytes, queue_logical_block_size(q)) >> 9;
>
> Same comment about SECTOR_SHIFT and a comment here. That being said,
> why do we even align here? Shouldn't bytes always be setor aligned here
> and this should be a WARN_ON or other sanity check? Probably the same
> for the previous patch.
Yes, the total bytes should be sector aligned.
I'm not exactly sure why we're counting it this way, though. We could just skip
the iterative addition and get the total from bio->bi_iter.bi_size. Unless
bio_orig has more segments that BIO_MAX_VECS, which doesn't seem possible.
