Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <D98ED975-F617-4885-8D3F-DCFDC524E933@oracle.com>
In-Reply-To: <D98ED975-F617-4885-8D3F-DCFDC524E933@oracle.com>
From:   Doug Anderson <dianders@chromium.org>
Date:   Mon, 24 Jul 2023 07:39:26 -0700
Message-ID: <CAD=FV=VuJZNirf1DtcE_d8-aBRb9O_fYk_r252WCzC-YHwYnbg@mail.gmail.com>
Subject: Re: VM Boot Hangs with Commit "Revert "scsi: core: run queue if SCSI
 device queue isn't ready and queue is idle"" on linux-5.4.y
To:     Sherry Yang <sherry.yang@oracle.com>
Cc:     Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>,
        George Kennedy <george.kennedy@oracle.com>,
        "linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi,

On Sun, Jul 23, 2023 at 10:28=E2=80=AFPM Sherry Yang <sherry.yang@oracle.co=
m> wrote:
>
> Hi Douglas,
>
> We observed linux-stable v5.4 VM boot hangs, but probably only 1 in thous=
ands of boots (less than 10,000 boots).  We started 16 VMs on a Bare Metal =
with loop reboots, I chose 10,000 boots as a threshold, and bisected it. Af=
ter a painful bisection, I found the culprit commit 578c8f09c04b (=E2=80=9C=
Revert scsi: core: run queue if SCSI device queue isnt ready and queue is i=
dle=E2=80=9D). This commit actually was merged to v5.8 the 1st time. It's a=
 series of patch set (https://www.spinics.net/lists/linux-block/msg51866.ht=
ml). Actually, in the 4-patch series, 2 of them have already been backporte=
d to linux-stable v5.4, but not at the same time:
>
> 1) ab3cee3762e5 (=E2=80=9Cblk-mq: In blk_mq_dispatch_rq_list() no budget =
is a reason to kick=E2=80=9D) in tag v5.4.86
> 2) 578c8f09c04b (=E2=80=9CRevert scsi: core: run queue if SCSI device que=
ue isnt ready and queue is idle=E2=80=9D) in tag v5.4.235, it=E2=80=99s bac=
kported as stable dependency for another commit
>
>         Signed-off-by: Douglas Anderson <dianders@chromium.org>
>         Reviewed-by: Ming Lei <ming.lei@redhat.com>
>         Acked-by: Martin K. Petersen <martin.petersen@oracle.com>
>         Signed-off-by: Jens Axboe <axboe@kernel.dk>
>         Stable-dep-of: c31e76bcc379 ("blk-mq: remove stale comment for bl=
k_mq_sched_mark_restart_hctx=E2=80=9D)
>         Signed-off-by: Sasha Levin <sashal@kernel.org>
>
> And I tried backporting the other 2 patches to v5.4, the issue is still r=
eproducible.
>
> I tested multiple kernels, the issue is not reproducible within 10,000 bo=
ots in the following kernels:
> 1) Linux v5.9
> 2) Linux v5.4.249 + revert of 578c8f09c04b (=E2=80=9CRevert scsi: core: r=
un queue if SCSI device queue isnt ready and queue is idle=E2=80=9D)
>
> Not exactly sure how this commit is affecting linux-stable v5.4, but I su=
spect some prerequisite commits are missing which lead to boot hangs on lin=
ux-stable v5.4 but not on higher releases. Could you take a look at this is=
sue and share your insight?

Ugh, I spent many days pouring over the code and digging through debug
traces in order to write those patches. I don't think I'd be able to
give any concrete advice without spending many days and being able to
reproduce multiple times with traces since pretty much any knowledge I
learned during the course of developing those patches has decayed over
the last several years. :( I don't happen to know any dependencies
offhand...

That being said, it seems like:

1. Backporting the revert (the 4th patch in the series) without all
the other patches in the series feels wrong. In the text of the revert
I explicitly refer to the other patches in the series as
prerequisites. I guess you said you tried backporting the other two
patches and they didn't help, though? That's no good. :(

2. I don't think the revert is actually important to backport to
stable. While the first 3 patches were important to fix the problems I
was seeing, the revert was just a cleanup. If the revert is causing
problems in 5.4.x then I'd suggest removing it from 5.4.x

Does that make sense? So ideally you'd submit 3 patches to the stable kerne=
l:

a) Revert the revert

b) Pick ("blk-mq: Add blk_mq_delay_run_hw_queues() API call")

c) Pick ("blk-mq: Rerun dispatching in the case of budget contention")


FWIW, we seem to have all 4 patches in the ChromeOS 5.4 kernel tree.
They all landed together plus 1 prerequisite.

* https://crrev.com/c/2155423 - FROMGIT: Revert "scsi: core: run queue
if SCSI device queue isn't ready and queue is idle"
* https://crrev.com/c/2133069 - FROMGIT: blk-mq: Rerun dispatching in
the case of budget contention
* https://crrev.com/c/2155422 - FROMGIT: blk-mq: Add
blk_mq_delay_run_hw_queues() API call
* https://crrev.com/c/2125232 - FROMGIT: blk-mq: In
blk_mq_dispatch_rq_list() "no budget" is a reason to kick
* https://crrev.com/c/2155421 - UPSTREAM: blk-mq: Put driver tag in
blk_mq_dispatch_rq_list() when no budget

When I saw the prerequisite in there I was hopeful that it was the one
you needed, but it looks like that's already in 5.4 stable so (I
presume) you've already been testing with it...

-Doug