linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: Ming Lei <ming.lei@redhat.com>
Cc: Christoph Hellwig <hch@lst.de>,
	Bjorn Helgaas <helgaas@kernel.org>, 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
Subject: Re: [PATCH 4/5] nvme-pci: simplify nvme_setup_irqs() via .setup_affinity callback
Date: Sun, 10 Feb 2019 17:39:20 +0100 (CET)	[thread overview]
Message-ID: <alpine.DEB.2.21.1902101731370.8784@nanos.tec.linutronix.de> (raw)
In-Reply-To: <20190125095347.17950-5-ming.lei@redhat.com>

On Fri, 25 Jan 2019, Ming Lei wrote:

> Use the callback of .setup_affinity() to re-caculate number
> of queues, and build irqs affinity with help of irq_build_affinity().
> 
> Then nvme_setup_irqs() gets simplified a lot.

I'm pretty sure you can achieve the same by reworking the core code without
that callback.
 
> +	/* Fill out vectors at the beginning that don't need affinity */
> +	for (curvec = 0; curvec < affd->pre_vectors; curvec++)
> +		cpumask_copy(&masks[curvec].mask, cpu_possible_mask);

cpu_possible_mask is wrong.  Why are you deliberately trying to make this
special? There is absolutely no reason to do so.

These interrupts are not managed and therefore the initial affinity has to
be irq_default_affinity. Setting them to cpu_possible_mask can and probably
will evade a well thought out default affinity mask, which was set to
isolate a set of cores from general purpose interrupts.

This is exactly the thing which happens with driver special stuff and which
needs to be avoided. There is nothing special about this NVME setup and
yes, I can see why the current core code is a bit tedious to work with, but
that does not justify that extra driver magic by any means.

Thanks,

	tglx


  reply	other threads:[~2019-02-10 16:39 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-25  9:53 [PATCH 0/5] genirq/affinity: introduce .setup_affinity to support allocating interrupt sets Ming Lei
2019-01-25  9:53 ` [PATCH 1/5] genirq/affinity: move allocation of 'node_to_cpumask' to irq_build_affinity_masks Ming Lei
2019-02-07 22:02   ` Bjorn Helgaas
2019-01-25  9:53 ` [PATCH 2/5] genirq/affinity: allow driver to setup managed IRQ's affinity Ming Lei
2019-02-07 22:21   ` Bjorn Helgaas
2019-02-10  9:22     ` Ming Lei
2019-02-10 16:30   ` Thomas Gleixner
2019-02-11  3:54     ` Ming Lei
2019-02-11 14:39       ` Bjorn Helgaas
2019-02-11 22:38         ` Thomas Gleixner
2019-02-12 11:17           ` Ming Lei
2019-01-25  9:53 ` [PATCH 3/5] genirq/affinity: introduce irq_build_affinity() Ming Lei
2019-01-25  9:53 ` [PATCH 4/5] nvme-pci: simplify nvme_setup_irqs() via .setup_affinity callback Ming Lei
2019-02-10 16:39   ` Thomas Gleixner [this message]
2019-02-11  3:58     ` Ming Lei
2019-02-10 18:49   ` Thomas Gleixner
2019-02-11  4:09     ` Ming Lei
2019-01-25  9:53 ` [PATCH 5/5] genirq/affinity: remove support for allocating interrupt sets Ming Lei
2019-02-07 22:22   ` Bjorn Helgaas
2019-01-25  9:56 ` [PATCH 0/5] genirq/affinity: introduce .setup_affinity to support " Ming Lei

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=alpine.DEB.2.21.1902101731370.8784@nanos.tec.linutronix.de \
    --to=tglx@linutronix.de \
    --cc=axboe@kernel.dk \
    --cc=hch@lst.de \
    --cc=helgaas@kernel.org \
    --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 \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).