From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 45C85C43381 for ; Fri, 15 Feb 2019 09:53:13 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 1BF852070D for ; Fri, 15 Feb 2019 09:53:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731002AbfBOJxH (ORCPT ); Fri, 15 Feb 2019 04:53:07 -0500 Received: from Galois.linutronix.de ([146.0.238.70]:52191 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730379AbfBOJxH (ORCPT ); Fri, 15 Feb 2019 04:53:07 -0500 Received: from p5492e0d8.dip0.t-ipconnect.de ([84.146.224.216] helo=nanos) by Galois.linutronix.de with esmtpsa (TLS1.2:DHE_RSA_AES_256_CBC_SHA256:256) (Exim 4.80) (envelope-from ) id 1guaAu-0007vQ-4W; Fri, 15 Feb 2019 10:52:52 +0100 Date: Fri, 15 Feb 2019 10:52:51 +0100 (CET) From: Thomas Gleixner To: Marc Zyngier cc: LKML , Ming Lei , Christoph Hellwig , Bjorn Helgaas , Jens Axboe , linux-block@vger.kernel.org, Sagi Grimberg , linux-nvme@lists.infradead.org, linux-pci@vger.kernel.org, Keith Busch , Sumit Saxena , Kashyap Desai , Shivasharan Srikanteshwara Subject: Re: [patch V5 4/8] nvme-pci: Simplify interrupt allocation In-Reply-To: <868syhs8tn.wl-marc.zyngier@arm.com> Message-ID: References: <20190214204755.819014197@linutronix.de> <20190214211759.699390983@linutronix.de> <868syhs8tn.wl-marc.zyngier@arm.com> User-Agent: Alpine 2.21 (DEB 202 2017-01-01) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Linutronix-Spam-Score: -1.0 X-Linutronix-Spam-Level: - X-Linutronix-Spam-Status: No , -1.0 points, 5.0 required, ALL_TRUSTED=-1,SHORTCIRCUIT=-0.0001 Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org On Fri, 15 Feb 2019, Marc Zyngier wrote: > On Thu, 14 Feb 2019 20:47:59 +0000, > Thomas Gleixner wrote: > > drivers/nvme/host/pci.c | 108 ++++++++++++------------------------------------ > > 1 file changed, 28 insertions(+), 80 deletions(-) > > > > --- a/drivers/nvme/host/pci.c > > +++ b/drivers/nvme/host/pci.c > > @@ -2041,41 +2041,32 @@ static int nvme_setup_host_mem(struct nv > > return ret; > > } > > > > -/* irq_queues covers admin queue */ > > -static void nvme_calc_io_queues(struct nvme_dev *dev, unsigned int irq_queues) > > +/* > > + * nirqs is the number of interrupts available for write and read > > + * queues. The core already reserved an interrupt for the admin queue. > > + */ > > +static void nvme_calc_irq_sets(struct irq_affinity *affd, unsigned int nrirqs) > > { > > - unsigned int this_w_queues = write_queues; > > - > > - WARN_ON(!irq_queues); > > - > > - /* > > - * Setup read/write queue split, assign admin queue one independent > > - * irq vector if irq_queues is > 1. > > - */ > > - if (irq_queues <= 2) { > > - dev->io_queues[HCTX_TYPE_DEFAULT] = 1; > > - dev->io_queues[HCTX_TYPE_READ] = 0; > > - return; > > - } > > + struct nvme_dev *dev = affd->priv; > > + unsigned int nr_read_queues; > > > > /* > > - * If 'write_queues' is set, ensure it leaves room for at least > > - * one read queue and one admin queue > > - */ > > - if (this_w_queues >= irq_queues) > > - this_w_queues = irq_queues - 2; > > - > > - /* > > - * If 'write_queues' is set to zero, reads and writes will share > > - * a queue set. > > - */ > > - if (!this_w_queues) { > > - dev->io_queues[HCTX_TYPE_DEFAULT] = irq_queues - 1; > > - dev->io_queues[HCTX_TYPE_READ] = 0; > > - } else { > > - dev->io_queues[HCTX_TYPE_DEFAULT] = this_w_queues; > > - dev->io_queues[HCTX_TYPE_READ] = irq_queues - this_w_queues - 1; > > - } > > + * If only one interrupt is available, combine write and read > > + * queues. If 'write_queues' is set, ensure it leaves room for at > > + * least one read queue. > > [Full disclaimer: I only have had two coffees this morning, and it is > only at the fourth that my brain is able to kick in...] > > I don't know much about NVME, but I feel like there is a small > disconnect between the code and the above comment, which says "leave > room for at least one read queue"... > > > + */ > > + if (nrirqs == 1) > > + nr_read_queues = 0; > > + else if (write_queues >= nrirqs) > > + nr_read_queues = nrirqs - 1; > > ... while this seem to ensure that we carve out one write queue out of > the irq set. It looks like a departure from the original code, which > would set nr_read_queues to 1 in that particular case. Bah. right you are.