Re: [QUESTION] problems report: rcu_read_unlock_special() called in irq_exit() causes dead loop

[Joel Fernandes <joelagnelf@nvidia.com>]

On 7/6/2025 11:06 PM, Qi Xi wrote:
> Hi Joel,
> 
> After reverting the previous 2 commits and applying the new bugfix, the problem
> is resolved now.
> 
> Thanks for your help!

Thanks! I will add your Tested-by tag to the commit and will post for further
review.

 - Joel


> 
> 
> Best regards,
> 
> Qi
> 
> 
> On 2025/7/5 21:12, Joel Fernandes wrote:
>> On Thu, Jul 03, 2025 at 09:04:31AM +0800, Xiongfeng Wang wrote:
>>>
>>> On 2025/7/3 1:24, Joel Fernandes wrote:
>>>>
>>>> On 7/2/2025 6:59 AM, Joel Fernandes wrote:
>>>>>
>>>>> On 7/2/2025 5:14 AM, Qi Xi wrote:
>>>>>> Hi Joel,
>>>>>>
>>>>>> After applying the 2 patches, the problem still exists. Compared to the
>>>>>> previous
>>>>>> fixes which did solve the problem, the difference is ct_in_irq() in the first
>>>>>> patch.
>>>>>>
>>>>>> I am wondering why "nesting != CT_NESTING_IRQ_NONIDLE" is added?
>>>>>>
>>>>>>
>>>>>> (previous fix: problem is solved)
>>>>>>
>>>>>> +bool ct_in_irq(void)
>>>>>> +{
>>>>>> +    return ct_nmi_nesting() != 0;
>>>>>> +}
>>>>>>
>>>>>> (current fix: problem still exists)
>>>>>>
>>>>>> +bool ct_in_irq(void)
>>>>>> +{
>>>>>> +    long nesting = ct_nmi_nesting();
>>>>>> +
>>>>>> +    return (nesting && nesting != CT_NESTING_IRQ_NONIDLE);
>>>>>> +}
>>>>> Oh gosh, thanks for spotting that! Indeed,  I had changed it to != 0 in the
>>>>> last
>>>>> version but applied an older patch. I will fix it in the tree. Thank you
>>>>> again!
>>>>>
>>>>> Neeraj, would you like this as a separate commit that you can then squash? Or
>>>>> could you fix it up in your tree?
>>>>>
>>>> Qi, Xiongfeng, I am currently working on alternative fix after discussing with
>>>> the crew. I will keep you posted with the fix, and would it to be good to get
>>>> your testing on it once I have it (hopefully in couple of days), thanks for the
>>>> report!
>>> Sure, we are glad to help test once we get the fix patch.
>> Could you try the following patch? I tested it and it fixes the issue for me.
>>
>> If you prefer git, then cherry-pick the HEAD commit from:
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/jfern/linux.git/log/?h=rcu/
>> irq-exit-v2-no-task
>> (cherry-pick sha a58cc91fdca766cb719fb8beee3bb10ffe8e9d58)
>>
>> ---8<---
>>
>> From: Joel Fernandes <joelagnelf@nvidia.com>
>> Subject: [PATCH] rcu: Fix rcu_read_unlock() deadloop due to IRQ work
>>
>> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
>> ---
>>   kernel/rcu/tree.h        | 11 ++++++++++-
>>   kernel/rcu/tree_plugin.h | 29 ++++++++++++++++++++++-------
>>   2 files changed, 32 insertions(+), 8 deletions(-)
>>
>> diff --git a/kernel/rcu/tree.h b/kernel/rcu/tree.h
>> index 3830c19cf2f6..f8f612269e6e 100644
>> --- a/kernel/rcu/tree.h
>> +++ b/kernel/rcu/tree.h
>> @@ -174,6 +174,15 @@ struct rcu_snap_record {
>>       unsigned long   jiffies;    /* Track jiffies value */
>>   };
>>   +/*
>> + * The IRQ work (deferred_qs_iw) is used by RCU to get scheduler's attention.
>> + * It can be in one of the following states:
>> + * - DEFER_QS_IDLE: An IRQ work was never scheduled.
>> + * - DEFER_QS_PENDING: An IRQ work was scheduler but never run.
>> + */
>> +#define DEFER_QS_IDLE        0
>> +#define DEFER_QS_PENDING    1
>> +
>>   /* Per-CPU data for read-copy update. */
>>   struct rcu_data {
>>       /* 1) quiescent-state and grace-period handling : */
>> @@ -192,7 +201,7 @@ struct rcu_data {
>>                       /*  during and after the last grace */
>>                       /* period it is aware of. */
>>       struct irq_work defer_qs_iw;    /* Obtain later scheduler attention. */
>> -    bool defer_qs_iw_pending;    /* Scheduler attention pending? */
>> +    int defer_qs_iw_pending;    /* Scheduler attention pending? */
>>       struct work_struct strict_work;    /* Schedule readers for strict GPs. */
>>         /* 2) batch handling */
>> diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
>> index dd1c156c1759..baf57745b42f 100644
>> --- a/kernel/rcu/tree_plugin.h
>> +++ b/kernel/rcu/tree_plugin.h
>> @@ -486,13 +486,16 @@ rcu_preempt_deferred_qs_irqrestore(struct task_struct
>> *t, unsigned long flags)
>>       struct rcu_node *rnp;
>>       union rcu_special special;
>>   +    rdp = this_cpu_ptr(&rcu_data);
>> +    if (rdp->defer_qs_iw_pending == DEFER_QS_PENDING)
>> +        rdp->defer_qs_iw_pending = DEFER_QS_IDLE;
>> +
>>       /*
>>        * If RCU core is waiting for this CPU to exit its critical section,
>>        * report the fact that it has exited.  Because irqs are disabled,
>>        * t->rcu_read_unlock_special cannot change.
>>        */
>>       special = t->rcu_read_unlock_special;
>> -    rdp = this_cpu_ptr(&rcu_data);
>>       if (!special.s && !rdp->cpu_no_qs.b.exp) {
>>           local_irq_restore(flags);
>>           return;
>> @@ -623,12 +626,24 @@ notrace void rcu_preempt_deferred_qs(struct task_struct *t)
>>    */
>>   static void rcu_preempt_deferred_qs_handler(struct irq_work *iwp)
>>   {
>> -    unsigned long flags;
>> -    struct rcu_data *rdp;
>> +    volatile unsigned long flags;
>> +    struct rcu_data *rdp = this_cpu_ptr(&rcu_data);
>>   -    rdp = container_of(iwp, struct rcu_data, defer_qs_iw);
>>       local_irq_save(flags);
>> -    rdp->defer_qs_iw_pending = false;
>> +
>> +    /*
>> +     * Requeue the IRQ work on next unlock in following situation:
>> +     * 1. rcu_read_unlock() queues IRQ work (state -> DEFER_QS_PENDING)
>> +     * 2. CPU enters new rcu_read_lock()
>> +     * 3. IRQ work runs but cannot report QS due to rcu_preempt_depth() > 0
>> +     * 4. rcu_read_unlock() does not re-queue work (state still PENDING)
>> +     * 5. Deferred QS reporting does not happen.
>> +     */
>> +    if (rcu_preempt_depth() > 0) {
>> +        WRITE_ONCE(rdp->defer_qs_iw_pending, DEFER_QS_IDLE);
>> +        local_irq_restore(flags);
>> +        return;
>> +    }
>>       local_irq_restore(flags);
>>   }
>>   @@ -675,7 +690,7 @@ static void rcu_read_unlock_special(struct task_struct *t)
>>               set_tsk_need_resched(current);
>>               set_preempt_need_resched();
>>               if (IS_ENABLED(CONFIG_IRQ_WORK) && irqs_were_disabled &&
>> -                expboost && !rdp->defer_qs_iw_pending && cpu_online(rdp->cpu)) {
>> +                expboost && rdp->defer_qs_iw_pending != DEFER_QS_PENDING &&
>> cpu_online(rdp->cpu)) {
>>                   // Get scheduler to re-evaluate and call hooks.
>>                   // If !IRQ_WORK, FQS scan will eventually IPI.
>>                   if (IS_ENABLED(CONFIG_RCU_STRICT_GRACE_PERIOD) &&
>> @@ -685,7 +700,7 @@ static void rcu_read_unlock_special(struct task_struct *t)
>>                   else
>>                       init_irq_work(&rdp->defer_qs_iw,
>>                                 rcu_preempt_deferred_qs_handler);
>> -                rdp->defer_qs_iw_pending = true;
>> +                rdp->defer_qs_iw_pending = DEFER_QS_PENDING;
>>                   irq_work_queue_on(&rdp->defer_qs_iw, rdp->cpu);
>>               }
>>           }