mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: David Christensen <drc@linux.vnet.ibm.com>
To: Michael Chan <michael.chan@broadcom.com>
Cc: Netdev <netdev@vger.kernel.org>,
	Siva Reddy Kallam <siva.kallam@broadcom.com>,
	Prashant Sreedharan <prashant@broadcom.com>,
	Michael Chan <mchan@broadcom.com>,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] tg3: driver sleeps indefinitely when EEH errors exceed eeh_max_freezes
Date: Mon, 15 Jun 2020 15:21:14 -0700	[thread overview]
Message-ID: <95bf20c6-a812-32ad-fd38-45cba7e10491@linux.vnet.ibm.com> (raw)
In-Reply-To: <CACKFLimd0a=Y8WyvqCt4BD7SU_Cg1vQ=baKs6-uPv0dZuCm=mw@mail.gmail.com>

On 6/15/20 1:45 PM, Michael Chan wrote:
> On Mon, Jun 15, 2020 at 12:01 PM David Christensen
> <drc@linux.vnet.ibm.com> wrote:
>>
>> The driver function tg3_io_error_detected() calls napi_disable twice,
>> without an intervening napi_enable, when the number of EEH errors exceeds
>> eeh_max_freezes, resulting in an indefinite sleep while holding rtnl_lock.
>>
>> The function is called once with the PCI state pci_channel_io_frozen and
>> then called again with the state pci_channel_io_perm_failure when the
>> number of EEH failures in an hour exceeds eeh_max_freezes.
>>
>> Protecting the calls to napi_enable/napi_disable with a new state
>> variable prevents the long sleep.
> 
> This works, but I think a simpler fix is to check tp->pcierr_recovery
> in tg3_io_error_detected() and skip most of the tg3 calls (including
> the one that disables NAPI) if the flag is true.

This might be the smallest change that would work.  Does it make sense 
to the reader?

diff --git a/drivers/net/ethernet/broadcom/tg3.c 
b/drivers/net/ethernet/broadcom/tg3.c
index 7a3b22b35238..1f37c69d213d 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -18168,8 +18168,8 @@ static pci_ers_result_t 
tg3_io_error_detected(struct pci_dev *pdev,

         rtnl_lock();

-       /* We probably don't have netdev yet */
-       if (!netdev || !netif_running(netdev))
+       /* May be second call or maybe we don't have netdev yet */
+       if (tp->pcierr_recovery || !netdev || !netif_running(netdev))
                 goto done;

         /* We needn't recover from permanent error */


  reply	other threads:[~2020-06-15 22:21 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-15 19:01 David Christensen
2020-06-15 20:45 ` Michael Chan
2020-06-15 22:21   ` David Christensen [this message]
2020-06-16  1:33     ` Michael Chan
2020-06-17 18:51 ` [PATCH v2] " David Christensen
2020-06-17 19:07   ` Michael Chan
2020-06-19  3:35   ` David Miller
2020-07-25  0:19   ` David Christensen
2020-07-27  7:26     ` Michael Chan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=95bf20c6-a812-32ad-fd38-45cba7e10491@linux.vnet.ibm.com \
    --to=drc@linux.vnet.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mchan@broadcom.com \
    --cc=michael.chan@broadcom.com \
    --cc=netdev@vger.kernel.org \
    --cc=prashant@broadcom.com \
    --cc=siva.kallam@broadcom.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®