Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190409090828.16282-1-bob.liu@oracle.com>
In-Reply-To: <20190409090828.16282-1-bob.liu@oracle.com>
From:   Jinpu Wang <jinpuwang@gmail.com>
Date:   Tue, 9 Apr 2019 11:29:42 +0200
Message-ID: <CAD9gYJKYu15b7ZuAvud-BzG5fRP=B33yPJuEq84nsN_8y3EC1Q@mail.gmail.com>
Subject: Re: [RESEND PATCH] blk-mq: fix hang caused by freeze/unfreeze sequence
To:     Bob Liu <bob.liu@oracle.com>, r.peniaev@gmail.com
Cc:     linux-block@vger.kernel.org, shirley.ma@oracle.com,
        "Martin K. Petersen" <martin.petersen@oracle.com>,
        Akinobu Mita <akinobu.mita@gmail.com>,
        Tejun Heo <tj@kernel.org>, Jens Axboe <axboe@kernel.dk>,
        Christoph Hellwig <hch@lst.de>,
        LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Bob Liu <bob.liu@oracle.com> =E4=BA=8E2019=E5=B9=B44=E6=9C=889=E6=97=A5=E5=
=91=A8=E4=BA=8C =E4=B8=8A=E5=8D=8811:11=E5=86=99=E9=81=93=EF=BC=9A
>
> This patch was proposed by Roman Pen[3] years ago.
> Recently we hit a bug which is likely caused by the same reason,so rebase=
d his
> fix to v5.1 and resend.
> Below is almost copied from that patch[3].
>
> ------
> Long time ago there was a similar fix proposed by Akinobu Mita[1],
> but it seems that time everyone decided to fix this subtle race in
> percpu-refcount and Tejun Heo[2] did an attempt (as I can see that
> patchset was not applied).
>
> The following is a description of a hang in blk_mq_freeze_queue_wait() -
> same fix but a bug from another angle.
>
> The hang happens on attempt to freeze a queue while another task does
> queue unfreeze.
>
> The root cause is an incorrect sequence of percpu_ref_reinit() and
> percpu_ref_kill() and as a result those two can be swapped:
>
>  CPU#0               CPU#1
>  ----------------    -----------------
>  percpu_ref_kill()
>
>                      percpu_ref_kill() << atomic reference does
>  percpu_ref_reinit()                   << not guarantee the order
>
>                      blk_mq_freeze_queue_wait() << HANG HERE
>
>                      percpu_ref_reinit()
>
> Firstly this wrong sequence raises two kernel warnings:
>
>   1st. WARNING at lib/percpu-recount.c:309
>        percpu_ref_kill_and_confirm called more than once
>
>   2nd. WARNING at lib/percpu-refcount.c:331
>
> But the most unpleasant effect is a hang of a blk_mq_freeze_queue_wait(),
> which waits for a zero of a q_usage_counter, which never happens
> because percpu-ref was reinited (instead of being killed) and stays in
> PERCPU state forever.
>
> The simplified sequence above can be reproduced on shared tags, when
> queue A is going to die meanwhile another queue B is in init state and
> is trying to freeze the queue A, which shares the same tags set:
>
>  CPU#0                           CPU#1
>  ------------------------------- ------------------------------------
>  q1 =3D blk_mq_init_queue(shared_tags)
>
>                                 q2 =3D blk_mq_init_queue(shared_tags):
>                                   blk_mq_add_queue_tag_set(shared_tags):
>                                     blk_mq_update_tag_set_depth(shared_ta=
gs):
>                                       blk_mq_freeze_queue(q1)
>  blk_cleanup_queue(q1)                 ...
>    blk_mq_freeze_queue(q1)   <<<->>>   blk_mq_unfreeze_queue(q1)
>
> [1] Message id: 1443287365-4244-7-git-send-email-akinobu.mita@gmail.com
> [2] Message id: 1443563240-29306-6-git-send-email-tj@kernel.org
> [3] https://patchwork.kernel.org/patch/9268199/
>
> Signed-off-by: Roman Pen <roman.penyaev@profitbricks.com>
> Signed-off-by: Bob Liu <bob.liu@oracle.com>
> Cc: Akinobu Mita <akinobu.mita@gmail.com>
> Cc: Tejun Heo <tj@kernel.org>
> Cc: Jens Axboe <axboe@kernel.dk>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: linux-block@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
>

Replaced Roman's email address.

We at 1 & 1 IONOS (former ProfitBricks) have been carried this patch
for some years,
it has been running in production for some years too, would be good to
see it in upstream :)

Thanks,

Jack Wang
Linux Kernel Developer @ 1 & 1 IONOS