Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: mJF33nMJ7fTVEMVBREYygAuhx1773lD5rJMX/hhAo4gsntdZU30tmqzbHyQwXRWxGjhW4ad96G
 +CdScOxXHtUrtjsWksdihPzv6w90xpVWWKSpWXH/noiu721lhlj09C2Y4sMbIckLcGMtxD6SH7
 FWqSZQ+J4vd/6TM2TKMV6nU1Z6f4/ErDSFVFst+SaEzniPCr7ild4SstbGWSyw90Muah7eeVXY
 i958L5h2YouJS1il9h0VvsS6SOX5baujK98Tp9JOn1/cvBpQuVLn8tBKWOSoVaXO9wbq6FLClu
 AZE=
From:   Chaitanya Kulkarni <Chaitanya.Kulkarni@wdc.com>
To:     Christoph Hellwig <hch@lst.de>
CC:     John Stultz <john.stultz@linaro.org>,
        Johannes Thumshirn <Johannes.Thumshirn@wdc.com>,
        Damien Le Moal <Damien.LeMoal@wdc.com>,
        Jens Axboe <axboe@kernel.dk>,
        David Anderson <dvander@google.com>,
        Alistair Delva <adelva@google.com>,
        Todd Kjos <tkjos@google.com>,
        Amit Pundir <amit.pundir@linaro.org>,
        YongQin Liu <yongqin.liu@linaro.org>,
        lkml <linux-kernel@vger.kernel.org>,
        "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>
Subject: Re: [REGRESSION] "split bio_kmalloc from bio_alloc_bioset" causing
 crash shortly after bootup
Thread-Topic: [REGRESSION] "split bio_kmalloc from bio_alloc_bioset" causing
 crash shortly after bootup
Thread-Index: AQHXCZD3vAo2wh9jek+o1gtQqbdo1w==
Date:   Tue, 23 Feb 2021 07:21:34 +0000
Message-ID: <BYAPR04MB496573915041C5AF50AAA7E786809@BYAPR04MB4965.namprd04.prod.outlook.com>
References: <CALAqxLUWjr2oR=5XxyGQ2HcC-TLARvboHRHHaAOUFq6_TsKXyw@mail.gmail.com>
 <BYAPR04MB4965F0B60169371A25CD423E86809@BYAPR04MB4965.namprd04.prod.outlook.com>
 <20210223071040.GB16980@lst.de>
Accept-Language: en-US
Content-Language: en-US
wdcipoutbound: EOP-TRUE
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 7493c64e-d6b2-477b-6cff-08d8d7cba6d5
X-MS-Exchange-CrossTenant-originalarrivaltime: 23 Feb 2021 07:21:34.8863
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: b61c8803-16f3-4c35-9b17-6f65f441df86
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: Q5aprTmzRqpdROA8VLQoD4Og88hJaH6Tw9cfEJbUfQF5OhETTvxh/nxVKTRTJQCvb89uVmBzhJZIKztdWNPZgzoZzve9z2aoYsZveB5nUWQ=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR04MB6549
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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