Re: [PATCH net-next 1/4] net: dev: Remove the preempt_disable() in netif_rx_internal().

[Sebastian Andrzej Siewior <bigeasy@linutronix.de>]

On 2022-02-03 13:41:13 [+0100], Toke Høiland-Jørgensen wrote:
> Sebastian Andrzej Siewior <bigeasy@linutronix.de> writes:
> 
> > It would but as mentioned previously: BH is disabled and
> > smp_processor_id() is stable.
> 
> Ah, right, because of the change in loopback to use netif_rx_ni()? But
> that bit of the analysis only comes later in your series, so at the very
> least you should be explaining this in the commit message here. Or you
> could potentially squash patches 1 and 2 and do both changes at once,
> since it's changing two bits of the same function and both need the same
> analysis...
> 
> However, if we're going with Eric's suggestion of an internal
> __netif_rx() for loopback that *doesn't* do local_bh_disable() then this
> code would end up being called without BH disable, so we'd need the
> migrate_disable() anyway, no?

Eric suggested to the __netif_rx() for loopback which is already in
BH-disabled section. So if that is the only "allowed" caller, we
wouldn't have to worry.
If __netif_rx() becomes more users and one calls it from preemptible
context then we have a problem (like netif_rx() vs netif_rx_ni()).

migrate_disable() will shut up smp_processor_id(), yes, but we need
something to process pending softirqs. Otherwise they are delayed until
the next IRQ, spin_unlock_bh(), etc.

> -Toke

Sebastian