From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f51.google.com (mail-wr1-f51.google.com [209.85.221.51]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id DDA72481FDD for ; Mon, 18 May 2026 16:37:48 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.51 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779122271; cv=none; b=Kg4omJpK+URgKz/r8SZQEeSgwcdbLvqNuYpeSFAqNr6Nk7+JbEHik9M9WTmhBjqGj0fSorOJqzh918EV+I32Iv7AVHVPlQsX8u56fz5GS9zLrQnAgAxcC/6Vyq1gIKjDqFR3a49rcIrGGwpRJNM3aZN7+QZJfgB58ysSLrUY7BI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779122271; c=relaxed/simple; bh=CaqRPTgKXyI2XV4PjCcbbusa6uw3fAD79fAQWeD1M6I=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=mDO/o2SPqYRFvWeC/j0Y9CPC3tFl6sLTD76F8J674IH2yaO9JKlxZ2HVXaEirFwA9d1gTFhivD7mQ6wYFoPHK0Qhl6aILcgAxhRk43lNjB8YSrKu+LOw8hKEIjv12hu7kbQiz103fFvlzi2/pRpXd6qMBUOXkOrOYjpX2JQGF1A= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=arista.com; spf=pass smtp.mailfrom=arista.com; dkim=pass (2048-bit key) header.d=arista.com header.i=@arista.com header.b=GTy5Vpzl; arc=none smtp.client-ip=209.85.221.51 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=arista.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=arista.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=arista.com header.i=@arista.com header.b="GTy5Vpzl" Received: by mail-wr1-f51.google.com with SMTP id ffacd0b85a97d-44e1ebb3122so1460874f8f.2 for ; Mon, 18 May 2026 09:37:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=arista.com; s=google; t=1779122267; x=1779727067; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=bWMCe3ghcYJBdMMLjkrfnvrFFAeOfYLES9EzhhtO5zQ=; b=GTy5VpzlP8Acq99bNuZNx4u0hzLo54Fz5DCJ0CCvKJOQm1PwwGA3kMSx78WU0wNyDA hvjm04HSG6vDQzDeUsCe5FluuTyu6kRqIjHGxdokpPqfjU1NMprli2kRNEiX80pvC5N6 eSd1OKVrMXWivL0VA/ayYiPEM1oI9opLqZukGoSxX7rvgdakCkJJnYpC8P1KNtiFrdck tY/Y2vM1VZsUPzwRLGmaQjXTNu+79LHkjE1S7f2TQwezGfExijV2oEDGSh329kdXcqBK zQwqp81/umNLiMZrff3gGrnoofepXZmfJaE0QY7tYtgZ6PHUw8E7YltP26sDynj3tT2f anyQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1779122267; x=1779727067; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=bWMCe3ghcYJBdMMLjkrfnvrFFAeOfYLES9EzhhtO5zQ=; b=H1yXvHyye4+6vCzSJ4NAI/Gvcyi5BGgtzci3zpIb9sRhT1xscljjlrYbZLDGyFkABp PK2NCLE09+JQWMJ6Mda7iIwveP/xeK8H90wlIpVYltKDmg/0X6XNz0wQgNteqDCRSGmG rNs8hUUxhYPoRGEfwb0sc+qSMFRYheFyp+BARpEMvot60b3ldzi4xvASIn88UHgAqxNH 85FFgenLtFW3/jMD25rTonyPMfXEZ2V7XHFZ65dHym9DutEUSz6A0eIiaEoZnS5OvZxN l+DO2uKJD6EIjXa4ibP19rc+PJPiukz3sHvKcSSKE5yun/aJ5/5RvO5GNRnX4vTY/mKY cULg== X-Forwarded-Encrypted: i=1; AFNElJ+KPHrSinWjZ24pguf6O1VSMN/HcLtRRrtQOw5DqbFQKBpl46YcIi8gxht3sDO/w2MgVrTzO+74rbDNnqw=@vger.kernel.org X-Gm-Message-State: AOJu0Yy5hwQqxqfdZH+ZrOaV7Zxa4Jl+HLd4Jjg28sgxF4FXc80KBbq2 Rxh75WCPh/42+s8Ol1C25P5tNOatr3LS/4Q/PxkS0dBV00RYGN3DxYWR8rRj1ZVBsw== X-Gm-Gg: Acq92OE/8ScHl6yBK0JRdYPSCaQUyf9NiWjHuiSZf3bKdtfESxhMWBdAeNoRwfIlJwH Bu7fQEZpGGX4KS+xvQhfWVhDp5xRKVMNO1v0eQ5+NN6CE5UdNCyHDrVQpDNOfNx5npxYJlVSywX ZAuwBziTftcsAVfYylNi6hBe8bE0qeGRlBRKJ41ZEeYvT40LkT25KjjrlKj0Fnue7Ovet2Hpq32 Lm5kXEI+919lEc1/cdKlUEc+w56LZSFBjIOkE70r8e4T27/aPw9Cay3ihFrHENipBvmIfvkwYrr FNHkoS79K99KPfqcEguxwadL/bohP3/Huvrr0edGMQwCJpZwlXJdHSri3c583M9HcAhx+d1Yq6d kSx68tPvujm7ApJo1jrkjG054pUUGY/RBysS3BrJUy5lJU4q2Mfgsf6YINxBXAWT2B3IkueuRFg BX4wcm6pBLdR00Jc4O0WxwIQA= X-Received: by 2002:a5d:64e4:0:b0:43e:b0f8:66f1 with SMTP id ffacd0b85a97d-45e5c5aebdcmr26353139f8f.43.1779122267158; Mon, 18 May 2026 09:37:47 -0700 (PDT) Received: from [10.83.43.186] ([159.134.255.34]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-45e6a135f0csm19940345f8f.27.2026.05.18.09.37.45 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 18 May 2026 09:37:46 -0700 (PDT) Message-ID: Date: Mon, 18 May 2026 17:37:45 +0100 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] tg3: guard napi_disable and pci_disable_device calls To: Pavan Chebbi Cc: mchan@broadcom.com, netdev@vger.kernel.org, andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com, kuba@kernel.org, pabeni@redhat.com, linux-kernel@vger.kernel.org References: Content-Language: en-US From: "Yury M." In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit It looks like your opinion is based on the fact that AER recovery failure is a critical issue after which the system should be considered unstable. But I disagree with that. AER error recovery failure can happen not only because of PCI_ERS_RESULT_NO_AER_DRIVER (arguably considered a buggy or unsupported PCIe bus configuration). There are other AER recovery failure reasons. And until the system is locked by tg3, it has a chance to be recovered, and tg3 should not be a blocker here. I would accept the case where, after AER recovery failure, the tg3 device stops working. We can consider it as an unsupported case where manual intervention is required to recover tg3. But in this case, it blocks rtnl_lock. For me, any AER error recovery failure is a legitimate condition in Linux. Maybe we can even consider that the whole bus is in an unexpected/unstable state. But we still should give the software a chance to recover the system. Especially when we are talking about the support of hot-pluggable devices. Insertion or removal of a new PCIe device (without AER recovery support) onto the same bus as the tg3 device should not cause an rtnl_lock hang. Sorry, maybe I'm wrong and AER recovery failure is really a critical and unexpected system error. Theoretically, in this particular case, we can try to fix PCI_ERS_RESULT_NO_AER_DRIVER and avoid changes in tg3. But I'm sure that there are other cases where AER recovery can fail and there are many other hot-pluggable devices that can return PCI_ERS_RESULT_NO_AER_DRIVER or cause another type of AER recovery failure. As such, it looks like it is better to handle AER recovery failure in tg3. Another long-term fix could be to add another AER error handler callback to signal drivers about AER recovery failure. On 5/18/26 16:43, Pavan Chebbi wrote: >> To do cleanup on device-stop or error handling is a common approach, and it is not a problem. The problem is that tg3 doesn't track the device status. If a cleanup was performed (napi_disable and pci_disable_device called), tg3 should know that the device is not in an initialized state. When we subsequently try to disable the device, tg3 should not try to cleanup again. That is the problem I'm trying to fix. Frankly speaking, I'm adding a flag which signals that device cleanup was completed during the handling of an AER error, so when tg3 stops/removes the device, it should not perform cleanup again. > I understand this. But my view is that you are trying to fix a > symptom. The real issue seems to be that you are having some devices > in your bus/subtree that don't support AER. > Looking at pcie_do_recovery(), the recovery aborts the moment > NO_AER_DRIVER is received by the port driver. I would like to think > that this is by design. > >> Maybe PCI_ERS_RESULT_NO_AER_DRIVER is a rare case in the PCIe world, but I think that in any case, the tg3 driver should correctly handle AER recovery failure. Recovery can fail not only because of the PCI_ERS_RESULT_NO_AER_DRIVER return code. The problem is that a double napi_disable call causes a soft lockup, and not just one driver/device stops functioning—the whole system is affected. >> > Like I said, the tg3 driver does handle the AER sequence correctly. > The problem really is the tg3 remove() is facing issues because the > PCIe devices' topology is not suitable for AER recovery. > The issue a consequence of tg3 suffering a stall during AER because it > never recvd slot_reset() and resume(). > > I am not sure if we should allow this fix for what I think is a > situation arising out of an unsupported topology.. > I will defer this to Michael or netdev maintainers for their feedback.