* [PATCH] cobalt/pipe: Fix invalid wait context in xnpipe_wakeup_proc()
@ 2025-08-29 15:59 Florian Bezdeka
2025-09-01 13:51 ` Jan Kiszka
2025-09-02 15:40 ` Jan Kiszka
0 siblings, 2 replies; 8+ messages in thread
From: Florian Bezdeka @ 2025-08-29 15:59 UTC (permalink / raw)
To: xenomai; +Cc: Florian Bezdeka, Jan Kiszka, Philippe Gerum
Recent Dovetail versions complained about an invalid wait context in
xnpipe_wakeup_proc(). It turned out that this warning is correct as this
function is called from the inband IRQ context (while syncing the IRQ
log) with hard IRQs enabled and inband stage stalled.
The call graph looks like that:
xnpipe_wakeup_proc()
wake_up_interruptible()
__wake_up_common_lock()
spin_lock_irqsave() <-- might sleep on PREEMPT_RT
As confirmed by Philippe in the discussion linked below, we have to
migrate the wakeup code from using a synthetic IRQ to irq_work to address
that issue.
The log splat report:
[ 151.574032]
[ 151.574039] =============================
[ 151.574043] [ BUG: Invalid wait context ]
[ 151.574046] 6.15.0 #1 Not tainted
[ 151.574048] -----------------------------
[ 151.574048] swapper/0/0 is trying to lock:
[ 151.574050] ffffffff841174c0 (&state->readq){....}-{3:3}, at: __wake_up+0x21/0x60
[ 151.574063] other info that might help us debug this:
[ 151.574064] context-{2:2}
[ 151.574065] no locks held by swapper/0/0.
[ 151.574066] stack backtrace:
[ 151.574073] CPU: 0 UID: 0 PID: 0 Comm: swapper/0 Not tainted 6.15.0 #1 PREEMPT(full)
[ 151.574078] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2~bpo12+1 04/01/2014
[ 151.574079] IRQ stage: Linux
[ 151.574083] Call Trace:
[ 151.574086] <IRQ>
[ 151.574088] dump_stack_lvl+0x79/0xe0
[ 151.574095] __lock_acquire+0x942/0xbf0
[ 151.574104] lock_acquire+0xe2/0x2f0
[ 151.574107] ? __wake_up+0x21/0x60
[ 151.574111] ? find_held_lock+0x2b/0x80
[ 151.574115] _raw_spin_lock_irqsave+0x49/0x60
[ 151.574120] ? __wake_up+0x21/0x60
[ 151.574122] __wake_up+0x21/0x60
[ 151.574125] xnpipe_wakeup_proc+0x152/0x590
[ 151.574132] handle_synthetic_irq+0xc2/0x250
[ 151.574137] arch_do_IRQ_pipelined+0xca/0x180
[ 151.574141] </IRQ>
[ 151.574142] <TASK>
[ 151.574144] sync_current_irq_stage+0xaa/0x110
[ 151.574147] inband_irq_enable+0x42/0x60
[ 151.574151] cpuidle_idle_call+0x17d/0x200
[ 151.574155] do_idle+0x89/0xd0
[ 151.574158] cpu_startup_entry+0x29/0x30
[ 151.574160] rest_init+0xf0/0x190
[ 151.574164] start_kernel+0x632/0x700
[ 151.574179] x86_64_start_reservations+0x18/0x30
[ 151.574185] x86_64_start_kernel+0x78/0x80
[ 151.574188] common_startup_64+0x13e/0x148
[ 151.574198] </TASK>
Link: https://lore.kernel.org/xenomai/87349may4v.fsf@xenomai.org/T/#t
Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
---
Cc: Jan Kiszka <jan.kiszka@siemens.com>
Cc: Philippe Gerum <rpm@xenomai.org>
---
kernel/cobalt/pipe.c | 19 +++++--------------
1 file changed, 5 insertions(+), 14 deletions(-)
diff --git a/kernel/cobalt/pipe.c b/kernel/cobalt/pipe.c
index 7519f3165..5ab561384 100644
--- a/kernel/cobalt/pipe.c
+++ b/kernel/cobalt/pipe.c
@@ -48,7 +48,7 @@ static LIST_HEAD(xnpipe_sleepq);
static LIST_HEAD(xnpipe_asyncq);
-static int xnpipe_wakeup_virq;
+static struct irq_work xnpipe_wakeup_work;
static struct class *xnpipe_class;
@@ -147,7 +147,7 @@ static inline void xnpipe_dequeue_all(struct xnpipe_state *state, int mask)
__sigpending; \
})
-static irqreturn_t xnpipe_wakeup_proc(int sirq, void *dev_id)
+static void xnpipe_wakeup_proc(struct irq_work *work)
{
struct xnpipe_state *state;
unsigned long rbits;
@@ -221,13 +221,11 @@ check_async:
}
out:
xnlock_put_irqrestore(&nklock, s);
-
- return IRQ_HANDLED;
}
static inline void xnpipe_schedule_request(void) /* hw IRQs off */
{
- pipeline_post_sirq(xnpipe_wakeup_virq);
+ irq_work_queue(&xnpipe_wakeup_work);
}
static inline ssize_t xnpipe_flush_bufq(void (*fn)(void *buf, void *xstate),
@@ -1149,6 +1147,8 @@ int xnpipe_mount(void)
state->nroutq = 0;
}
+ init_irq_work(&xnpipe_wakeup_work, xnpipe_wakeup_proc);
+
#if LINUX_VERSION_CODE >= KERNEL_VERSION(6,4,0)
xnpipe_class = class_create("rtpipe");
#else
@@ -1180,13 +1180,6 @@ int xnpipe_mount(void)
return -EBUSY;
}
- xnpipe_wakeup_virq = pipeline_create_inband_sirq(xnpipe_wakeup_proc);
- if (xnpipe_wakeup_virq < 0) {
- printk(XENO_ERR
- "unable to reserve synthetic IRQ for message pipes\n");
- return xnpipe_wakeup_virq;
- }
-
return 0;
}
@@ -1194,8 +1187,6 @@ void xnpipe_umount(void)
{
int i;
- pipeline_delete_inband_sirq(xnpipe_wakeup_virq);
-
unregister_chrdev(XNPIPE_DEV_MAJOR, "rtpipe");
for (i = 0; i < XNPIPE_NDEVS; i++)
---
base-commit: ccdceccd2508596ebd114981890b5b47b650058a
change-id: 20250829-wip-flo-migrate-cobalt-pipe-to-irq-work-359bc1f0a8de
Best regards,
--
Florian Bezdeka <florian.bezdeka@siemens.com>
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH] cobalt/pipe: Fix invalid wait context in xnpipe_wakeup_proc()
2025-08-29 15:59 [PATCH] cobalt/pipe: Fix invalid wait context in xnpipe_wakeup_proc() Florian Bezdeka
@ 2025-09-01 13:51 ` Jan Kiszka
2025-09-01 15:28 ` Philippe Gerum
2025-09-02 21:15 ` Florian Bezdeka
2025-09-02 15:40 ` Jan Kiszka
1 sibling, 2 replies; 8+ messages in thread
From: Jan Kiszka @ 2025-09-01 13:51 UTC (permalink / raw)
To: Florian Bezdeka, xenomai; +Cc: Philippe Gerum
On 29.08.25 17:59, Florian Bezdeka wrote:
> Recent Dovetail versions complained about an invalid wait context in
> xnpipe_wakeup_proc(). It turned out that this warning is correct as this
> function is called from the inband IRQ context (while syncing the IRQ
> log) with hard IRQs enabled and inband stage stalled.
>
> The call graph looks like that:
> xnpipe_wakeup_proc()
> wake_up_interruptible()
> __wake_up_common_lock()
> spin_lock_irqsave() <-- might sleep on PREEMPT_RT
>
So, the warning is issued independent of PREEMPT_RT being enabled but
the bug would be limited to that configuration, right?
Since when is the kernel complaining? It is wrong since PREEMPT_RT is
available, officially 6.12 then. Then this should go into 3.3-stable as
well - noted.
> As confirmed by Philippe in the discussion linked below, we have to
> migrate the wakeup code from using a synthetic IRQ to irq_work to address
> that issue.
>
Hmm, can't we use something that is a thread when needed and an IRQ
otherwise?
> The log splat report:
> [ 151.574032]
> [ 151.574039] =============================
> [ 151.574043] [ BUG: Invalid wait context ]
> [ 151.574046] 6.15.0 #1 Not tainted
> [ 151.574048] -----------------------------
> [ 151.574048] swapper/0/0 is trying to lock:
> [ 151.574050] ffffffff841174c0 (&state->readq){....}-{3:3}, at: __wake_up+0x21/0x60
> [ 151.574063] other info that might help us debug this:
> [ 151.574064] context-{2:2}
> [ 151.574065] no locks held by swapper/0/0.
> [ 151.574066] stack backtrace:
> [ 151.574073] CPU: 0 UID: 0 PID: 0 Comm: swapper/0 Not tainted 6.15.0 #1 PREEMPT(full)
> [ 151.574078] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2~bpo12+1 04/01/2014
> [ 151.574079] IRQ stage: Linux
> [ 151.574083] Call Trace:
> [ 151.574086] <IRQ>
> [ 151.574088] dump_stack_lvl+0x79/0xe0
> [ 151.574095] __lock_acquire+0x942/0xbf0
> [ 151.574104] lock_acquire+0xe2/0x2f0
> [ 151.574107] ? __wake_up+0x21/0x60
> [ 151.574111] ? find_held_lock+0x2b/0x80
> [ 151.574115] _raw_spin_lock_irqsave+0x49/0x60
> [ 151.574120] ? __wake_up+0x21/0x60
> [ 151.574122] __wake_up+0x21/0x60
> [ 151.574125] xnpipe_wakeup_proc+0x152/0x590
> [ 151.574132] handle_synthetic_irq+0xc2/0x250
> [ 151.574137] arch_do_IRQ_pipelined+0xca/0x180
> [ 151.574141] </IRQ>
> [ 151.574142] <TASK>
> [ 151.574144] sync_current_irq_stage+0xaa/0x110
> [ 151.574147] inband_irq_enable+0x42/0x60
> [ 151.574151] cpuidle_idle_call+0x17d/0x200
> [ 151.574155] do_idle+0x89/0xd0
> [ 151.574158] cpu_startup_entry+0x29/0x30
> [ 151.574160] rest_init+0xf0/0x190
> [ 151.574164] start_kernel+0x632/0x700
> [ 151.574179] x86_64_start_reservations+0x18/0x30
> [ 151.574185] x86_64_start_kernel+0x78/0x80
> [ 151.574188] common_startup_64+0x13e/0x148
> [ 151.574198] </TASK>
>
> Link: https://lore.kernel.org/xenomai/87349may4v.fsf@xenomai.org/T/#t
> Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
> ---
> Cc: Jan Kiszka <jan.kiszka@siemens.com>
> Cc: Philippe Gerum <rpm@xenomai.org>
> ---
> kernel/cobalt/pipe.c | 19 +++++--------------
> 1 file changed, 5 insertions(+), 14 deletions(-)
>
> diff --git a/kernel/cobalt/pipe.c b/kernel/cobalt/pipe.c
> index 7519f3165..5ab561384 100644
> --- a/kernel/cobalt/pipe.c
> +++ b/kernel/cobalt/pipe.c
> @@ -48,7 +48,7 @@ static LIST_HEAD(xnpipe_sleepq);
>
> static LIST_HEAD(xnpipe_asyncq);
>
> -static int xnpipe_wakeup_virq;
> +static struct irq_work xnpipe_wakeup_work;
>
> static struct class *xnpipe_class;
>
> @@ -147,7 +147,7 @@ static inline void xnpipe_dequeue_all(struct xnpipe_state *state, int mask)
> __sigpending; \
> })
>
> -static irqreturn_t xnpipe_wakeup_proc(int sirq, void *dev_id)
> +static void xnpipe_wakeup_proc(struct irq_work *work)
> {
> struct xnpipe_state *state;
> unsigned long rbits;
> @@ -221,13 +221,11 @@ check_async:
> }
> out:
> xnlock_put_irqrestore(&nklock, s);
> -
> - return IRQ_HANDLED;
> }
>
> static inline void xnpipe_schedule_request(void) /* hw IRQs off */
> {
> - pipeline_post_sirq(xnpipe_wakeup_virq);
> + irq_work_queue(&xnpipe_wakeup_work);
> }
>
> static inline ssize_t xnpipe_flush_bufq(void (*fn)(void *buf, void *xstate),
> @@ -1149,6 +1147,8 @@ int xnpipe_mount(void)
> state->nroutq = 0;
> }
>
> + init_irq_work(&xnpipe_wakeup_work, xnpipe_wakeup_proc);
> +
> #if LINUX_VERSION_CODE >= KERNEL_VERSION(6,4,0)
> xnpipe_class = class_create("rtpipe");
> #else
> @@ -1180,13 +1180,6 @@ int xnpipe_mount(void)
> return -EBUSY;
> }
>
> - xnpipe_wakeup_virq = pipeline_create_inband_sirq(xnpipe_wakeup_proc);
> - if (xnpipe_wakeup_virq < 0) {
> - printk(XENO_ERR
> - "unable to reserve synthetic IRQ for message pipes\n");
> - return xnpipe_wakeup_virq;
> - }
> -
> return 0;
> }
>
> @@ -1194,8 +1187,6 @@ void xnpipe_umount(void)
> {
> int i;
>
> - pipeline_delete_inband_sirq(xnpipe_wakeup_virq);
> -
> unregister_chrdev(XNPIPE_DEV_MAJOR, "rtpipe");
>
> for (i = 0; i < XNPIPE_NDEVS; i++)
>
I was briefly checking if we have more inband sirq issues, but the other
two users, registry and xnselect, lock different and unaffected.
But I'm still wondering if we need both irq_work and inband_sirq. And
how to shorten the number of steps to the actually required minimum. No
real conclusion yet, but thoughts on how to either obsolete one in favor
of the other or how to avoid that we take another thread wakeup when not
needed are welcome.
Jan
--
Siemens AG, Foundational Technologies
Linux Expert Center
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] cobalt/pipe: Fix invalid wait context in xnpipe_wakeup_proc()
2025-09-01 13:51 ` Jan Kiszka
@ 2025-09-01 15:28 ` Philippe Gerum
2025-09-01 16:49 ` Jan Kiszka
2025-09-02 21:15 ` Florian Bezdeka
1 sibling, 1 reply; 8+ messages in thread
From: Philippe Gerum @ 2025-09-01 15:28 UTC (permalink / raw)
To: Jan Kiszka; +Cc: Florian Bezdeka, xenomai
Jan Kiszka <jan.kiszka@siemens.com> writes:
> On 29.08.25 17:59, Florian Bezdeka wrote:
>> Recent Dovetail versions complained about an invalid wait context in
>> xnpipe_wakeup_proc(). It turned out that this warning is correct as this
>> function is called from the inband IRQ context (while syncing the IRQ
>> log) with hard IRQs enabled and inband stage stalled.
>>
>> The call graph looks like that:
>> xnpipe_wakeup_proc()
>> wake_up_interruptible()
>> __wake_up_common_lock()
>> spin_lock_irqsave() <-- might sleep on PREEMPT_RT
>>
>
> So, the warning is issued independent of PREEMPT_RT being enabled but
> the bug would be limited to that configuration, right?
>
> Since when is the kernel complaining? It is wrong since PREEMPT_RT is
> available, officially 6.12 then. Then this should go into 3.3-stable as
> well - noted.
>
>> As confirmed by Philippe in the discussion linked below, we have to
>> migrate the wakeup code from using a synthetic IRQ to irq_work to address
>> that issue.
>>
>
> Hmm, can't we use something that is a thread when needed and an IRQ
> otherwise?
>
Unless PREEMPT_RT is enabled, we can't guarantee threaded handling, but
we can always guarantee irq-based handling by setting IRQ_WORK_HARD_IRQ
for the irq_work struct.
--
Philippe.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] cobalt/pipe: Fix invalid wait context in xnpipe_wakeup_proc()
2025-09-01 15:28 ` Philippe Gerum
@ 2025-09-01 16:49 ` Jan Kiszka
2025-09-02 9:13 ` Philippe Gerum
0 siblings, 1 reply; 8+ messages in thread
From: Jan Kiszka @ 2025-09-01 16:49 UTC (permalink / raw)
To: Philippe Gerum; +Cc: Florian Bezdeka, xenomai
On 01.09.25 17:28, Philippe Gerum wrote:
> Jan Kiszka <jan.kiszka@siemens.com> writes:
>
>> On 29.08.25 17:59, Florian Bezdeka wrote:
>>> Recent Dovetail versions complained about an invalid wait context in
>>> xnpipe_wakeup_proc(). It turned out that this warning is correct as this
>>> function is called from the inband IRQ context (while syncing the IRQ
>>> log) with hard IRQs enabled and inband stage stalled.
>>>
>>> The call graph looks like that:
>>> xnpipe_wakeup_proc()
>>> wake_up_interruptible()
>>> __wake_up_common_lock()
>>> spin_lock_irqsave() <-- might sleep on PREEMPT_RT
>>>
>>
>> So, the warning is issued independent of PREEMPT_RT being enabled but
>> the bug would be limited to that configuration, right?
>>
>> Since when is the kernel complaining? It is wrong since PREEMPT_RT is
>> available, officially 6.12 then. Then this should go into 3.3-stable as
>> well - noted.
>>
>>> As confirmed by Philippe in the discussion linked below, we have to
>>> migrate the wakeup code from using a synthetic IRQ to irq_work to address
>>> that issue.
>>>
>>
>> Hmm, can't we use something that is a thread when needed and an IRQ
>> otherwise?
>>
>
> Unless PREEMPT_RT is enabled, we can't guarantee threaded handling, but
> we can always guarantee irq-based handling by setting IRQ_WORK_HARD_IRQ
> for the irq_work struct.
>
This was not what I was thinking of. To my understanding, we need
threading here ONLY if PREEMPT_RT is enabled because only then __wake_up
can be sleepy. With the conversion to irq_work, we would go via an extra
kernel thread even when not needed.
Jan
--
Siemens AG, Foundational Technologies
Linux Expert Center
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] cobalt/pipe: Fix invalid wait context in xnpipe_wakeup_proc()
2025-09-01 16:49 ` Jan Kiszka
@ 2025-09-02 9:13 ` Philippe Gerum
2025-09-02 15:38 ` Jan Kiszka
0 siblings, 1 reply; 8+ messages in thread
From: Philippe Gerum @ 2025-09-02 9:13 UTC (permalink / raw)
To: Jan Kiszka; +Cc: Philippe Gerum, Florian Bezdeka, xenomai
Jan Kiszka <jan.kiszka@siemens.com> writes:
> On 01.09.25 17:28, Philippe Gerum wrote:
>> Jan Kiszka <jan.kiszka@siemens.com> writes:
>>
>>> On 29.08.25 17:59, Florian Bezdeka wrote:
>>>> Recent Dovetail versions complained about an invalid wait context in
>>>> xnpipe_wakeup_proc(). It turned out that this warning is correct as this
>>>> function is called from the inband IRQ context (while syncing the IRQ
>>>> log) with hard IRQs enabled and inband stage stalled.
>>>>
>>>> The call graph looks like that:
>>>> xnpipe_wakeup_proc()
>>>> wake_up_interruptible()
>>>> __wake_up_common_lock()
>>>> spin_lock_irqsave() <-- might sleep on PREEMPT_RT
>>>>
>>>
>>> So, the warning is issued independent of PREEMPT_RT being enabled but
>>> the bug would be limited to that configuration, right?
>>>
>>> Since when is the kernel complaining? It is wrong since PREEMPT_RT is
>>> available, officially 6.12 then. Then this should go into 3.3-stable as
>>> well - noted.
>>>
>>>> As confirmed by Philippe in the discussion linked below, we have to
>>>> migrate the wakeup code from using a synthetic IRQ to irq_work to address
>>>> that issue.
>>>>
>>>
>>> Hmm, can't we use something that is a thread when needed and an IRQ
>>> otherwise?
>>>
>>
>> Unless PREEMPT_RT is enabled, we can't guarantee threaded handling, but
>> we can always guarantee irq-based handling by setting IRQ_WORK_HARD_IRQ
>> for the irq_work struct.
>>
>
> This was not what I was thinking of. To my understanding, we need
> threading here ONLY if PREEMPT_RT is enabled because only then __wake_up
> can be sleepy. With the conversion to irq_work, we would go via an extra
> kernel thread even when not needed.
If PREEMPT_RT is disabled, irq_work does run on top of a synthetic irq,
no thread involved.
--
Philippe.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] cobalt/pipe: Fix invalid wait context in xnpipe_wakeup_proc()
2025-09-02 9:13 ` Philippe Gerum
@ 2025-09-02 15:38 ` Jan Kiszka
0 siblings, 0 replies; 8+ messages in thread
From: Jan Kiszka @ 2025-09-02 15:38 UTC (permalink / raw)
To: Philippe Gerum; +Cc: Florian Bezdeka, xenomai
On 02.09.25 11:13, Philippe Gerum wrote:
> Jan Kiszka <jan.kiszka@siemens.com> writes:
>
>> On 01.09.25 17:28, Philippe Gerum wrote:
>>> Jan Kiszka <jan.kiszka@siemens.com> writes:
>>>
>>>> On 29.08.25 17:59, Florian Bezdeka wrote:
>>>>> Recent Dovetail versions complained about an invalid wait context in
>>>>> xnpipe_wakeup_proc(). It turned out that this warning is correct as this
>>>>> function is called from the inband IRQ context (while syncing the IRQ
>>>>> log) with hard IRQs enabled and inband stage stalled.
>>>>>
>>>>> The call graph looks like that:
>>>>> xnpipe_wakeup_proc()
>>>>> wake_up_interruptible()
>>>>> __wake_up_common_lock()
>>>>> spin_lock_irqsave() <-- might sleep on PREEMPT_RT
>>>>>
>>>>
>>>> So, the warning is issued independent of PREEMPT_RT being enabled but
>>>> the bug would be limited to that configuration, right?
>>>>
>>>> Since when is the kernel complaining? It is wrong since PREEMPT_RT is
>>>> available, officially 6.12 then. Then this should go into 3.3-stable as
>>>> well - noted.
>>>>
>>>>> As confirmed by Philippe in the discussion linked below, we have to
>>>>> migrate the wakeup code from using a synthetic IRQ to irq_work to address
>>>>> that issue.
>>>>>
>>>>
>>>> Hmm, can't we use something that is a thread when needed and an IRQ
>>>> otherwise?
>>>>
>>>
>>> Unless PREEMPT_RT is enabled, we can't guarantee threaded handling, but
>>> we can always guarantee irq-based handling by setting IRQ_WORK_HARD_IRQ
>>> for the irq_work struct.
>>>
>>
>> This was not what I was thinking of. To my understanding, we need
>> threading here ONLY if PREEMPT_RT is enabled because only then __wake_up
>> can be sleepy. With the conversion to irq_work, we would go via an extra
>> kernel thread even when not needed.
>
> If PREEMPT_RT is disabled, irq_work does run on top of a synthetic irq,
> no thread involved.
Perfect, thanks for clarifying.
Jan
--
Siemens AG, Foundational Technologies
Linux Expert Center
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] cobalt/pipe: Fix invalid wait context in xnpipe_wakeup_proc()
2025-09-01 13:51 ` Jan Kiszka
2025-09-01 15:28 ` Philippe Gerum
@ 2025-09-02 21:15 ` Florian Bezdeka
1 sibling, 0 replies; 8+ messages in thread
From: Florian Bezdeka @ 2025-09-02 21:15 UTC (permalink / raw)
To: Jan Kiszka, xenomai; +Cc: Philippe Gerum
On Mon, 2025-09-01 at 15:51 +0200, Jan Kiszka wrote:
> I was briefly checking if we have more inband sirq issues, but the other
> two users, registry and xnselect, lock different and unaffected.
I checked other x3 users of sirqs last week as well and came to the
same conclusion.
>
> But I'm still wondering if we need both irq_work and inband_sirq. And
> how to shorten the number of steps to the actually required minimum. No
> real conclusion yet, but thoughts on how to either obsolete one in favor
> of the other or how to avoid that we take another thread wakeup when not
> needed are welcome.
Seems I'm too late. Philippe already clarified this.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] cobalt/pipe: Fix invalid wait context in xnpipe_wakeup_proc()
2025-08-29 15:59 [PATCH] cobalt/pipe: Fix invalid wait context in xnpipe_wakeup_proc() Florian Bezdeka
2025-09-01 13:51 ` Jan Kiszka
@ 2025-09-02 15:40 ` Jan Kiszka
1 sibling, 0 replies; 8+ messages in thread
From: Jan Kiszka @ 2025-09-02 15:40 UTC (permalink / raw)
To: Florian Bezdeka, xenomai; +Cc: Philippe Gerum
On 29.08.25 17:59, Florian Bezdeka wrote:
> Recent Dovetail versions complained about an invalid wait context in
> xnpipe_wakeup_proc(). It turned out that this warning is correct as this
> function is called from the inband IRQ context (while syncing the IRQ
> log) with hard IRQs enabled and inband stage stalled.
>
> The call graph looks like that:
> xnpipe_wakeup_proc()
> wake_up_interruptible()
> __wake_up_common_lock()
> spin_lock_irqsave() <-- might sleep on PREEMPT_RT
>
> As confirmed by Philippe in the discussion linked below, we have to
> migrate the wakeup code from using a synthetic IRQ to irq_work to address
> that issue.
>
> The log splat report:
> [ 151.574032]
> [ 151.574039] =============================
> [ 151.574043] [ BUG: Invalid wait context ]
> [ 151.574046] 6.15.0 #1 Not tainted
> [ 151.574048] -----------------------------
> [ 151.574048] swapper/0/0 is trying to lock:
> [ 151.574050] ffffffff841174c0 (&state->readq){....}-{3:3}, at: __wake_up+0x21/0x60
> [ 151.574063] other info that might help us debug this:
> [ 151.574064] context-{2:2}
> [ 151.574065] no locks held by swapper/0/0.
> [ 151.574066] stack backtrace:
> [ 151.574073] CPU: 0 UID: 0 PID: 0 Comm: swapper/0 Not tainted 6.15.0 #1 PREEMPT(full)
> [ 151.574078] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2~bpo12+1 04/01/2014
> [ 151.574079] IRQ stage: Linux
> [ 151.574083] Call Trace:
> [ 151.574086] <IRQ>
> [ 151.574088] dump_stack_lvl+0x79/0xe0
> [ 151.574095] __lock_acquire+0x942/0xbf0
> [ 151.574104] lock_acquire+0xe2/0x2f0
> [ 151.574107] ? __wake_up+0x21/0x60
> [ 151.574111] ? find_held_lock+0x2b/0x80
> [ 151.574115] _raw_spin_lock_irqsave+0x49/0x60
> [ 151.574120] ? __wake_up+0x21/0x60
> [ 151.574122] __wake_up+0x21/0x60
> [ 151.574125] xnpipe_wakeup_proc+0x152/0x590
> [ 151.574132] handle_synthetic_irq+0xc2/0x250
> [ 151.574137] arch_do_IRQ_pipelined+0xca/0x180
> [ 151.574141] </IRQ>
> [ 151.574142] <TASK>
> [ 151.574144] sync_current_irq_stage+0xaa/0x110
> [ 151.574147] inband_irq_enable+0x42/0x60
> [ 151.574151] cpuidle_idle_call+0x17d/0x200
> [ 151.574155] do_idle+0x89/0xd0
> [ 151.574158] cpu_startup_entry+0x29/0x30
> [ 151.574160] rest_init+0xf0/0x190
> [ 151.574164] start_kernel+0x632/0x700
> [ 151.574179] x86_64_start_reservations+0x18/0x30
> [ 151.574185] x86_64_start_kernel+0x78/0x80
> [ 151.574188] common_startup_64+0x13e/0x148
> [ 151.574198] </TASK>
>
> Link: https://lore.kernel.org/xenomai/87349may4v.fsf@xenomai.org/T/#t
> Signed-off-by: Florian Bezdeka <florian.bezdeka@siemens.com>
> ---
> Cc: Jan Kiszka <jan.kiszka@siemens.com>
> Cc: Philippe Gerum <rpm@xenomai.org>
> ---
> kernel/cobalt/pipe.c | 19 +++++--------------
> 1 file changed, 5 insertions(+), 14 deletions(-)
>
> diff --git a/kernel/cobalt/pipe.c b/kernel/cobalt/pipe.c
> index 7519f3165..5ab561384 100644
> --- a/kernel/cobalt/pipe.c
> +++ b/kernel/cobalt/pipe.c
> @@ -48,7 +48,7 @@ static LIST_HEAD(xnpipe_sleepq);
>
> static LIST_HEAD(xnpipe_asyncq);
>
> -static int xnpipe_wakeup_virq;
> +static struct irq_work xnpipe_wakeup_work;
>
> static struct class *xnpipe_class;
>
> @@ -147,7 +147,7 @@ static inline void xnpipe_dequeue_all(struct xnpipe_state *state, int mask)
> __sigpending; \
> })
>
> -static irqreturn_t xnpipe_wakeup_proc(int sirq, void *dev_id)
> +static void xnpipe_wakeup_proc(struct irq_work *work)
> {
> struct xnpipe_state *state;
> unsigned long rbits;
> @@ -221,13 +221,11 @@ check_async:
> }
> out:
> xnlock_put_irqrestore(&nklock, s);
> -
> - return IRQ_HANDLED;
> }
>
> static inline void xnpipe_schedule_request(void) /* hw IRQs off */
> {
> - pipeline_post_sirq(xnpipe_wakeup_virq);
> + irq_work_queue(&xnpipe_wakeup_work);
> }
>
> static inline ssize_t xnpipe_flush_bufq(void (*fn)(void *buf, void *xstate),
> @@ -1149,6 +1147,8 @@ int xnpipe_mount(void)
> state->nroutq = 0;
> }
>
> + init_irq_work(&xnpipe_wakeup_work, xnpipe_wakeup_proc);
> +
> #if LINUX_VERSION_CODE >= KERNEL_VERSION(6,4,0)
> xnpipe_class = class_create("rtpipe");
> #else
> @@ -1180,13 +1180,6 @@ int xnpipe_mount(void)
> return -EBUSY;
> }
>
> - xnpipe_wakeup_virq = pipeline_create_inband_sirq(xnpipe_wakeup_proc);
> - if (xnpipe_wakeup_virq < 0) {
> - printk(XENO_ERR
> - "unable to reserve synthetic IRQ for message pipes\n");
> - return xnpipe_wakeup_virq;
> - }
> -
> return 0;
> }
>
> @@ -1194,8 +1187,6 @@ void xnpipe_umount(void)
> {
> int i;
>
> - pipeline_delete_inband_sirq(xnpipe_wakeup_virq);
> -
> unregister_chrdev(XNPIPE_DEV_MAJOR, "rtpipe");
>
> for (i = 0; i < XNPIPE_NDEVS; i++)
>
> ---
> base-commit: ccdceccd2508596ebd114981890b5b47b650058a
> change-id: 20250829-wip-flo-migrate-cobalt-pipe-to-irq-work-359bc1f0a8de
>
> Best regards,
Thanks, applied to next.
Jan
--
Siemens AG, Foundational Technologies
Linux Expert Center
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-09-02 21:25 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-29 15:59 [PATCH] cobalt/pipe: Fix invalid wait context in xnpipe_wakeup_proc() Florian Bezdeka
2025-09-01 13:51 ` Jan Kiszka
2025-09-01 15:28 ` Philippe Gerum
2025-09-01 16:49 ` Jan Kiszka
2025-09-02 9:13 ` Philippe Gerum
2025-09-02 15:38 ` Jan Kiszka
2025-09-02 21:15 ` Florian Bezdeka
2025-09-02 15:40 ` Jan Kiszka
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.