From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753839AbcBAQRZ (ORCPT ); Mon, 1 Feb 2016 11:17:25 -0500 Received: from mail-shaop0215.outbound.protection.partner.outlook.cn ([42.159.161.215]:12642 "EHLO cn01-SHA-obe.outbound.protection.partner.outlook.cn" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752170AbcBAQRV (ORCPT ); Mon, 1 Feb 2016 11:17:21 -0500 From: Wenbo Wang To: "Busch, Keith" , Wenbo Wang , "axboe@fb.com" CC: "linux-kernel@vger.kernel.org" , "linux-nvme@lists.infradead.org" Subject: RE: [PATCH] NVMe: do not touch sq door bell if nvmeq has been suspended Thread-Topic: [PATCH] NVMe: do not touch sq door bell if nvmeq has been suspended Thread-Index: AQHRXQdLsUB4hBdIiEurcoY6kDvY9J8XWREAgAADXiA= Date: Mon, 1 Feb 2016 16:17:09 +0000 Message-ID: References: <1454341324-21273-1-git-send-email-mail_weber_wang@163.com> In-Reply-To: Accept-Language: zh-CN, en-US Content-Language: zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=wenbo.wang@memblaze.com; x-originating-ip: [111.204.49.2] x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:BJXPR01MB200; x-exchange-antispam-report-cfa: BCL:0;PCL:0;RULEID:;SRVR:BJXPR01MB200;BCL:0;PCL:0;RULEID:;SRVR:BJXPR01MB200; x-forefront-prvs: 0839D067E7 x-forefront-antispam-report: SFV:NSPM;SFS:(10009020)(6009001)(189002)(329002)(13464003)(328002)(199002)(377454003)(56816005)(81156007)(94316002)(5001770100001)(97736004)(59766002)(63696004)(50986002)(97186001)(97336001)(87936001)(93136001)(87266001)(76796001)(51856002)(76786001)(3280700002)(49866002)(54316003)(56776002)(47976003)(6116002)(53806002)(95416001)(81816001)(47736002)(54356002)(98676001)(47446003)(90146001)(65816002)(74706001)(19580395003)(81686001)(74366001)(19580405001)(74876001)(101416001)(93516002)(33656002)(94946001)(92566002)(2900100001)(105586002)(86362001)(66066001)(2950100001)(106116001)(106356001)(11100500001)(5004730100002)(69226001)(1220700001)(4326007)(189998001)(77096005)(5003600100002)(586003)(3660700001)(102836003)(2501003)(3470700001)(10400500002)(5002640100001)(3846002)(5008740100001)(13296004);DIR:OUT;SFP:1101;SCL:1;SRVR:BJXPR01MB200;H:BJXPR01MB199.CHNPR01.prod.partner.outlook.cn;FPR:;SPF:None;PTR:InfoNoRecords;MX:1;LANG:en; Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 X-OriginatorOrg: memblaze.com X-MS-Exchange-CrossTenant-originalarrivaltime: 01 Feb 2016 16:17:09.9177 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: a7c7cec6-5dd2-4af2-a218-b0f5e8246be0 X-MS-Exchange-Transport-CrossTenantHeadersStamped: BJXPR01MB200 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id u11GHUsI011839 No, this issue has not been seen yet. Since blk_mq_run_hw_queue tests queue stopped in the very beginning. It should be possible to race with nvme_dev_disable. I agree that the request shall be re-queued. -----Original Message----- From: Busch, Keith [mailto:keith.busch@intel.com] Sent: Tuesday, February 2, 2016 12:00 AM To: Wenbo Wang; axboe@fb.com Cc: linux-kernel@vger.kernel.org; Wenbo Wang; linux-nvme@lists.infradead.org Subject: RE: [PATCH] NVMe: do not touch sq door bell if nvmeq has been suspended Does this ever happen? The queue should be stopped before the bar is unmapped. If that's insufficient to guard against this, we've another problem this patch does not cover. That command will just timeout since it was accepted by the driver, but not actually submitted to anything. The request needs to be requeued or return BLK_MQ_RQ_QUEUE_BUSY. > -----Original Message----- > From: Wenbo Wang [mailto:mail_weber_wang@163.com] > Sent: Monday, February 01, 2016 8:42 AM > To: axboe@fb.com; Busch, Keith > Cc: linux-kernel@vger.kernel.org; Wenbo Wang; Wenbo Wang; > linux-nvme@lists.infradead.org > Subject: [PATCH] NVMe: do not touch sq door bell if nvmeq has been > suspended > > If __nvme_submit_cmd races with nvme_dev_disable, nvmeq could have > been suspended and dev->bar could have been unmapped. Do not touch sq > door bell in this case. > > Signed-off-by: Wenbo Wang > Reviewed-by: Wenwei Tao > CC: linux-nvme@lists.infradead.org > --- > drivers/nvme/host/pci.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c index > 8b1a725..2288712 100644 > --- a/drivers/nvme/host/pci.c > +++ b/drivers/nvme/host/pci.c > @@ -325,7 +325,8 @@ static void __nvme_submit_cmd(struct nvme_queue > *nvmeq, > > if (++tail == nvmeq->q_depth) > tail = 0; > - writel(tail, nvmeq->q_db); > + if (likely(nvmeq->cq_vector >= 0)) > + writel(tail, nvmeq->q_db); > nvmeq->sq_tail = tail; > } > > -- > 1.8.3.1 From mboxrd@z Thu Jan 1 00:00:00 1970 From: wenbo.wang@memblaze.com (Wenbo Wang) Date: Mon, 1 Feb 2016 16:17:09 +0000 Subject: [PATCH] NVMe: do not touch sq door bell if nvmeq has been suspended In-Reply-To: References: <1454341324-21273-1-git-send-email-mail_weber_wang@163.com> Message-ID: No, this issue has not been seen yet. Since blk_mq_run_hw_queue tests queue stopped in the very beginning. It should be possible to race with nvme_dev_disable. I agree that the request shall be re-queued. -----Original Message----- From: Busch, Keith [mailto:keith.busch@intel.com] Sent: Tuesday, February 2, 2016 12:00 AM To: Wenbo Wang; axboe at fb.com Cc: linux-kernel at vger.kernel.org; Wenbo Wang; linux-nvme at lists.infradead.org Subject: RE: [PATCH] NVMe: do not touch sq door bell if nvmeq has been suspended Does this ever happen? The queue should be stopped before the bar is unmapped. If that's insufficient to guard against this, we've another problem this patch does not cover. That command will just timeout since it was accepted by the driver, but not actually submitted to anything. The request needs to be requeued or return BLK_MQ_RQ_QUEUE_BUSY. > -----Original Message----- > From: Wenbo Wang [mailto:mail_weber_wang at 163.com] > Sent: Monday, February 01, 2016 8:42 AM > To: axboe at fb.com; Busch, Keith > Cc: linux-kernel at vger.kernel.org; Wenbo Wang; Wenbo Wang; > linux-nvme at lists.infradead.org > Subject: [PATCH] NVMe: do not touch sq door bell if nvmeq has been > suspended > > If __nvme_submit_cmd races with nvme_dev_disable, nvmeq could have > been suspended and dev->bar could have been unmapped. Do not touch sq > door bell in this case. > > Signed-off-by: Wenbo Wang > Reviewed-by: Wenwei Tao > CC: linux-nvme at lists.infradead.org > --- > drivers/nvme/host/pci.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c index > 8b1a725..2288712 100644 > --- a/drivers/nvme/host/pci.c > +++ b/drivers/nvme/host/pci.c > @@ -325,7 +325,8 @@ static void __nvme_submit_cmd(struct nvme_queue > *nvmeq, > > if (++tail == nvmeq->q_depth) > tail = 0; > - writel(tail, nvmeq->q_db); > + if (likely(nvmeq->cq_vector >= 0)) > + writel(tail, nvmeq->q_db); > nvmeq->sq_tail = tail; > } > > -- > 1.8.3.1