Re: [PATCH v2 7/9] sched: define TIF_ALLOW_RESCHED

[Ankur Arora]

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