LKML Archive on lore.kernel.org
 help / color / Atom feed
From: John Garry <john.garry@huawei.com>
To: Marc Zyngier <maz@kernel.org>
Cc: Ming Lei <ming.lei@redhat.com>, <tglx@linutronix.de>,
	"chenxiang (M)" <chenxiang66@hisilicon.com>,
	<bigeasy@linutronix.de>, <linux-kernel@vger.kernel.org>,
	<hare@suse.com>, <hch@lst.de>, <axboe@kernel.dk>,
	<bvanassche@acm.org>, <peterz@infradead.org>, <mingo@redhat.com>
Subject: Re: [PATCH RFC 1/1] genirq: Make threaded handler use irq affinity for managed interrupt
Date: Mon, 16 Dec 2019 14:17:41 +0000
Message-ID: <a5f6a542-2dbc-62de-52e2-bd5413b5db51@huawei.com> (raw)
In-Reply-To: <50d9ba606e1e3ee1665a0328ffac67ac@www.loen.fr>

Hi Marc,

>>
>> I'm just wondering if non-managed interrupts should be included in
>> the load balancing calculation? Couldn't irqbalance (if active) start
>> moving non-managed interrupts around anyway?
> 
> But they are, aren't they? See what we do in irq_set_affinity:
> 
> +        atomic_inc(per_cpu_ptr(&cpu_lpi_count, cpu));
> +        atomic_dec(per_cpu_ptr(&cpu_lpi_count,
> +                       its_dev->event_map.col_map[id]));
> 
> We don't try to "rebalance" anything based on that though, not that
> I think we should.

Ah sorry, I meant whether they should not be included. In 
its_irq_domain_activate(), we increment the per-cpu lpi count and also 
use its_pick_target_cpu() to find the least loaded cpu. I am asking 
whether we should just stick with the old policy for non-managed 
interrupts here.

After checking D05, I see a very significant performance hit for SAS 
controller performance - ~40% throughout lowering.

With this patch, now we have effective affinity targeted at seemingly 
"random" CPUs, as opposed to all just using CPU0. This affects performance.

The difference is that when we use managed interrupts - like for NVME or 
D06 SAS controller - the irq cpu affinity mask matches the CPUs which 
enqueue the requests to the queue associated with the interrupt. So 
there is an efficiency is enqueuing and deqeueing on same CPU group - 
all related to blk multi-queue. And this is not the case for non-managed 
interrupts.

>>
>>> Please give this new patch a shot on your system (my D05 doesn't have
>>> any managed devices):
>>
>> We could consider supporting platform msi managed interrupts, but I
>> doubt the value.
> 
> It shouldn't be hard to do, and most of the existing code could be
> moved to the generic level. As for the value, I'm not convinced
> either. For example D05 uses the MBIGEN as an intermediate interrupt
> controller, so MSIs are from the PoV of MBIGEN, and not the SAS device
> attached to it. Not the best design...

JFYI, I did raise this following topic before, but that's as far as I got:

https://marc.info/?l=linux-block&m=150722088314310&w=2

> 
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/commit/?h=irq/its-balance-mappings&id=1e987d83b8d880d56c9a2d8a86289631da94e55a 
>>>
>>>
>>
>> I quickly tested that in my NVMe env, and I see a performance boost
>> of 1055K -> 1206K IOPS. Results at bottom.
> 
> OK, that's encouraging.
> 
>> Here's the irq mapping dump:
> 
> [...]
> 
> Looks good.
> 
>> I'm still getting the CPU lockup (even on CPUs which have a single
>> NVMe completion interrupt assigned), which taints these results. That
>> lockup needs to be fixed.
> 
> Is this interrupt screaming to the point where it prevents the completion
> thread from making forward progress? What if you don't use threaded
> interrupts?

Yeah, just switching to threaded interrupts solves it (nvme core has a 
switch for this). So there was a big discussion on this topic a while ago:

https://lkml.org/lkml/2019/8/20/45 (couldn't find this on lore)

The conclusion there was to switch to irq poll, but leiming though that 
it was another issue - see earlier mail:

https://lore.kernel.org/lkml/20191210014335.GA25022@ming.t460p/

> 
>> We'll check on our SAS env also. I did already hack something up
>> similar to your change and again we saw a boost there.
> 
> OK. Please keep me posted. If the result is overall positive, I'll
> push this into -next for some soaking.
> 

ok, thanks

John

  reply index

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-06 14:35 [PATCH RFC 0/1] Threaded handler uses irq affinity for when the interrupt is managed John Garry
2019-12-06 14:35 ` [PATCH RFC 1/1] genirq: Make threaded handler use irq affinity for managed interrupt John Garry
2019-12-06 15:22   ` Marc Zyngier
2019-12-06 16:16     ` John Garry
2019-12-07  8:03   ` Ming Lei
2019-12-09 14:30     ` John Garry
2019-12-09 15:09       ` Hannes Reinecke
2019-12-09 15:17         ` Marc Zyngier
2019-12-09 15:25           ` Hannes Reinecke
2019-12-09 15:36             ` Marc Zyngier
2019-12-09 15:49           ` Qais Yousef
2019-12-09 15:55             ` Marc Zyngier
2019-12-10  1:43       ` Ming Lei
2019-12-10  9:45         ` John Garry
2019-12-10 10:06           ` Ming Lei
2019-12-10 10:28           ` Marc Zyngier
2019-12-10 10:59             ` John Garry
2019-12-10 11:36               ` Marc Zyngier
2019-12-10 12:05                 ` John Garry
2019-12-10 18:32                   ` Marc Zyngier
2019-12-11  9:41                     ` John Garry
2019-12-13 10:07                       ` John Garry
2019-12-13 10:31                         ` Marc Zyngier
2019-12-13 12:08                           ` John Garry
2019-12-14 10:59                             ` Marc Zyngier
2019-12-11 17:09         ` John Garry
2019-12-12 22:38           ` Ming Lei
2019-12-13 11:12             ` John Garry
2019-12-13 13:18               ` Ming Lei
2019-12-13 15:43                 ` John Garry
2019-12-13 17:12                   ` Ming Lei
2019-12-13 17:50                     ` John Garry
2019-12-14 13:56                   ` Marc Zyngier
2019-12-16 10:47                     ` John Garry
2019-12-16 11:40                       ` Marc Zyngier
2019-12-16 14:17                         ` John Garry [this message]
2019-12-16 18:00                           ` Marc Zyngier
2019-12-16 18:50                             ` John Garry
2019-12-20 11:30                               ` John Garry
2019-12-20 14:43                                 ` Marc Zyngier
2019-12-20 15:38                                   ` John Garry
2019-12-20 16:16                                     ` Marc Zyngier
2019-12-20 23:31                                     ` Ming Lei
2019-12-23  9:07                                       ` Marc Zyngier
2019-12-23 10:26                                         ` John Garry
2019-12-23 10:47                                           ` Marc Zyngier
2019-12-23 11:35                                             ` John Garry
2019-12-24  1:59                                             ` Ming Lei
2019-12-24 11:20                                               ` Marc Zyngier
2019-12-25  0:48                                                 ` Ming Lei
2020-01-02 10:35                                                   ` John Garry
2020-01-03  0:46                                                     ` Ming Lei
2020-01-03 10:41                                                       ` John Garry
2020-01-03 11:29                                                         ` Ming Lei
2020-01-03 11:50                                                           ` John Garry
2020-01-04 12:03                                                             ` Ming Lei
2020-05-30  7:46 ` [tip: irq/core] irqchip/gic-v3-its: Balance initial LPI affinity across CPUs tip-bot2 for Marc Zyngier

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=a5f6a542-2dbc-62de-52e2-bd5413b5db51@huawei.com \
    --to=john.garry@huawei.com \
    --cc=axboe@kernel.dk \
    --cc=bigeasy@linutronix.de \
    --cc=bvanassche@acm.org \
    --cc=chenxiang66@hisilicon.com \
    --cc=hare@suse.com \
    --cc=hch@lst.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maz@kernel.org \
    --cc=ming.lei@redhat.com \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --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

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git
	git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git