From: Philippe Gerum <rpm@xenomai.org>
To: Hongzhan Chen <hongzhan.chen@intel.com>
Cc: xenomai@lists.linux.dev
Subject: Re: [PATCH] pipeline: irq: ack the irq when it is postponed inband irq from outside of the pipeline entry path.
Date: Fri, 15 Sep 2023 14:10:16 +0200 [thread overview]
Message-ID: <87sf7foaky.fsf@xenomai.org> (raw)
In-Reply-To: <20230907125623.12266-1-hongzhan.chen@intel.com>
Hongzhan Chen <hongzhan.chen@intel.com> writes:
> 1. To avoid double ack for the interrupt that already acked in
> pipeline entry path, we add new flag to note such status and
> differentiate.
> 2. For postponed stacked edge irq that is handling outside of the
> pipeline entry path, as discussed in [1], ack the irq directly
> because it would never be acked in the pipeline entry path.
>
> [1]: https://lore.kernel.org/xenomai/BY5PR11MB4276FEFA2C6D88C4B6CFFEFCF2E9A@BY5PR11MB4276.namprd11.prod.outlook.com/T/#t
>
> Signed-off-by: Hongzhan Chen <hongzhan.chen@intel.com>
>
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index 6aca6c036ada..b8d8aba38e17 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -862,15 +862,25 @@ void handle_edge_irq(struct irq_desc *desc)
> if (on_pipeline_entry()) {
> chip->irq_ack(&desc->irq_data);
> desc->istate |= IRQS_EDGE;
> - handle_oob_irq(desc);
> + if (!handle_oob_irq(desc)) {
> +
> + /* For inband irq, we tag acked status
> + * to avoid double ack
> + */
> + if (!oob_stage_present() || !irq_settings_is_oob(desc))
> + desc->istate |= IRQS_ACK;
This code could move to handle_oob_irq(), provided that we account for
this generic behavior in the x86-specific APIC flow handler.
> + }
> +
> goto out_unlock;
> }
>
> kstat_incr_irqs_this_cpu(desc);
>
> /* Start handling the irq */
> - if (!irqs_pipelined())
> + if (!(desc->istate & IRQS_ACK))
Disabling the IRQ pipeline should compile out any Dovetail-specific
code. In addition to ensuring zero overhead in this case, this is
important when chasing bugs as well, so that we can reliably compare
pipelined vs non-pipelined behavior with the very same kernel tree.
> chip->irq_ack(&desc->irq_data);
> + else
> + desc->istate &= ~IRQS_ACK;
Ok, this is going somewhere. I believe we need to generalize this
approach for all kinds of flow handlers, so that cascading any of them
would work, not only the edge one. There is also yet another tricky case
to care about when dealing with unmasked acked/eoi events, i.e.:
[IRQ-A] (acked, cascading, deferred handling to in-band stage)
|
+--> handler(A): demux IRQ-B (in-band)
...
synchronize_pipeline_on_irq()
hard irqs ON
[IRQ-A]
|
+--> handler(A): demux IRQ-B
...
synchronize_pipeline_on_irq()
hard irqs ON
flow_handler(B) (replay)
...
flow_handler(B) (replay)
...
and so on (a few intermediate steps were omitted). So we might have a
parent IRQ piling up prior to (re)playing the earlier one in-band with
hard irqs on, since we acked it early in the pipelining injection
path. IOW, the code which tracks the ack/eoi status for a cascaded event
from a stacked irqchip should be robust wrt this case too.
To achieve this, we could track the deferral status of irq events,
i.e. whether we expect them to be replayed in-band, instead of tracking
their acknowledge status on pipeline entry - this would disambiguate
some logic. We'd also need to track the state of the interrupt flow in a
more elaborate way than Dovetail currently implements. I'll follow up
with a patch illustrating this.
--
Philippe.
next prev parent reply other threads:[~2023-09-15 13:08 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-07 12:56 [PATCH] pipeline: irq: ack the irq when it is postponed inband irq from outside of the pipeline entry path Hongzhan Chen
2023-09-15 12:10 ` Philippe Gerum [this message]
-- strict thread matches above, loose matches on Subject: below --
2023-09-06 1:41 Hongzhan Chen
2023-09-07 13:25 ` Chen, Hongzhan
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=87sf7foaky.fsf@xenomai.org \
--to=rpm@xenomai.org \
--cc=hongzhan.chen@intel.com \
--cc=xenomai@lists.linux.dev \
/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 read-only IMAP folder(s) and NNTP newsgroup(s).