linux-rt-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: Grygorii Strashko <grygorii.strashko@ti.com>
Cc: bigeasy@linutronix.de, linux-rt-users@vger.kernel.org,
	linux-kernel@vger.kernel.org, Sekhar Nori <nsekhar@ti.com>
Subject: Re: [v4.1.10-rt10][PATCH 1/2] genirq: introduce new generic_handle_irq_rt_wa() api
Date: Tue, 3 Nov 2015 20:51:03 +0100 (CET)	[thread overview]
Message-ID: <alpine.DEB.2.11.1511032014580.4032@nanos> (raw)
In-Reply-To: <563906C5.8030801@ti.com>

On Tue, 3 Nov 2015, Grygorii Strashko wrote:
> On 11/02/2015 09:38 PM, Thomas Gleixner wrote:
> > 
> > Why aren't you simply marking these demultiplex handlers with IRQ_NO_THREAD?
> > 
> In general, it's possible. But, in this case, worst scenario will look like:

> dra7xx_pcie_msi_irq_handler()
> -> dw_handle_msi_irq()
>    [code simplified]
>    -> for (i = 0; i < MAX_MSI_IRQS; i++) {
> 	...
> 	generic_handle_irq(Y(i));
> 	...
>    }
> where MAX_MSI_IRQS = 32 now, but potentially can be increased up to 256.

And you really oversimplified the code above. The reality is:

    for (i = 0; i < MAX_MSI_CTRLS: i++) {
    	u32 status = read_msi_ctrl(i);

	for_each_bit(status)
		handle_irq();
    }

So sure, the worst case here is MAX_MSI_CTRLS * 32, but if all
possible 256 MSI interrupts are pending at the same time, you have
other problems than that.

In the current configuration (32 interrupts), which cannot change
because it's hardwired in silicon, this is a single status read and
assuming that only a few (most of the time it will be exactly ONE) of
those interrupts are pending at the same time is pretty much a sane
assumption. If it wouldn't be then all users of chained interrupt
handlers which usually demultiplex 32 interrupts would suffer from
that problem already.

Aside of that, you would prevent that any of these PCIe interrupts can
be utilized as a "fast" non threaded interrupt on RT. And that I would
consider a real bad limitation for no value. 

MSI has been invented to overcome the issues of wired interrupts
(demultiplexing and sharing), so I don't know why the involved
hardware designers came to the conclusion that demultiplexing MSI
interrupts in software is a sane approach. But then I really gave up
trying to understand hardware designers long ago.

The only sane way to deal with that is to actually mark those handlers
NOTRHEAD and document the limitations of your hardware, so your
customers won't trip over it. If they insist on having 32 MSI
producers on that PCIe bus and make them fire all at the same time,
then you still can provide them your "solution".

Just face it, it's a bad hardware design decision and adding a half
baken hackery which actually hurts sane use cases is not making it any
better.

Thanks,

	tglx






  reply	other threads:[~2015-11-03 19:51 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-02 19:30 [v4.1.10-rt10][PATCH 0/2] PCI: dra7xx/dwc: fix "WARNING: CPU: 1 PID: 82 at kernel/irq/handle.c:150 handle_irq_event_percpu" Grygorii Strashko
2015-11-02 19:30 ` [v4.1.10-rt10][PATCH 1/2] genirq: introduce new generic_handle_irq_rt_wa() api Grygorii Strashko
2015-11-02 19:38   ` Thomas Gleixner
2015-11-03 19:11     ` Grygorii Strashko
2015-11-03 19:51       ` Thomas Gleixner [this message]
2015-11-03 20:18         ` Sebastian Andrzej Siewior
2015-11-05 16:44           ` Grygorii Strashko
2015-11-02 19:30 ` [v4.1.10-rt10] [PATCH 2/2] PCI: dra7xx: fix "WARNING: CPU: 1 PID: 82 at kernel/irq/handle.c:150 handle_irq_event_percpu" Grygorii Strashko
2015-11-03 13:46 ` [v4.1.10-rt10][PATCH 0/2] PCI: dra7xx/dwc: " Sebastian Andrzej Siewior
2015-11-03 19:11   ` Grygorii Strashko
2015-11-03 20:01     ` Thomas Gleixner

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.11.1511032014580.4032@nanos \
    --to=tglx@linutronix.de \
    --cc=bigeasy@linutronix.de \
    --cc=grygorii.strashko@ti.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rt-users@vger.kernel.org \
    --cc=nsekhar@ti.com \
    /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).