Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> writes:
> On Fri, 8 Sept 2023 at 00:03, Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
>>
>> Which on a PREEMPT_COUNT=n build will cause preemption while holding the
>> spinlock. I think something like the below will cause sufficient
>> warnings to avoid growing patterns like that.
>
> Hmm. I don't think that warning is valid.
>
> Disabling preemption is actually fine if it's done in an interrupt,
> iow if we have
>
> allow_resched();
> -> irq happens
> spin_lock(); // Ok and should *not* complain
> ...
> spin_unlock();
> <- irq return (and preemption)
>
> which actually makes me worry about the nested irq case, because this
> would *not* be ok:
>
> allow_resched();
> -> irq happens
> -> *nested* irq happens
> <- nested irq return (andapreemption)
>
> ie the allow_resched() needs to still honor the irq count, and a
> nested irq return obviously must not cause any preemption.
IIUC, this should be equivalent to:
01 allow_resched();
02 -> irq happens
03 preempt_count_add(HARDIRQ_OFFSET);
04 -> nested irq happens
05 preempt_count_add(HARDIRQ_OFFSET);
06
07 preempt_count_sub(HARDIRQ_OFFSET);
08 <- nested irq return
09 preempt_count_sub(HARDIRQ_OFFSET);
So, even if there were nested interrrupts, then the !preempt_count()
check in raw_irqentry_exit_cond_resched() should ensure that no
preemption happens until after line 09.
> I've lost sight of the original patch series, and I assume / hope that
> the above isn't actually an issue, but exactly because I've lost sight
> of the original patches and only have this one in my mailbox I wanted
> to check.
Yeah, sorry about that. The irqentry_exit_allow_resched() is pretty much
this:
+void irqentry_exit_allow_resched(void)
+{
+ if (resched_allowed())
+ raw_irqentry_exit_cond_resched();
+}
So, as long as raw_irqentry_exit_cond_resched() won't allow early
preemption, having allow_resched() set, shouldn't either.
--
ankur
