Messages in this thread | | | From | Ankur Arora <> | Subject | Re: [PATCH v2 7/9] sched: define TIF_ALLOW_RESCHED | Date | Fri, 08 Sep 2023 22:30:57 -0700 |
| |
Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Fri, 8 Sept 2023 at 00:03, Peter Zijlstra <peterz@infradead.org> 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
| |