From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heiner Kallweit Subject: Re: [PATCH net-next] r8169: perform reset synchronously in __rtl8169_resume Date: Tue, 22 May 2018 00:38:56 +0200 Message-ID: References: <040eb6ae-bb21-2d27-14e0-b291cb4cc1c9@gmail.com> <20180521212422.GA25067@electric-eye.fr.zoreil.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: David Miller , Realtek linux nic maintainers , "netdev@vger.kernel.org" To: Francois Romieu Return-path: Received: from mail-wm0-f65.google.com ([74.125.82.65]:52523 "EHLO mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750858AbeEUWjF (ORCPT ); Mon, 21 May 2018 18:39:05 -0400 Received: by mail-wm0-f65.google.com with SMTP id w194-v6so27662666wmf.2 for ; Mon, 21 May 2018 15:39:04 -0700 (PDT) In-Reply-To: <20180521212422.GA25067@electric-eye.fr.zoreil.com> Sender: netdev-owner@vger.kernel.org List-ID: Am 21.05.2018 um 23:24 schrieb Francois Romieu: > Heiner Kallweit : > [...] >> diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c >> index 75dfac024..1eb4f625a 100644 >> --- a/drivers/net/ethernet/realtek/r8169.c >> +++ b/drivers/net/ethernet/realtek/r8169.c >> @@ -7327,9 +7327,9 @@ static void __rtl8169_resume(struct net_device *dev) >> rtl_lock_work(tp); >> napi_enable(&tp->napi); >> set_bit(RTL_FLAG_TASK_ENABLED, tp->wk.flags); >> + if (netif_running(dev)) >> + rtl_reset_work(tp); >> rtl_unlock_work(tp); >> - >> - rtl_schedule_task(tp, RTL_FLAG_TASK_RESET_PENDING); >> } >> >> static int rtl8169_resume(struct device *device) > > netif_running() returns true before rtl_open(): issuing rtl_reset_work() > synchronously against uninitialized rings right at the start of rtl_open() > won't work as expected. > rtl8169_runtime_resume() has a check for tp->TxDescArray at the beginning. Therefore it will return immediately before even calling __rtl8169_resume(), because tp->TxDescArray is set in rtl_open() only. So I don't think the described issue exists. However I have to admit that I wasn't aware that netif_running() is true already when entering rtl_open(), so thanks for the hint. > pm_runtime_get_sync() could be issued later in rtl_open() (but I would > not mind something different with runtime_{resume/suspend} in open/close). >