On Thu, 20 Jul 2023 20:13:07 +0200 Alexander Lobakin wrote:

IOW, it reports we're in softirq no bloody matter if interrupts are
enabled or not. Either I did something wrong or the entire in_*irq()
family, including interrupt_context_level(), doesn't protect from
anything at all and doesn't work the way that most devs expect it to work?

(or was it just me? :D)

I guess the only way to be sure is to always check irqs_disabled() when
in_softirq() returns true.

We can as well check
	(in_softirq() && !irqs_disabled() && !in_hardirq())
?

The interrupt_context_level() thing is fairly new, I think.
Who knows what happens to it going forward...

quoted

quoted

Right now page pool only supports BH and process contexts. IOW the
"else" branch of if (in_softirq()) in page pool is expecting to be
in process context.

Supporting hard irq would mean we need to switch to _irqsave() locking.
That's likely way too costly.

Or stash the freed pages away and free them lazily.

Or add a lockdep warning and hope nobody will ever free a page-pool
backed skb from hard IRQ context :)  

I told you under the previous version that this function is not supposed
to be called under hardirq context, so we don't need to check for it :D
But I was assuming nobody would try to do that. Seems like not really
(netcons) if you want to sanitize this...

netcons or anyone who freed socket-less skbs from hardirq.
Until pp recycling was added freeing an skb from hardirq was legal,
AFAICT.