Re: [PATCH v3] net: mvneta: re-enable percpu interrupt on resume

[Sebastian Andrzej Siewior]

On 2026-06-18 21:56:12 [+0800], Zhou, Yun wrote:
> 
> > But if the thread is idle then you have one enable too many, don't you?
> > Well you have the NAPI callback which does disable on the local CPU and
> > this resume which enables it on every CPU. So this does not look right.
> > 
> 
> The enable in resume is intentionally unconditional and idempotent
> (writing MPIC_INT_CLEAR_MASK on an already unmasked IRQ is a no-op).
> 
> > The interesting question is what happens to the enable_percpu_irq() from
> > the mvneta_poll(). Is it lost? And if so, how/ why?
> > 
> 
> The enable_percpu_irq() from mvneta_poll is not "lost" — it never
> gets a chance to execute. The sequence is:
> 
> 1. mvneta_percpu_isr: disable_percpu_irq() + napi_schedule()
> 2. PM freezes kthreads (on PREEMPT_RT, softirq runs in kthread)

But napi_schedule() runs in ksoftird, doesn't it? The per-CPU IRQs are
not threaded. That does not look optimal.

> 3. NAPI poll cannot run → enable_percpu_irq() is never called
> 4. mvneta_stop_dev → napi_disable(): cancels the scheduled poll
>     but does NOT execute the completion path (no enable_percpu_irq)

napi_schedule() sets NAPIF_STATE_SCHED.
napi_disable() sets NAPI_STATE_DISABLE and waits until NAPIF_STATE_SCHED
is cleared. So if NAPIF_STATE_SCHED was set then enable_percpu_irq()
will be invoked unless it leaves somewhere early.
However, if DISABLED was already set then it disables the IRQ source but
does not schedule NAPI. This is probably what happens.

> 5. Resume → napi_enable(): resets NAPI state but MPIC stays masked
> 
> The unconditional enable in resume covers this case. When NAPI was
> idle at suspend time, the extra enable is harmless.

There is no desc::depth counting here, that got me confused. But that
per-CPU irq is not optimal. Is this a SoC limitation or a design choice?
Having a NAPI instance with IRQ per queue and those configured and
spread among CPUs should cause less trouble and is what others do.
In fact is the only net driver using per-CPU interrupts.

> BR,
> Yun

Sebastian