Re: pegged softirq and NAPI race (?)

[Jeff Kirsher <jeffrey.t.kirsher@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 2907 bytes --]

On Tue, 2018-09-18 at 14:21 -0700, Eric Dumazet wrote:
> 
> On 09/18/2018 02:13 PM, Eric Dumazet wrote:
> > On Tue, Sep 18, 2018 at 1:37 PM Song Liu <songliubraving@fb.com>
> > wrote:
> > > 
> > > Looks like a patch like the following fixes the issue for ixgbe.
> > > But I
> > > cannot explain it yet.
> > > 
> > > Does this ring a bell?
> > 
> > I dunno, it looks like the NIC is  generating an interrupt while it
> > should not,
> > and constantly sets NAPI_STATE_MISSED.
> > 
> > Or maybe we need to properly check napi_complete_done() return
> > value.
> > 
> > diff --git a/drivers/net/ethernet/intel/ixgb/ixgb_main.c
> > b/drivers/net/ethernet/intel/ixgb/ixgb_main.c
> > index
> > d3e72d0f66ef428b08e4bd88508e05b734bc43a4..c4c565c982a98a5891603cedc
> > dcb72dc1c401813
> > 100644
> > --- a/drivers/net/ethernet/intel/ixgb/ixgb_main.c
> > +++ b/drivers/net/ethernet/intel/ixgb/ixgb_main.c
> > @@ -1773,8 +1773,8 @@ ixgb_clean(struct napi_struct *napi, int
> > budget)
> >         ixgb_clean_rx_irq(adapter, &work_done, budget);
> > 
> >         /* If budget not fully consumed, exit the polling mode */
> > -       if (work_done < budget) {
> > -               napi_complete_done(napi, work_done);
> > +       if (work_done < budget &&
> > +           napi_complete_done(napi, work_done)) {
> >                 if (!test_bit(__IXGB_DOWN, &adapter->flags))
> >                         ixgb_irq_enable(adapter);
> >         }
> > 
> 
> 
> ixgbe patch would be :
> 
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index
> 604282f03d236e4358fc91e64d8ba00a9b36cb8c..80d00aecb6e3e3e950ce6309bfe
> 3639953dd73d9 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -3196,12 +3196,12 @@ int ixgbe_poll(struct napi_struct *napi, int
> budget)
>                 return budget;
>  
>         /* all work done, exit the polling mode */
> -       napi_complete_done(napi, work_done);
> -       if (adapter->rx_itr_setting & 1)
> -               ixgbe_set_itr(q_vector);
> -       if (!test_bit(__IXGBE_DOWN, &adapter->state))
> -               ixgbe_irq_enable_queues(adapter, BIT_ULL(q_vector-
> >v_idx));
> -
> +       if (napi_complete_done(napi, work_done)) {
> +               if (adapter->rx_itr_setting & 1)
> +                       ixgbe_set_itr(q_vector);
> +               if (!test_bit(__IXGBE_DOWN, &adapter->state))
> +                       ixgbe_irq_enable_queues(adapter,
> BIT_ULL(q_vector->v_idx));
> +       }
>         return min(work_done, budget - 1);
>  }
>  

Eric, after Song does some testing on these changes, will you be
submitting a formal patch?  If so, make sure to send it to 
intel-wired-lan@lists.osuosl.org mailing list so I can pick up the fix.

By the way, thanks!

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]