stmmac: Race in coalesce timer and NAPI

stmmac: Race in coalesce timer and NAPI

[Jose Abreu]

Hello,

I'm getting a race in stmmac coalesce timer and the
napi_schedule() interrupt and I'm asking for advice. Currently,
we are scheduling NAPI in coalesce timer but this leads to
stmmac_tx_clean() deadlock because this function tries to acquire
queue lock.

I find that this is not expected because only one instance of
NAPI should run at same time so I was wondering if it is possible
that xmit() callback is causing the deadlock ?

BTW, this is solved by:
    - Directly call stmmac_tx_clean() in timer function AND
    - Use netif_tx_trylock() in stmmac_tx_clean(). Then, if queue
is already locked we re-arm coalesce timer or reschedule NAPI.

This is easily reproducible in an ARM board with 8 core running
at 100MHz each.

Thanks and Best Regards,
Jose Miguel Abreu

Re: stmmac: Race in coalesce timer and NAPI

[Eric Dumazet]

On 09/21/2018 02:19 AM, Jose Abreu wrote:
> Hello,
> 
> I'm getting a race in stmmac coalesce timer and the
> napi_schedule() interrupt and I'm asking for advice. Currently,
> we are scheduling NAPI in coalesce timer but this leads to
> stmmac_tx_clean() deadlock because this function tries to acquire
> queue lock.

This is strange. Which lock are you talking about ?

The napi_schedule() stuff should be enough to protect your use case.


> 
> I find that this is not expected because only one instance of
> NAPI should run at same time so I was wondering if it is possible
> that xmit() callback is causing the deadlock ?
> 
> BTW, this is solved by:
>     - Directly call stmmac_tx_clean() in timer function AND
>     - Use netif_tx_trylock() in stmmac_tx_clean(). Then, if queue
> is already locked we re-arm coalesce timer or reschedule NAPI.
> 
> This is easily reproducible in an ARM board with 8 core running
> at 100MHz each.
> 
> Thanks and Best Regards,
> Jose Miguel Abreu
> 

It looks to me stmmac_napi_poll() should not apply/consume any budget for TX completion.

The budget for a NAPI poll shared by RX and TX is really only for the RX side.

netpoll will specificall call the poll() with budget==0 to only drain TX