Re: [PATCH net 1/1] net: stmmac: add fsleep() in HW Rx timestamp checking loop

Jakub Kicinski
2022-04-14T08:43:21Z

On Thu, 14 Apr 2022 15:29:34 +0800 Tan Tee Min wrote:
> > > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
> > > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
> > > @@ -279,10 +279,11 @@ static int dwmac4_wrback_get_rx_timestamp_status(void *desc, void *next_desc,
> > > /* Check if timestamp is OK from context descriptor */
> > > do {
> > > ret = dwmac4_rx_check_timestamp(next_desc);
> > > - if (ret < 0)
> > > + if (ret <= 0)
> > > goto exit;
> > > i++;
> > >
> > > + fsleep(1);
> >
> > This is nutty. Why isn't this code using proper deferral mechanisms
> > like work or kthread?
>
> Appreciate your comment.
> The dwmac4_wrback_get_rx_timestamp_status() is called by stmmac_rx()
> function which is scheduled by NAPI framework.
> Do we still need to create deferred work inside NAPI work?
> Would you mind to explain it more in detail?

fsleep() is a big hammer, can you try cpu_relax() and bumping the max
loop count a little?