Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <0166d13c-dc55-c376-28ca-dae0a872b518@intel.com>
Date:   Wed, 12 Apr 2023 16:33:06 -0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.9.1
Subject: Re: [PATCH net-next v2] net/mlx5: stop waiting for PCI link if reset
 is required
Content-Language: en-US
To:     Niklas Schnelle <schnelle@linux.ibm.com>,
        Saeed Mahameed <saeedm@nvidia.com>,
        Leon Romanovsky <leon@kernel.org>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>
CC:     Gerd Bayer <gbayer@linux.ibm.com>,
        Alexander Schmidt <alexs@linux.ibm.com>,
        Leon Romanovsky <leonro@nvidia.com>, <netdev@vger.kernel.org>,
        <linux-rdma@vger.kernel.org>, <linux-kernel@vger.kernel.org>
References: <20230411105103.2835394-1-schnelle@linux.ibm.com>
From:   Jacob Keller <jacob.e.keller@intel.com>
In-Reply-To: <20230411105103.2835394-1-schnelle@linux.ibm.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?bVN5NHMrNkYzZFRTaHdoeHRFdVlJLzh1ZTUwM1lCbHJjL0I3TkRuT0phWVpx?=
 =?utf-8?B?cDJQT1paU2dFeG1FWWs5VS9kMnh3Y1RiaFN0cnVwOUZsem5IME5HcTdFbDNV?=
 =?utf-8?B?Mlc0YzNPN0p2dHZkZG9Qb0FaRlFZaFdJdFo0NHR1Z0dIdkU5U3ZBQzhEVnR5?=
 =?utf-8?B?cnBpZW4wUXFZamVUL2UvSXZVc2VId3ozNnpXZmVTWnczdkFyTUdwRW04U0Vm?=
 =?utf-8?B?cDJGaStuK3NMdkxDd3UrdWJubG5wVUE0dS8vdTQvZWFlelRVSDlZdlprRkls?=
 =?utf-8?B?bjNIZ3RpVzRTalUzamdkN2JCdm80UXBVRm5jalFDY1VIZStLY29icktKdW82?=
 =?utf-8?B?d0oxN3FoZFFCell5UmxJc04rNXQ2VjBGRFRXTHQ3dTd0NGYwK3pnTzNsalRz?=
 =?utf-8?B?cE5Ua1FvY1ZyYmxVNzFicHo4WEFzSjdXOEx5bW1IazdYSElWVnQrUFkwcDZW?=
 =?utf-8?B?TGZIcVZMYzVZZk5KNnBIS2gwWURBaTZrOTEyK1dWVzlMQzRYN3FrV3o1SkJS?=
 =?utf-8?B?L1hGdzNJbDJPcGxHT3N2WUlSREVDUDNmY3MyeUxNMERnUzJnU2RwdjhpYTI2?=
 =?utf-8?B?Z2dhZVl3VTg4bVY1ZTVibHNlSnJoT21WVzNEblh6cjFkRmZjSXhheWcvMnFx?=
 =?utf-8?B?bFovdXhmMVNjNUx4WnVJZ0hkYStZV1RoU1dzYmNQa3k5NityczBKMDBER1Fa?=
 =?utf-8?B?NVc5L0g4aFpid1pqZGVEYWkyR1FTWXJuZnhWb1JDRzY0NUExN3ZWRmUzb1pP?=
 =?utf-8?B?QmNzeFNZZFE1dTZQSnR0bzJHc0lvSDdld0RuYWVNZjNMeHlUTUNWYSs3bjFT?=
 =?utf-8?B?dTNCK01DemdjTGVhUjJjWHZTRlVmZzhtWVFoS1lVcHlEOHZ4dW5zUk9DcGVr?=
 =?utf-8?B?dnFZVjVvVExHYktQdEkyTThhdVlSRFp1clN6RUY3QUdmc3NUUHJNNnF1Wloy?=
 =?utf-8?B?M0tENXZXV29HOGEvQjRkdDB5MElYVkpXSXlQeXAxbWtxbld4azVqNGNsQU9x?=
 =?utf-8?B?em83SUNYR0Jya0tlL2ZSYnF4NzFqenM5dEJwdndQaG0wdFkyMnUxTVJZZ05k?=
 =?utf-8?B?ZmpkRHlxRGNOU3AyenU1ajE4YWU1cWtEaGZ2NXNCU1hRQ3F6WS9FazdVMVVG?=
 =?utf-8?B?YTRtTk8ySUh4K2dpYnc5NE9kWFVKYk9RWWlSUFBLejd6MHY1cXBESlcrNGxP?=
 =?utf-8?B?anJ1a1U4aldoemtHRTg2SnZGTlR5ZTViQmxsN24wbUxneTJWcVVOc0JFQUZ5?=
 =?utf-8?B?NDhEQkpWSDlDVkZMRVp2SlErRVVqU01LU2VlTm43ZEJaMzNubGs0VnVmTFd0?=
 =?utf-8?B?RFF5Z0lWNTA0ZlRZdjkzWmN5bkJIRXowY0dDYUxodldlWnpIQWM4UlRRc0FV?=
 =?utf-8?B?ejN1aEpDOFREUkZiekQxdzl0T09GRE80VE1nc3I4RlpoQjNiT0JWQzl1bXlD?=
 =?utf-8?B?b1BvN2lrb0VKbTR5aVZSVWpzdkErVjBqNHVSMHI5SmRIcEM4TGRNcXhRQVZ0?=
 =?utf-8?B?MjdaMmtkMmp5NFhwRlUzVU9OaVlJN3hETncyM2tXZHZEVGxhYmxseEZkQUdB?=
 =?utf-8?B?SkVJRmtMK2J4VlA3OVZTMnI5RVFKTE1RU2hleHE0MXl6ZlFZSXJZbTlOMUZM?=
 =?utf-8?B?d3BvQ1lSdk1zQ2NnMlowQWJaVFZYZlVqSkhSd3A3V3lHU0FxSTNsaXB4S2RX?=
 =?utf-8?B?Uk1aUzk0ajc0TnBSTENubkh0ZURKNjVHanArWmp0M3RWNzNEVTNyZjlDMnM1?=
 =?utf-8?B?eU5WOE9HL3ZRenZoeVU1NnYrY3VtWDlLam9LRVZ2VlZJMVpzOWMzeTJNUUtM?=
 =?utf-8?B?czRjcjN4dy9mazkxcXJ0b0hHcWdyNU5NVFNDd243NEtnWTY3M3B6ZkxMemxM?=
 =?utf-8?B?WnBsWHVnclZleUFxem5tbkZDZ3hRdExpUHAxVjlvM0RadmIvc1RWOXY2QW90?=
 =?utf-8?B?dTZEbkY1SmJxeGtiakI5OXlCbTBWcUg5ZU01cHluejg1eTUrcHFBOE9CbnNm?=
 =?utf-8?B?T0JZLytCMklZaVlQSHVwMHlmMFpwZVNMM1J1QVBYU3p5OGZzZUIzZWNNSzVI?=
 =?utf-8?B?OTBPZHphNGpEeE1GMjZQaDYxQnBMZEtHcEcwS0FtNlo5TVF6RjF4d0I5NS9M?=
 =?utf-8?B?d29uMFFqRmdzWW5JU1NKZ2hrN0JaZFRhMkVlLzZoWnVDSm41NzU2UnIrWEpv?=
 =?utf-8?B?VHc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 47b6dd38-b263-490b-ac79-08db3bae40cb
X-MS-Exchange-CrossTenant-AuthSource: CO1PR11MB5089.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 Apr 2023 23:33:00.2629
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: xgqky70DVfCDdrclfX6T6kzWvFIpDu3FMVTQsSX6TlMQJSpDscP5JAyT42buaGgt2I56rruZ4Ulv6RR67mYrf64hvw5ioAu7rvbtYDgj9Rw=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ2PR11MB7670
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-5.5 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_MED,SPF_HELO_NONE,SPF_NONE,URIBL_BLOCKED autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 4/11/2023 3:51 AM, Niklas Schnelle wrote:
> After an error on the PCI link, the driver does not need to wait
> for the link to become functional again as a reset is required. Stop
> the wait loop in this case to accelerate the recovery flow.
> 

Ok, so if the PCI link is completely offline (pci_channel_offline) then
we just bail out immediately and fail to recover, reporting to the user
as-such. Then a system administrator can setup in and perform the
appropriate reset? Rather than not reporting until the timeout
completes. Essentially, we know that this will never recover at this
point so stop wasting time.

Makes sense.

> Co-developed-by: Alexander Schmidt <alexs@linux.ibm.com>
> Signed-off-by: Alexander Schmidt <alexs@linux.ibm.com>
> Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
> Link: https://lore.kernel.org/r/20230403075657.168294-1-schnelle@linux.ibm.com
> Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
> ---

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

>  drivers/net/ethernet/mellanox/mlx5/core/health.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/health.c b/drivers/net/ethernet/mellanox/mlx5/core/health.c
> index f9438d4e43ca..81ca44e0705a 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/health.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/health.c
> @@ -325,6 +325,8 @@ int mlx5_health_wait_pci_up(struct mlx5_core_dev *dev)
>  	while (sensor_pci_not_working(dev)) {
>  		if (time_after(jiffies, end))
>  			return -ETIMEDOUT;
> +		if (pci_channel_offline(dev->pdev))
> +			return -EIO;
>  		msleep(100);
>  	}
>  	return 0;
> @@ -332,10 +334,16 @@ int mlx5_health_wait_pci_up(struct mlx5_core_dev *dev)
>  
>  static int mlx5_health_try_recover(struct mlx5_core_dev *dev)
>  {
> +	int rc;
> +
>  	mlx5_core_warn(dev, "handling bad device here\n");
>  	mlx5_handle_bad_state(dev);
> -	if (mlx5_health_wait_pci_up(dev)) {
> -		mlx5_core_err(dev, "health recovery flow aborted, PCI reads still not working\n");
> +	rc = mlx5_health_wait_pci_up(dev);
> +	if (rc) {
> +		if (rc == -ETIMEDOUT)
> +			mlx5_core_err(dev, "health recovery flow aborted, PCI reads still not working\n");
> +		else
> +			mlx5_core_err(dev, "health recovery flow aborted, PCI channel offline\n");
>  		return -EIO;
>  	}
>  	mlx5_core_err(dev, "starting health recovery flow\n");
> 
> base-commit: 09a9639e56c01c7a00d6c0ca63f4c7c41abe075d