From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Hemminger Subject: Re: [PATCH net] r8169: fix NAPI handling under high load Date: Tue, 16 Oct 2018 16:03:55 -0700 Message-ID: <20181016160355.1cc0a2e9@xeon-e3> References: <8f84fe39-3d8d-396d-3b97-027e0a83f8cb@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Cc: Heiner Kallweit , David Miller , Realtek linux nic maintainers , "netdev@vger.kernel.org" To: Holger =?UTF-8?B?SG9mZnN0w6R0dGU=?= Return-path: Received: from mail-pl1-f196.google.com ([209.85.214.196]:41365 "EHLO mail-pl1-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726697AbeJQG4h (ORCPT ); Wed, 17 Oct 2018 02:56:37 -0400 Received: by mail-pl1-f196.google.com with SMTP id q17-v6so11709897plr.8 for ; Tue, 16 Oct 2018 16:03:59 -0700 (PDT) In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: On Tue, 16 Oct 2018 23:17:31 +0200 Holger Hoffst=C3=A4tte wrote: > On 10/16/18 22:37, Heiner Kallweit wrote: > > rtl_rx() and rtl_tx() are called only if the respective bits are set > > in the interrupt status register. Under high load NAPI may not be > > able to process all data (work_done =3D=3D budget) and it will schedule > > subsequent calls to the poll callback. > > rtl_ack_events() however resets the bits in the interrupt status > > register, therefore subsequent calls to rtl8169_poll() won't call > > rtl_rx() and rtl_tx() - chip interrupts are still disabled. =20 >=20 > Very interesting! Could this be the reason for the mysterious > hangs & resets we experienced when enabling BQL for r8169? > They happened more often with TSO/GSO enabled and several people > attempted to fix those hangs unsuccessfully; it was later reverted > and has been since then (#87cda7cb43). > If this bug has been there "forever" it might be tempting to > re-apply BQL and see what happens. Any chance you could give that > a try? I'll gladly test patches, just like I'll run this one. >=20 > cheers > Holger Many drivers have buggy usage of napi_complete_done. Might even be worth forcing all network drivers to check the return value. But fixing 150 broken drivers will be a nuisance. diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index dc1d9ed33b31..c38bc66ffe74 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -466,7 +466,8 @@ static inline bool napi_reschedule(struct napi_struct *= napi) return false; } =20 -bool napi_complete_done(struct napi_struct *n, int work_done); +bool __must_check napi_complete_done(struct napi_struct *n, int work_done); + /** * napi_complete - NAPI processing complete * @n: NAPI context