Re: [PATCH] dovetail: Fix interrupt re-enabling after hibernation

[Philippe Gerum <rpm@xenomai.org>]

Florian Bezdeka <florian.bezdeka@siemens.com> writes:

> On Thu, 2025-03-20 at 17:41 +0100, Jan Kiszka wrote:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>> 
>> We had unbalanced hard_cond_local_irq_disable here so far.
>> 
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>> 
>> Minus one hunk that is already in current 6.12.y, this needs to be 
>> applied to older stable versions as well.
>> 
>>  kernel/power/hibernate.c | 3 +++
>>  1 file changed, 3 insertions(+)
>> 
>> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
>> index 733b1a196f09e..e1c05f276a453 100644
>> --- a/kernel/power/hibernate.c
>> +++ b/kernel/power/hibernate.c
>> @@ -352,6 +352,7 @@ static int create_image(int platform_mode)
>>  	syscore_resume();
>>  
>>   Enable_irqs:
>> +	hard_cond_local_irq_enable();
>>  	system_state = SYSTEM_RUNNING;
>>  	local_irq_enable();
>
> General question about ordering here:
>
> The order hard_cond_local_irq_enable() followed by local_irq_enable()
> seems wrong to me. We enable hard irqs first, then we unstall the
> inband stage.
>
> That means that an hard irq might arrive and will end up in the IRQ
> log, which is not processed by local_irq_enable(), so we are delaying
> the IRQ processing to an unknown later time.
>

We are not delaying any IRQ, the implementation would just log any
incoming IRQ into the log, until __inband_irq_enable() eventually
disables them the hard way to check the log content, calling
sync_current_irq_stage() if any is to be played back.

> The other way around would make more sense to me. Is my understanding
> right?
>

Nope. The proper ordering is _always_ hard_local_irq_enable() _then_
local_irq_enable. There is also an historical reason for this (see
comment in inband_irq_enable()).

-- 
Philippe.