On 2/22/21 23:10, Christoph Hellwig wrote:
> Well, that is a somewhat odd calling convention. What about the patch below
> instead? That being we really need to kill this bouncing code off..
If we can kill it off soon it will be great.
>
> diff --git a/block/bounce.c b/block/bounce.c
> index fc55314aa4269a..789fbcacb1e92a 100644
> --- a/block/bounce.c
> +++ b/block/bounce.c
> @@ -214,9 +214,9 @@ static void bounce_end_io_read_isa(struct bio *bio)
> __bounce_end_io_read(bio, &isa_page_pool);
> }
>
> -static struct bio *bounce_clone_bio(struct bio *bio_src, gfp_t gfp_mask,
> - struct bio_set *bs)
> +static struct bio *bounce_clone_bio(struct bio *bio_src, bool passthrough)
> {
> + unsigned int nr_vecs = bio_segments(bio_src);
> struct bvec_iter iter;
> struct bio_vec bv;
> struct bio *bio;
> @@ -242,8 +242,10 @@ static struct bio *bounce_clone_bio(struct bio *bio_src, gfp_t gfp_mask,
> * asking for trouble and would force extra work on
> * __bio_clone_fast() anyways.
> */
> -
> - bio = bio_alloc_bioset(gfp_mask, bio_segments(bio_src), bs);
> + if (passthrough)
> + bio = bio_kmalloc(GFP_NOIO, nr_vecs);
> + else
> + bio = bio_alloc_bioset(GFP_NOIO, nr_vecs, &bounce_bio_set);
> if (!bio)
> return NULL;
> bio->bi_bdev = bio_src->bi_bdev;
> @@ -269,11 +271,11 @@ static struct bio *bounce_clone_bio(struct bio *bio_src, gfp_t gfp_mask,
> break;
> }
>
> - if (bio_crypt_clone(bio, bio_src, gfp_mask) < 0)
> + if (bio_crypt_clone(bio, bio_src, GFP_NOIO) < 0)
> goto err_put;
>
> if (bio_integrity(bio_src) &&
> - bio_integrity_clone(bio, bio_src, gfp_mask) < 0)
> + bio_integrity_clone(bio, bio_src, GFP_NOIO) < 0)
> goto err_put;
>
> bio_clone_blkg_association(bio, bio_src);
> @@ -313,8 +315,7 @@ static void __blk_queue_bounce(struct request_queue *q, struct bio **bio_orig,
> submit_bio_noacct(*bio_orig);
> *bio_orig = bio;
> }
> - bio = bounce_clone_bio(*bio_orig, GFP_NOIO, passthrough ? NULL :
> - &bounce_bio_set);
> + bio = bounce_clone_bio(*bio_orig, passthrough);
>
> /*
> * Bvec table can't be updated by bio_for_each_segment_all(),
>
Seems like this fixes the issue and does the cleanup in one patch, looks
good.
