From: Scott Wood <scottwood@freescale•com>
To: Benjamin Herrenschmidt <benh@kernel•crashing.org>
Cc: Laurentiu Tudor <Laurentiu.Tudor@freescale•com>,
linuxppc-dev@lists•ozlabs.org, Stuart Yoder <b08248@gmail•com>
Subject: Re: [PATCH] powerpc/booke64: Configurable lazy interrupt disabling
Date: Mon, 30 Jan 2012 17:13:07 -0600 [thread overview]
Message-ID: <4F272403.1020000@freescale.com> (raw)
In-Reply-To: <1327961726.28487.60.camel@pasglop>
On 01/30/2012 04:15 PM, Benjamin Herrenschmidt wrote:
> On Mon, 2012-01-30 at 15:47 -0600, Scott Wood wrote:
>
>> Only the first one will happen in a context where we want to store. The
>> issue is if we get another higher priority interrupt when we enable, and
>> that enables interrupts and we get the doorbell that wants to run the
>> saved irq. If we get priorities out of order we'll EOI the wrong interrupt.
>
> Hrm, ok, what about in handle_masked we just "save" it onto some kind of
> PACA local stack ? Then on enable, before actually turning EE back, we
> see if there's something there and we hit do_IRQ() if there is. Your
> get_irq() would preferrably pop things out of that little stack.
>
> Any hole in that scheme ?
If we never enable EE, there's no need for a stack -- we disable EE on
the first interrupt and can leave it in EPR. It's similar to my
original patch, but with the exception hack replaced with a call to
do_IRQ(). The quality of the regs you pass (if any) may suffer, which
is why I did the exception hack, but I can live with that if you can.
>> IIRC we now never enable interrupts while servicing one (are individual
>> handlers banned from doing this too?),
>
> No I think they still can.
OK. Another option could be to use the doorbell and store EPR
somewhere, but make sure if we get a real interrupt and there's a
pending interrupt stored, we clear it out and process both in proper
order. When the doorbell eventually fires it's a nop. Testing this
would require some effort, though. Better to stick with the simple
scheme where we never enable EE with a pending interrupt.
>>> However the main thing is that this significantly improves the quality
>>> of the samples obtained from performance interrupts which can now act as
>>> pseudo-NMI up to a certain point.
>>
>> Which is compensation for the hardware not doing it right with a proper
>> critical interrupt or equivalent, but yeah, that's a benefit.
>
> Right, server has no concept really of critical interrupts.
Would be nice if the embedded version used critical, though (or could be
configured to do so).
-Scott
next prev parent reply other threads:[~2012-01-30 23:13 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-01-18 14:35 [PATCH] powerpc/booke64: Configurable lazy interrupt disabling Laurentiu Tudor
2012-01-18 21:10 ` Benjamin Herrenschmidt
2012-01-19 13:18 ` Tudor Laurentiu
2012-01-19 19:21 ` Stuart Yoder
2012-01-19 19:29 ` Stuart Yoder
2012-01-20 23:05 ` Benjamin Herrenschmidt
2012-01-23 19:21 ` Scott Wood
2012-01-23 20:50 ` Benjamin Herrenschmidt
2012-01-25 14:32 ` Tudor Laurentiu
2012-01-30 21:47 ` Scott Wood
2012-01-30 22:15 ` Benjamin Herrenschmidt
2012-01-30 23:13 ` Scott Wood [this message]
2012-01-20 23:02 ` Benjamin Herrenschmidt
2012-01-23 19:31 ` Scott Wood
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=4F272403.1020000@freescale.com \
--to=scottwood@freescale$(echo .)com \
--cc=Laurentiu.Tudor@freescale$(echo .)com \
--cc=b08248@gmail$(echo .)com \
--cc=benh@kernel$(echo .)crashing.org \
--cc=linuxppc-dev@lists$(echo .)ozlabs.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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