Re: pegged softirq and NAPI race (?)

[Eric Dumazet <eric.dumazet@gmail.com>]

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..c4c565c982a98a5891603cedcdcb72dc1c401813
> 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..80d00aecb6e3e3e950ce6309bfe3639953dd73d9 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);
 }