Re: [PATCH v4 05/17] irq & spin_lock: Add counted interrupt disabling/enabling

On 8/5/26 7:50 PM, Boqun Feng wrote:
> On Wed, Aug 05, 2026 at 07:40:18PM +0530, Shrikanth Hegde wrote:
>>
>> Hi Boqun,
>>
>>>
>>> Something as below? Going to send it to kernel build bot and see if it
>>> works for all configs.
>>>
>>> ----------------->8
>>> diff --git a/include/linux/interrupt_rc.h b/include/linux/interrupt_rc.h
>>> index b9a7f05ecf42..39f30bc65548 100644
>>> --- a/include/linux/interrupt_rc.h
>>> +++ b/include/linux/interrupt_rc.h
>>> @@ -12,6 +12,7 @@
>>>     */
>>>
>>>    #include <linux/irqflags.h>
>>> +#include <linux/debug_locks.h>
>>>    #include <linux/preempt.h>
>>>    #include <linux/processor.h>
>>>    #include <linux/smp.h>
>>> @@ -63,6 +64,12 @@ static inline void local_interrupt_disable(void)
>>>
>>>           new_count = hardirq_disable_enter();
>>>
>>> +       /* Is hardirq disable count overflow soon? */
>>> +       if (IS_ENABLED(CONFIG_DEBUG_PREEMPT))
>>> +               DEBUG_LOCKS_WARN_ON((new_count & HARDIRQ_DISABLE_MASK) +
>>> +                                   (10 << HARDIRQ_DISABLE_SHIFT) >
>>> +                                   HARDIRQ_DISABLE_MASK);
>>> +
>>
>> This needs a return here right? Else we will see warning for 10 times
>> and then overflow happens and we will call _local_interrupt_disable. No?
>>
> 
> DEBUG_LOCKS_WARN_ON() uses debug_locks_off() to avoid this, so we won't
> see it 10 times. The reason not using return here, because we would
> introduce unpaired local_interrupt_disable() if we returned:

Yes, it could be a weird case if the overflow actually happens.
So just warning maybe enough to catch such callers.

Maybe your kunit test can actually help test the behavior with the loop
count.

> 
> 	// hardirq disable count is n
> 	local_interrupt_disable(); // hardirq disable count is n + 1
> 
> 	local_interrupt_disable(); <- trigger the warning, if we return
> 				   // hardirq disable count is n + 1

Likely i am missing to understand.

Isn't the count incremented earlier than return?
I.e even if return happens it should be n + 2 right?

> 
> 	local_interrupt_enable(); // hardirq disable count is n
> 
> 	local_interrupt_enable(); // hardirq disable count is n - 1
> 
> Regards,
> Boqun
> 
>> Not sure, if below is any better? (Igore whitespace mangling)
>>
>> if (IS_ENABLED(CONFIG_DEBUG_PREEMPT) &&
>>      DEBUG_LOCKS_WARN_ON((preempt_count() & HARDIRQ_DISABLE_MASK) >=
>> 			 HARDIRQ_DISABLE_MASK - (10 << HARDIRQ_DISABLE_SHIFT)))
>> 	return;
>>
>>>           /* Interrupts can happen here, but it's OK, see __irq_exit_rcu(). */
>>>
>>>           if ((new_count & HARDIRQ_DISABLE_MASK) == HARDIRQ_DISABLE_OFFSET)
>>> @@ -73,6 +80,11 @@ static inline void local_interrupt_enable(void)
>>>    {
>>>           int new_count;
>>>
>>> +       /* Unpaired local_interrupt_enable()? Warn and abort. */
>>> +       if (IS_ENABLED(CONFIG_DEBUG_PREEMPT) &&
>>> +           DEBUG_LOCKS_WARN_ON((preempt_count() & HARDIRQ_DISABLE_MASK) == 0))
>>> +               return;
>>> +
>>>           new_count = hardirq_disable_exit();
>>>
>>>           if ((new_count & HARDIRQ_DISABLE_MASK) == 0) >
>>