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=-4.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SIGNED_OFF_BY,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 ACD88C43381 for ; Fri, 15 Feb 2019 09:25:36 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 80702206B7 for ; Fri, 15 Feb 2019 09:25:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2390871AbfBOJYy (ORCPT ); Fri, 15 Feb 2019 04:24:54 -0500 Received: from usa-sjc-mx-foss1.foss.arm.com ([217.140.101.70]:56804 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726473AbfBOJYy (ORCPT ); Fri, 15 Feb 2019 04:24:54 -0500 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 62187A78; Fri, 15 Feb 2019 01:24:53 -0800 (PST) Received: from big-swifty.misterjones.org (usa-sjc-mx-foss1.foss.arm.com [217.140.101.70]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 5EB043F557; Fri, 15 Feb 2019 01:24:46 -0800 (PST) Date: Fri, 15 Feb 2019 09:24:36 +0000 Message-ID: <868syhs8tn.wl-marc.zyngier@arm.com> From: Marc Zyngier To: Thomas Gleixner Cc: LKML , Ming Lei , Christoph Hellwig , Bjorn Helgaas , Jens Axboe , , Sagi Grimberg , , , Keith Busch , Sumit Saxena , Kashyap Desai , Shivasharan Srikanteshwara Subject: Re: [patch V5 4/8] nvme-pci: Simplify interrupt allocation In-Reply-To: <20190214211759.699390983@linutronix.de> References: <20190214204755.819014197@linutronix.de> <20190214211759.699390983@linutronix.de> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 EasyPG/1.0.0 Emacs/25.1 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO) Organization: ARM Ltd MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org On Thu, 14 Feb 2019 20:47:59 +0000, Thomas Gleixner wrote: > > From: Ming Lei > > The NVME PCI driver contains a tedious mechanism for interrupt > allocation, which is necessary to adjust the number and size of interrupt > sets to the maximum available number of interrupts which depends on the > underlying PCI capabilities and the available CPU resources. > > It works around the former short comings of the PCI and core interrupt > allocation mechanims in combination with interrupt sets. > > The PCI interrupt allocation function allows to provide a maximum and a > minimum number of interrupts to be allocated and tries to allocate as > many as possible. This worked without driver interaction as long as there > was only a single set of interrupts to handle. > > With the addition of support for multiple interrupt sets in the generic > affinity spreading logic, which is invoked from the PCI interrupt > allocation, the adaptive loop in the PCI interrupt allocation did not > work for multiple interrupt sets. The reason is that depending on the > total number of interrupts which the PCI allocation adaptive loop tries > to allocate in each step, the number and the size of the interrupt sets > need to be adapted as well. Due to the way the interrupt sets support was > implemented there was no way for the PCI interrupt allocation code or the > core affinity spreading mechanism to invoke a driver specific function > for adapting the interrupt sets configuration. > > As a consequence the driver had to implement another adaptive loop around > the PCI interrupt allocation function and calling that with maximum and > minimum interrupts set to the same value. This ensured that the > allocation either succeeded or immediately failed without any attempt to > adjust the number of interrupts in the PCI code. > > The core code now allows drivers to provide a callback to recalculate the > number and the size of interrupt sets during PCI interrupt allocation, > which in turn allows the PCI interrupt allocation function to be called > in the same way as with a single set of interrupts. The PCI code handles > the adaptive loop and the interrupt affinity spreading mechanism invokes > the driver callback to adapt the interrupt set configuration to the > current loop value. This replaces the adaptive loop in the driver > completely. > > Implement the NVME specific callback which adjusts the interrupt sets > configuration and remove the adaptive allocation loop. > > [ tglx: Simplify the callback further and restore the dropped adjustment of > number of sets ] > > Signed-off-by: Ming Lei > Signed-off-by: Thomas Gleixner > > --- > 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. Thanks, M. -- Jazz is not dead, it just smell funny.