Re: RE: [EXT] Re: [PATCH net v1] octeontx2-pf: Fix page pool cache index corruption.

[Sebastian Andrzej Siewior]

On 2023-09-06 08:24:53 [+0000], Ratheesh Kannoth wrote:
> > From: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
> > Subject: [EXT] Re: [PATCH net v1] octeontx2-pf: Fix page pool cache index
> > corruption.
> > 
> > > +	napi_schedule(wrk->napi);
> > 
> > This will delay NAPI until "some random point in the future" for instance if an
> > interrupt on _this_ CPU fires. You only set the softirq state and never enforce
> > it here. This works as intended if invoked from an IRQ but this here a worker/
> > process context.
> ACK.  Do we need to be so precise here ? Anyway we are short of rx buffers and want to schedule NAPI after some time 
> (in delayed workqueue) to recheck on rx buffers.  Softirq state will be checked even on timer interrupt returns, right ?.  I was thinking 
> Case - what will happen if workqueue never got a chance to run if system is stressed with interrupts. 

As of this patch, the timer will wake the worker and worker will
schedule NAPI. (The timer CPU and the worker CPU can be different.)
If nothing else happens the CPU, which run the worker, will go idle.
With NO_HZ enabled you should see warning that the CPU is going idle
with pending softirqs.
I prefer to be precise what is going on so there are no surprises.

If the CPU is stressed with interrupts then it is probably busy enough
and not running the worker to ask for memory (which it did not have
earlier) is probably the smallest problem.

I'm not against the worker (with the extra steps) but I prefer to have
napi schedule done properly.

> > You can either put local_bh_disable()/enable() around napi_schedule() or
> > use it from a timer callback and skip the worker.
> Switching to  a timer callback makes sense. 

oki.

> -Ratheesh

Sebastian