From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from esa5.hgst.iphmx.com ([216.71.153.144]:3641 "EHLO esa5.hgst.iphmx.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754837AbdDLSY2 (ORCPT ); Wed, 12 Apr 2017 14:24:28 -0400 From: Bart Van Assche To: "tom.leiming@gmail.com" CC: "linux-block@vger.kernel.org" , "stable@vger.kernel.org" , "axboe@kernel.dk" , "snitzer@redhat.com" Subject: Re: [PATCH] blk-mq: Fix blk_execute_rq_nowait() handling of dying queues Date: Wed, 12 Apr 2017 18:24:24 +0000 Message-ID: <1492021462.2764.13.camel@sandisk.com> References: <20170411235848.8686-1-bart.vanassche@sandisk.com> In-Reply-To: Content-Language: en-US Content-Type: text/plain; charset="iso-8859-1" Content-ID: Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Sender: stable-owner@vger.kernel.org List-ID: On Wed, 2017-04-12 at 13:01 +0800, Ming Lei wrote: > On Wed, Apr 12, 2017 at 7:58 AM, Bart Van Assche > wrote: > >=20 > > diff --git a/block/blk-exec.c b/block/blk-exec.c > > index 8cd0e9bc8dc8..f7d9bed2cb15 100644 > > --- a/block/blk-exec.c > > +++ b/block/blk-exec.c > > @@ -57,10 +57,13 @@ void blk_execute_rq_nowait(struct request_queue *q,= struct gendisk *bd_disk, > > rq->end_io =3D done; > >=20 > > /* > > - * don't check dying flag for MQ because the request won't > > - * be reused after dying flag is set > > + * The blk_freeze_queue() call in blk_set_queue_dying() and the > > + * test of the "dying" flag in blk_queue_enter() guarantee that > > + * blk_execute_rq_nowait() won't be called anymore after the "d= ying" > > + * flag has been set. >=20 > That never be guaranteed, see the following case: >=20 > 1) blk_get_request() is called just before queue is set as dying in anoth= er path >=20 > 2) the request is allocated successfully and passed to > blk_execute_rq_nowait() even > though queue has been set as dying Hello Ming, Shouldn't the blk-mq code guarantee that blk_execute_rq_nowait() won't be called anymore after the "dying" flag has been set? I think changing the blk_freeze_queue_start() call into blk_freeze_queue() in blk_set_queue_dyin= g() is sufficient to achieve this. Note: after I had posted this patch I have been able to reproduce the issue described in the patch description. Although I still think we need the patc= h at the start of this e-mail thread, it doesn't fix the issue I described. Bart.=