Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753395AbdGSJVE (ORCPT ); Wed, 19 Jul 2017 05:21:04 -0400 Received: from mail-wm0-f67.google.com ([74.125.82.67]:34619 "EHLO mail-wm0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751911AbdGSJVB (ORCPT ); Wed, 19 Jul 2017 05:21:01 -0400 Subject: Re: [PATCH] nvme: Acknowledge completion queue on each iteration To: Sinan Kaya , linux-nvme@lists.infradead.org, timur@codeaurora.org Cc: linux-arm-msm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Keith Busch , Jens Axboe , Christoph Hellwig , linux-kernel@vger.kernel.org References: <1500330983-27501-1-git-send-email-okaya@codeaurora.org> From: Sagi Grimberg Message-ID: <5595ca25-f616-c0f8-fb2c-241a951e8848@grimberg.me> Date: Wed, 19 Jul 2017 12:20:57 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <1500330983-27501-1-git-send-email-okaya@codeaurora.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1124 Lines: 34 > Code is moving the completion queue doorbell after processing all completed > events and sending callbacks to the block layer on each iteration. > > This is causing a performance drop when a lot of jobs are queued towards > the HW. Move the completion queue doorbell on each loop instead and allow new > jobs to be queued by the HW. > > Signed-off-by: Sinan Kaya > --- > drivers/nvme/host/pci.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c > index d10d2f2..33d9b5b 100644 > --- a/drivers/nvme/host/pci.c > +++ b/drivers/nvme/host/pci.c > @@ -810,13 +810,12 @@ static void nvme_process_cq(struct nvme_queue *nvmeq) > > while (nvme_read_cqe(nvmeq, &cqe)) { > nvme_handle_cqe(nvmeq, &cqe); > + nvme_ring_cq_doorbell(nvmeq); > consumed++; > } > > - if (consumed) { > - nvme_ring_cq_doorbell(nvmeq); > + if (consumed) > nvmeq->cqe_seen = 1; > - } > } Agree with Keith that this is definitely not the way to go, it adds mmio operations in the hot path with very little gain (if at all).