From: Steven Rostedt <rostedt@xxxxxxxxxxx> Date: Fri, 21 Jul 2023 12:00:40 -0400 > On Fri, 21 Jul 2023 17:34:41 +0200 > Alexander Lobakin <aleksander.lobakin@xxxxxxxxx> wrote: [...] >>> + level += !!(pc & (NMI_MASK)); >>> + level += !!(pc & (NMI_MASK | HARDIRQ_MASK)); >>> + level += !!(pc & (NMI_MASK | HARDIRQ_MASK | SOFTIRQ_OFFSET)); >> >> This doesn't take into account that we can switch the context manually >> via local_bh_disable() / local_irq_save() etc. During the testing of the > > You cannot manually switch interrupt context. > >> separate issue[0], I've found that the function returns 1 in both just >> softirq and softirq under local_irq_save(). >> Is this intended? Shouldn't that be > > That is intended behavior. > > local_bh_disable() and local_irq_save() is not a context switch. It is just > preventing that context from happening. The interrupt_context_level() is to > tell us what context we are running in, not what context is disabled. > >> >> level += !!(pc & (NMI_MASK)); >> level += !!(pc * (NMI_MASK | HARDIRQ_MASK)) || irqs_disabled(); >> level += !!(pc * (NMI_MASK | HARDIRQ_MASK | SOFTIRQ_OFFSET)) || >> in_atomic(); >> >> ? >> Otherwise, the result it returns is not really "context level". > > local_bh_disable() use to (and perhaps still does in some configurations) > confuse things. But read the comment in kernel/softirq.c > > /* > * SOFTIRQ_OFFSET usage: > * > * On !RT kernels 'count' is the preempt counter, on RT kernels this applies > * to a per CPU counter and to task::softirqs_disabled_cnt. > * > * - count is changed by SOFTIRQ_OFFSET on entering or leaving softirq > * processing. > * > * - count is changed by SOFTIRQ_DISABLE_OFFSET (= 2 * SOFTIRQ_OFFSET) > * on local_bh_disable or local_bh_enable. > * > * This lets us distinguish between whether we are currently processing > * softirq and whether we just have bh disabled. > */ > > Just because you disable interrupts does not mean you are in interrupt > context. Ah okay, thanks! IOW, if we want to check in some code that we're certainly have interrupts enabled and are not in the interrupt context, we must always do if (!(in_hardirq() || irqs_disabled())) , nothing more elegant / already existing / ...? > > -- Steve > > >> >>> + >>> + return level; >>> +} >>> + >> [0] >> https://lore.kernel.org/netdev/b3884ff9-d903-948d-797a-1830a39b1e71@xxxxxxxxx >> >> Thanks, >> Olek > Thanks, Olek
