From: Bjorn Helgaas <helgaas@kernel.org>
To: Ming Lei <ming.lei@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>,
Thomas Gleixner <tglx@linutronix.de>,
Jens Axboe <axboe@kernel.dk>,
linux-block@vger.kernel.org, Sagi Grimberg <sagi@grimberg.me>,
linux-nvme@lists.infradead.org, linux-kernel@vger.kernel.org,
linux-pci@vger.kernel.org, Keith Busch <keith.busch@intel.com>
Subject: Re: [PATCH V3 2/5] genirq/affinity: store irq set vectors in 'struct irq_affinity'
Date: Wed, 13 Feb 2019 09:07:24 -0600 [thread overview]
Message-ID: <20190213150723.GC96272@google.com> (raw)
In-Reply-To: <20190213105041.13537-3-ming.lei@redhat.com>
On Wed, Feb 13, 2019 at 06:50:38PM +0800, Ming Lei wrote:
> Currently the array of irq set vectors is provided by driver.
>
> irq_create_affinity_masks() can be simplied a bit by treating the
> non-irq-set case as single irq set.
s/simplied a bit/simplified/
> So move this array into 'struct irq_affinity', and pre-define the max
> set number as 4, which should be enough for normal cases.
s/irq/IRQ/
You have a real mixture of capitalization across the changelogs.
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
> drivers/nvme/host/pci.c | 5 ++---
> include/linux/interrupt.h | 6 ++++--
> kernel/irq/affinity.c | 18 +++++++++++-------
> 3 files changed, 17 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index 022ea1ee63f8..0086bdf80ea1 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -2081,12 +2081,11 @@ static void nvme_calc_io_queues(struct nvme_dev *dev, unsigned int irq_queues)
> static int nvme_setup_irqs(struct nvme_dev *dev, unsigned int nr_io_queues)
> {
> struct pci_dev *pdev = to_pci_dev(dev->dev);
> - int irq_sets[2];
> struct irq_affinity affd = {
> .pre_vectors = 1,
> - .nr_sets = ARRAY_SIZE(irq_sets),
> - .sets = irq_sets,
> + .nr_sets = 2,
> };
> + int *irq_sets = affd.set_vectors;
> int result = 0;
> unsigned int irq_queues, this_p_queues;
>
> diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
> index 1ed1014c9684..a20150627a32 100644
> --- a/include/linux/interrupt.h
> +++ b/include/linux/interrupt.h
> @@ -259,6 +259,8 @@ struct irq_affinity_notify {
> void (*release)(struct kref *ref);
> };
>
> +#define IRQ_MAX_SETS 4
This is a pretty generic name. Maybe it should include a hint that it's
related to affinity?
> /**
> * struct irq_affinity - Description for automatic irq affinity assignements
> * @pre_vectors: Don't apply affinity to @pre_vectors at beginning of
> @@ -266,13 +268,13 @@ struct irq_affinity_notify {
> * @post_vectors: Don't apply affinity to @post_vectors at end of
> * the MSI(-X) vector space
> * @nr_sets: Length of passed in *sets array
> - * @sets: Number of affinitized sets
> + * @set_vectors: Number of affinitized sets
> */
> struct irq_affinity {
> int pre_vectors;
> int post_vectors;
> int nr_sets;
> - int *sets;
> + int set_vectors[IRQ_MAX_SETS];
> };
>
> /**
> diff --git a/kernel/irq/affinity.c b/kernel/irq/affinity.c
> index 9200d3b26f7d..b868b9d3df7f 100644
> --- a/kernel/irq/affinity.c
> +++ b/kernel/irq/affinity.c
> @@ -244,7 +244,7 @@ irq_create_affinity_masks(int nvecs, struct irq_affinity *affd)
> int affvecs = nvecs - affd->pre_vectors - affd->post_vectors;
> int curvec, usedvecs;
> struct irq_affinity_desc *masks = NULL;
> - int i, nr_sets;
> + int i;
>
> /*
> * If there aren't any vectors left after applying the pre/post
> @@ -253,6 +253,9 @@ irq_create_affinity_masks(int nvecs, struct irq_affinity *affd)
> if (nvecs == affd->pre_vectors + affd->post_vectors)
> return NULL;
>
> + if (affd->nr_sets > IRQ_MAX_SETS)
> + return NULL;
> +
> masks = kcalloc(nvecs, sizeof(*masks), GFP_KERNEL);
> if (!masks)
> return NULL;
> @@ -264,12 +267,13 @@ irq_create_affinity_masks(int nvecs, struct irq_affinity *affd)
> * Spread on present CPUs starting from affd->pre_vectors. If we
> * have multiple sets, build each sets affinity mask separately.
> */
> - nr_sets = affd->nr_sets;
> - if (!nr_sets)
> - nr_sets = 1;
> + if (!affd->nr_sets) {
> + affd->nr_sets = 1;
> + affd->set_vectors[0] = affvecs;
> + }
>
> - for (i = 0, usedvecs = 0; i < nr_sets; i++) {
> - int this_vecs = affd->sets ? affd->sets[i] : affvecs;
> + for (i = 0, usedvecs = 0; i < affd->nr_sets; i++) {
> + int this_vecs = affd->set_vectors[i];
> int ret;
>
> ret = irq_build_affinity_masks(affd, curvec, this_vecs,
> @@ -316,7 +320,7 @@ int irq_calc_affinity_vectors(int minvec, int maxvec, const struct irq_affinity
> int i;
>
> for (i = 0, set_vecs = 0; i < affd->nr_sets; i++)
> - set_vecs += affd->sets[i];
> + set_vecs += affd->set_vectors[i];
> } else {
> get_online_cpus();
> set_vecs = cpumask_weight(cpu_possible_mask);
> --
> 2.9.5
>
WARNING: multiple messages have this Message-ID (diff)
From: helgaas@kernel.org (Bjorn Helgaas)
Subject: [PATCH V3 2/5] genirq/affinity: store irq set vectors in 'struct irq_affinity'
Date: Wed, 13 Feb 2019 09:07:24 -0600 [thread overview]
Message-ID: <20190213150723.GC96272@google.com> (raw)
In-Reply-To: <20190213105041.13537-3-ming.lei@redhat.com>
On Wed, Feb 13, 2019@06:50:38PM +0800, Ming Lei wrote:
> Currently the array of irq set vectors is provided by driver.
>
> irq_create_affinity_masks() can be simplied a bit by treating the
> non-irq-set case as single irq set.
s/simplied a bit/simplified/
> So move this array into 'struct irq_affinity', and pre-define the max
> set number as 4, which should be enough for normal cases.
s/irq/IRQ/
You have a real mixture of capitalization across the changelogs.
> Signed-off-by: Ming Lei <ming.lei at redhat.com>
> ---
> drivers/nvme/host/pci.c | 5 ++---
> include/linux/interrupt.h | 6 ++++--
> kernel/irq/affinity.c | 18 +++++++++++-------
> 3 files changed, 17 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index 022ea1ee63f8..0086bdf80ea1 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -2081,12 +2081,11 @@ static void nvme_calc_io_queues(struct nvme_dev *dev, unsigned int irq_queues)
> static int nvme_setup_irqs(struct nvme_dev *dev, unsigned int nr_io_queues)
> {
> struct pci_dev *pdev = to_pci_dev(dev->dev);
> - int irq_sets[2];
> struct irq_affinity affd = {
> .pre_vectors = 1,
> - .nr_sets = ARRAY_SIZE(irq_sets),
> - .sets = irq_sets,
> + .nr_sets = 2,
> };
> + int *irq_sets = affd.set_vectors;
> int result = 0;
> unsigned int irq_queues, this_p_queues;
>
> diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
> index 1ed1014c9684..a20150627a32 100644
> --- a/include/linux/interrupt.h
> +++ b/include/linux/interrupt.h
> @@ -259,6 +259,8 @@ struct irq_affinity_notify {
> void (*release)(struct kref *ref);
> };
>
> +#define IRQ_MAX_SETS 4
This is a pretty generic name. Maybe it should include a hint that it's
related to affinity?
> /**
> * struct irq_affinity - Description for automatic irq affinity assignements
> * @pre_vectors: Don't apply affinity to @pre_vectors at beginning of
> @@ -266,13 +268,13 @@ struct irq_affinity_notify {
> * @post_vectors: Don't apply affinity to @post_vectors at end of
> * the MSI(-X) vector space
> * @nr_sets: Length of passed in *sets array
> - * @sets: Number of affinitized sets
> + * @set_vectors: Number of affinitized sets
> */
> struct irq_affinity {
> int pre_vectors;
> int post_vectors;
> int nr_sets;
> - int *sets;
> + int set_vectors[IRQ_MAX_SETS];
> };
>
> /**
> diff --git a/kernel/irq/affinity.c b/kernel/irq/affinity.c
> index 9200d3b26f7d..b868b9d3df7f 100644
> --- a/kernel/irq/affinity.c
> +++ b/kernel/irq/affinity.c
> @@ -244,7 +244,7 @@ irq_create_affinity_masks(int nvecs, struct irq_affinity *affd)
> int affvecs = nvecs - affd->pre_vectors - affd->post_vectors;
> int curvec, usedvecs;
> struct irq_affinity_desc *masks = NULL;
> - int i, nr_sets;
> + int i;
>
> /*
> * If there aren't any vectors left after applying the pre/post
> @@ -253,6 +253,9 @@ irq_create_affinity_masks(int nvecs, struct irq_affinity *affd)
> if (nvecs == affd->pre_vectors + affd->post_vectors)
> return NULL;
>
> + if (affd->nr_sets > IRQ_MAX_SETS)
> + return NULL;
> +
> masks = kcalloc(nvecs, sizeof(*masks), GFP_KERNEL);
> if (!masks)
> return NULL;
> @@ -264,12 +267,13 @@ irq_create_affinity_masks(int nvecs, struct irq_affinity *affd)
> * Spread on present CPUs starting from affd->pre_vectors. If we
> * have multiple sets, build each sets affinity mask separately.
> */
> - nr_sets = affd->nr_sets;
> - if (!nr_sets)
> - nr_sets = 1;
> + if (!affd->nr_sets) {
> + affd->nr_sets = 1;
> + affd->set_vectors[0] = affvecs;
> + }
>
> - for (i = 0, usedvecs = 0; i < nr_sets; i++) {
> - int this_vecs = affd->sets ? affd->sets[i] : affvecs;
> + for (i = 0, usedvecs = 0; i < affd->nr_sets; i++) {
> + int this_vecs = affd->set_vectors[i];
> int ret;
>
> ret = irq_build_affinity_masks(affd, curvec, this_vecs,
> @@ -316,7 +320,7 @@ int irq_calc_affinity_vectors(int minvec, int maxvec, const struct irq_affinity
> int i;
>
> for (i = 0, set_vecs = 0; i < affd->nr_sets; i++)
> - set_vecs += affd->sets[i];
> + set_vecs += affd->set_vectors[i];
> } else {
> get_online_cpus();
> set_vecs = cpumask_weight(cpu_possible_mask);
> --
> 2.9.5
>
next prev parent reply other threads:[~2019-02-13 15:07 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-13 10:50 [PATCH V3 0/5] genirq/affinity: add .calc_sets for improving IRQ allocation & spread Ming Lei
2019-02-13 10:50 ` Ming Lei
2019-02-13 10:50 ` [PATCH V3 1/5] genirq/affinity: don't mark 'affd' as const Ming Lei
2019-02-13 10:50 ` Ming Lei
2019-02-13 15:04 ` Bjorn Helgaas
2019-02-13 15:04 ` Bjorn Helgaas
2019-02-13 20:56 ` Thomas Gleixner
2019-02-13 20:56 ` Thomas Gleixner
2019-02-13 21:31 ` Keith Busch
2019-02-13 21:31 ` Keith Busch
2019-02-13 21:41 ` Thomas Gleixner
2019-02-13 21:41 ` Thomas Gleixner
2019-02-13 22:37 ` Keith Busch
2019-02-13 22:37 ` Keith Busch
2019-02-14 8:50 ` Thomas Gleixner
2019-02-14 8:50 ` Thomas Gleixner
2019-02-14 13:04 ` 陈华才
2019-02-14 13:04 ` 陈华才
2019-02-14 13:31 ` Thomas Gleixner
2019-02-14 13:31 ` Thomas Gleixner
2019-02-19 0:42 ` 陈华才
2019-02-19 0:42 ` 陈华才
2019-02-19 6:19 ` Thomas Gleixner
2019-02-19 6:19 ` Thomas Gleixner
2019-02-19 16:12 ` Keith Busch
2019-02-19 16:12 ` Keith Busch
2019-02-13 10:50 ` [PATCH V3 2/5] genirq/affinity: store irq set vectors in 'struct irq_affinity' Ming Lei
2019-02-13 10:50 ` Ming Lei
2019-02-13 15:07 ` Bjorn Helgaas [this message]
2019-02-13 15:07 ` Bjorn Helgaas
2019-02-13 10:50 ` [PATCH V3 3/5] genirq/affinity: add new callback for caculating set vectors Ming Lei
2019-02-13 10:50 ` Ming Lei
2019-02-13 15:11 ` Bjorn Helgaas
2019-02-13 15:11 ` Bjorn Helgaas
2019-02-13 20:58 ` Thomas Gleixner
2019-02-13 20:58 ` Thomas Gleixner
2019-02-13 10:50 ` [PATCH V3 4/5] nvme-pci: avoid irq allocation retrying via .calc_sets Ming Lei
2019-02-13 10:50 ` Ming Lei
2019-02-13 15:13 ` Bjorn Helgaas
2019-02-13 15:13 ` Bjorn Helgaas
2019-02-13 21:26 ` Thomas Gleixner
2019-02-13 21:26 ` Thomas Gleixner
2019-02-13 10:50 ` [PATCH V3 5/5] genirq/affinity: Document .calc_sets as required in case of multiple sets Ming Lei
2019-02-13 10:50 ` Ming Lei
2019-02-13 15:16 ` Bjorn Helgaas
2019-02-13 15:16 ` Bjorn Helgaas
2019-02-13 14:36 ` [PATCH V3 0/5] genirq/affinity: add .calc_sets for improving IRQ allocation & spread Jens Axboe
2019-02-13 14:36 ` Jens Axboe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190213150723.GC96272@google.com \
--to=helgaas@kernel.org \
--cc=axboe@kernel.dk \
--cc=hch@lst.de \
--cc=keith.busch@intel.com \
--cc=linux-block@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-nvme@lists.infradead.org \
--cc=linux-pci@vger.kernel.org \
--cc=ming.lei@redhat.com \
--cc=sagi@grimberg.me \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.