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: +85CvSmQ/rcud5HfaUNII7kgr9FhykEOkiVnNyWT2leersN3u5oFL2UzbZObUWqowg4hqVUkik
 b/wrs1W3T9p0nvGfA+D47tcK48zdb+Ec05qiYAHwpNU05h21l25Sl1dT6lRcMP7f0iWrU3MVin
 D8/GN27c9ylZ0oVsMiNzaL0WTtUbm1UQ8BKVAaRAYHLGLqdnpDpVouV1/+mVa9kFZvI2bxz4RN
 Jncb/J4ZaGGnG6EPWhvWqbGwk+/WrbClsb9ZSrh8LQSSmnUBrbtAWIjmUPCYoq1Nzgo8s1f8Ey
 HuU=
From:   Chaitanya Kulkarni <Chaitanya.Kulkarni@wdc.com>
To:     John Stultz <john.stultz@linaro.org>
CC:     David Anderson <dvander@google.com>,
        Christoph Hellwig <hch@lst.de>,
        Johannes Thumshirn <Johannes.Thumshirn@wdc.com>,
        Damien Le Moal <Damien.LeMoal@wdc.com>,
        Jens Axboe <axboe@kernel.dk>,
        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 04:58:38 +0000
Message-ID: <BYAPR04MB496545CBAACF978B9E11B77B86809@BYAPR04MB4965.namprd04.prod.outlook.com>
References: <CALAqxLUWjr2oR=5XxyGQ2HcC-TLARvboHRHHaAOUFq6_TsKXyw@mail.gmail.com>
 <BYAPR04MB496566A72BC5641BAC7D279F86809@BYAPR04MB4965.namprd04.prod.outlook.com>
 <CALAqxLXWs0GUZv=zWFK8hvnnkEgfMXvr_tZPyPaPBra=k9yf-A@mail.gmail.com>
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: e7ca2191-e124-44c2-aaf9-08d8d7b7af29
X-MS-Exchange-CrossTenant-originalarrivaltime: 23 Feb 2021 04:58:38.9456
 (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: pHoh6UHhtB+KwbNv731fdy1PI0PVlhTH2vH/NOJdEpn4C5AvR6i9gS1ac1yNdTYF0vpV27NIC1cRVwZy5+I9X0j4n2eRnd2elmhxE7z8Las=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR04MB4037
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 2/22/21 20:22, John Stultz wrote:=0A=
> On Mon, Feb 22, 2021 at 7:39 PM Chaitanya Kulkarni=0A=
> <Chaitanya.Kulkarni@wdc.com> wrote:=0A=
>> On 2/22/21 19:07, John Stultz wrote:=0A=
>>> [   34.784901] ueventd: LoadWithAliases was unable to load platform:reg=
ulatory=0A=
>>> [   34.785313]  bio_alloc_bioset+0x14/0x230=0A=
>>> [   34.796189]  bio_clone_fast+0x28/0x80=0A=
>>> [   34.799848]  bio_split+0x50/0xd0=0A=
>>> [   34.803072]  blk_crypto_fallback_encrypt_bio+0x2ec/0x5e8=0A=
>>> [   34.808384]  blk_crypto_fallback_bio_prep+0xfc/0x140=0A=
>>> [   34.813345]  __blk_crypto_bio_prep+0x13c/0x150=0A=
>>> [   34.817784]  submit_bio_noacct+0x3c0/0x548=0A=
>>> [   34.821880]  submit_bio+0x48/0x200=0A=
>>> [   34.825278]  ext4_io_submit+0x50/0x68=0A=
>>> [   34.828939]  ext4_writepages+0x558/0xca8=0A=
>>> [   34.832860]  do_writepages+0x58/0x108=0A=
>>> [   34.836522]  __writeback_single_inode+0x44/0x510=0A=
>>> [   34.841137]  writeback_sb_inodes+0x1e0/0x4a8=0A=
>>> [   34.845404]  __writeback_inodes_wb+0x78/0xe8=0A=
>>> [   34.849670]  wb_writeback+0x274/0x3e8=0A=
>>> [   34.853328]  wb_workfn+0x308/0x5f0=0A=
>>> [   34.856726]  process_one_work+0x1ec/0x4d0=0A=
>>> [   34.860734]  worker_thread+0x44/0x478=0A=
>>> [   34.864392]  kthread+0x140/0x150=0A=
>>> [   34.867618]  ret_from_fork+0x10/0x30=0A=
>>> [   34.871197] Code: a9ba7bfd 910003fd f9000bf3 7900bfa1 (f9403441)=0A=
>>> [   34.877289] ---[ end trace e6c2a3ab108278f0 ]---=0A=
>>> [   34.893636] Kernel panic - not syncing: Oops: Fatal exception=0A=
>>>=0A=
>> If you have time then until you get the reply from others, can you try=
=0A=
>> following patch ?=0A=
>>=0A=
>> diff --git a/block/bio.c b/block/bio.c=0A=
>> index a1c4d2900c7a..9976400ec66a 100644=0A=
>> --- a/block/bio.c=0A=
>> +++ b/block/bio.c=0A=
>> @@ -663,7 +663,10 @@ struct bio *bio_clone_fast(struct bio *bio, gfp_t=
=0A=
>> gfp_mask, struct bio_set *bs)=0A=
>>  {=0A=
>>         struct bio *b;=0A=
>>=0A=
>> -       b =3D bio_alloc_bioset(gfp_mask, 0, bs);=0A=
>> +       if (bs)=0A=
>> +               b =3D bio_alloc_bioset(gfp_mask, 0, bs);=0A=
>> +       else=0A=
>> +               b =3D bio_kmalloc(gfp_mask, 0);=0A=
>>         if (!b)=0A=
>>                 return NULL;=0A=
>>=0A=
>> P.S.This is purely based on the code inspection and it may not solve you=
r=0A=
>> issue. Proceed with the caution as it may *break* your system.=0A=
> So with an initial quick test, this patch (along with the follow-on=0A=
> one you sent) seems to avoid the issue.=0A=
Thanks for bisecting, the patch you are referring to is with the similar=0A=
fix for bio bounce ? =0A=
> I'm wondering if given there are multiple call sites, that in=0A=
> bio_alloc_bioset() would something like the following make more sense?=0A=
> (apologies, copy pasted so this is whitespace corrupted)=0A=
> thanks=0A=
> -john=0A=
I've asked Christoph about how we can go about this issue, based on his=0A=
reply I can send a patch.=0A=
=0A=
I think having kmalloc is what we want to avoid in the fast path, with=0A=
that in=0A=
mind in my opinion we need to fix the call sites with kmalloc call if numbe=
r=0A=
is small,  which I think it is. Let's wait.=0A=
=0A=
Meanwhile it will be great if you can share the result of the thorough=0A=
testing.=0A=
=0A=
> diff --git a/block/bio.c b/block/bio.c=0A=
> index a1c4d2900c7a..391d5cde79fc 100644=0A=
> --- a/block/bio.c=0A=
> +++ b/block/bio.c=0A=
> @@ -402,6 +402,9 @@ struct bio *bio_alloc_bioset(gfp_t gfp_mask,=0A=
> unsigned short nr_iovecs,=0A=
>         struct bio *bio;=0A=
>         void *p;=0A=
>=0A=
> +       if(!bs)=0A=
> +               return bio_kmalloc(gfp_mask, 0);=0A=
it may not work since nr_iovecs needs to be passed=0A=
> +=0A=
>         /* should not use nobvec bioset for nr_iovecs > 0 */=0A=
>         if (WARN_ON_ONCE(!mempool_initialized(&bs->bvec_pool) && nr_iovec=
s > 0))=0A=
>                 return NULL;=0A=
>=0A=
=0A=