From mboxrd@z Thu Jan 1 00:00:00 1970 From: Song Liu Subject: Re: pegged softirq and NAPI race (?) Date: Tue, 18 Sep 2018 20:37:45 +0000 Message-ID: <0FD562CC-CDE9-43C8-9623-B42AC7A208C8@fb.com> References: Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Cc: Alexei Starovoitov , netdev , "Jeff Kirsher" , Alexander Duyck , "michael.chan@broadcom.com" , Kernel Team To: Eric Dumazet Return-path: Received: from mx0a-00082601.pphosted.com ([67.231.145.42]:41652 "EHLO mx0a-00082601.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729770AbeISCMM (ORCPT ); Tue, 18 Sep 2018 22:12:12 -0400 In-Reply-To: Content-Language: en-US Content-ID: <63D26F5B53B5CD49BBA03F5CD6DB13C8@namprd15.prod.outlook.com> Sender: netdev-owner@vger.kernel.org List-ID: > On Sep 18, 2018, at 11:17 AM, Eric Dumazet wrote: >=20 > On Tue, Sep 18, 2018 at 10:51 AM Alexei Starovoitov wrote: >>=20 >> On 9/18/18 6:45 AM, Eric Dumazet wrote: >>> On Tue, Sep 18, 2018 at 1:41 AM Song Liu wrote: >>>>=20 >>>> We are debugging this issue that netconsole message triggers pegged so= ftirq >>>> (ksoftirqd taking 100% CPU for many seconds). We found this issue in >>>> production with both bnxt and ixgbe, on a 4.11 based kernel. This is e= asily >>>> reproducible with ixgbe on 4.11, and latest net/net-next (see [1] for = more >>>> detail). >>>>=20 >>>> After debugging for some time, we found that this issue is likely rela= ted >>>> to 39e6c8208d7b ("net: solve a NAPI race"). After reverting this commi= t, >>>> the steps described in [1] cannot reproduce the issue on ixgbe. Revert= ing >>>> this commit also reduces the chances we hit the issue with bnxt (it st= ill >>>> happens with a lower rate). >>>>=20 >>>> I tried to fix this issue with relaxed variant (or older version) of >>>> napi_schedule_prep() in netpoll, just like the one on napi_watchdog(). >>>> However, my tests do not always go as expected. >>>>=20 >>>> Please share your comments/suggestions on which direction shall we try >>>> to fix this. >>>>=20 >>>> Thanks in advance! >>>> Song >>>>=20 >>>>=20 >>>> [1] https://urldefense.proofpoint.com/v2/url?u=3Dhttps-3A__www.spinics= .net_lists_netdev_msg522328.html&d=3DDwIBaQ&c=3D5VD0RTtNlTh3ycd41b3MUw&r=3D= i6WobKxbeG3slzHSIOxTVtYIJw7qjCE6S0spDTKL-J4&m=3DiSaOapj1kxjhGYLgQr0Qd8mQCzV= dobmgT1L4JwFvzxs&s=3DlCEhrz6wQJUUaJOkxFmtOszAgkf3Jh4reX_i1GbI5RI&e=3D >>>=20 >>> You have not traced ixgbe to understand why driver hits >>> "clean_complete=3Dfalse" all the time ? >>=20 >> Eric, >>=20 >> I'm looking at commit 39e6c8208d7b and wondering that it's doing >> clear_bit(NAPI_STATE_MISSED,..); >> for busy_poll_stop(), but not for netpoll. >> Can that be an issue? >>=20 >> and then something like below is needed: >> diff --git a/net/core/netpoll.c b/net/core/netpoll.c >> index 57557a6a950c..a848be6b503c 100644 >> --- a/net/core/netpoll.c >> +++ b/net/core/netpoll.c >> @@ -172,6 +172,7 @@ static void poll_one_napi(struct napi_struct *napi) >> trace_napi_poll(napi, work, 0); >>=20 >> clear_bit(NAPI_STATE_NPSVC, &napi->state); >> + clear_bit(NAPI_STATE_MISSED, &napi->state); >> } >=20 >=20 > NAPI_STATE_MISSED should only be cleared under strict circumstances. >=20 > The clear in busy_poll_stop() is an optimization really (as explained > in the comment) >=20 > It is cleared when napi_complete_done() is eventually called, but if > ixgbe always handle 64 RX frames in its poll function, > napi_complete_done() will not be called. The bug is in ixgbe, > pretending its poll function should be called forever. Looks like a patch like the following fixes the issue for ixgbe. But I cannot explain it yet.=20 Does this ring a bell? Thanks, Song diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/et= hernet/intel/ixgbe/ixgbe_main.c index 787c84fb20dd..51611f799dae 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c @@ -3059,11 +3059,14 @@ static irqreturn_t ixgbe_msix_other(int irq, void *= data) static irqreturn_t ixgbe_msix_clean_rings(int irq, void *data) { struct ixgbe_q_vector *q_vector =3D data; + struct napi_struct *napi =3D &q_vector->napi; /* EIAM disabled interrupts (on this vector) for us */ - if (q_vector->rx.ring || q_vector->tx.ring) - napi_schedule_irqoff(&q_vector->napi); + if ((q_vector->rx.ring || q_vector->tx.ring) && + !napi_disable_pending(napi) && + !test_and_set_bit(NAPI_STATE_SCHED, &napi->state)) + __napi_schedule_irqoff(napi); return IRQ_HANDLED; }