Re: EEH recovery failing on mlx5 card

Re: EEH recovery failing on mlx5 card

[Leon Romanovsky]

+ Moshe

On Mon, Jul 17, 2023 at 12:48:37PM +0530, Ganesh G R wrote:
> Hi,
> 
> mlx5 cards are failing to recover from PCI errors, Upon investigation we found that the
> driver is trying to do MMIO in the middle of EEH error handling.
> The following fix in mlx5_pci_err_detected() is fixing the issue, Do you think its the right fix?
> 
> @@ -1847,6 +1847,7 @@ static pci_ers_result_t mlx5_pci_err_detected(struct pci_dev *pdev,
>         mlx5_unload_one(dev, true);
>         mlx5_drain_health_wq(dev);
>         mlx5_pci_disable_device(dev);
> +       cancel_delayed_work_sync(&clock->timer.overflow_work);
>         res = state == pci_channel_io_perm_failure ?
>                 PCI_ERS_RESULT_DISCONNECT : PCI_ERS_RESULT_NEED_RESET;
> 
> Regards
> Ganesh

Re: EEH recovery failing on mlx5 card

[Moshe Shemesh]

On 7/17/2023 4:10 PM, Leon Romanovsky wrote:
> 
> + Moshe
> 
> On Mon, Jul 17, 2023 at 12:48:37PM +0530, Ganesh G R wrote:
>> Hi,
>>
>> mlx5 cards are failing to recover from PCI errors, Upon investigation we found that the
>> driver is trying to do MMIO in the middle of EEH error handling.
>> The following fix in mlx5_pci_err_detected() is fixing the issue, Do you think its the right fix?
>>
>> @@ -1847,6 +1847,7 @@ static pci_ers_result_t mlx5_pci_err_detected(struct pci_dev *pdev,
>>          mlx5_unload_one(dev, true);
>>          mlx5_drain_health_wq(dev);
>>          mlx5_pci_disable_device(dev);
>> +       cancel_delayed_work_sync(&clock->timer.overflow_work);
>>          res = state == pci_channel_io_perm_failure ?
>>                  PCI_ERS_RESULT_DISCONNECT : PCI_ERS_RESULT_NEED_RESET;
>>

Hi Ganesh,
Thanks for pointing to this issue and its solution!
I would rather fix it in the work itself.
Please test this fix instead :

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c 
b/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
index 973babfaff25..2ad0bcc0f1b1 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
@@ -227,10 +227,15 @@ static void mlx5_timestamp_overflow(struct 
work_struct *work)
         clock = container_of(timer, struct mlx5_clock, timer);
         mdev = container_of(clock, struct mlx5_core_dev, clock);

+       if (mdev->state == MLX5_DEVICE_STATE_INTERNAL_ERROR)
+              goto out;
+
         write_seqlock_irqsave(&clock->lock, flags);
         timecounter_read(&timer->tc);
         mlx5_update_clock_info_page(mdev);
         write_sequnlock_irqrestore(&clock->lock, flags);
+
+out:
         schedule_delayed_work(&timer->overflow_work, 
timer->overflow_period);
  }


>> Regards
>> Ganesh

Re: EEH recovery failing on mlx5 card

[Moshe Shemesh]

On 7/18/2023 9:47 PM, Ganesh G R wrote:
> 
> On 7/17/23 9:48 PM, Moshe Shemesh wrote:
> 
>> On 7/17/2023 4:10 PM, Leon Romanovsky wrote:
>>> + Moshe
>>> On Mon, Jul 17, 2023 at 12:48:37PM +0530, Ganesh G R wrote:
>>>> Hi,
>>>> mlx5 cards are failing to recover from PCI errors, Upon investigation we found that the
>>>> driver is trying to do MMIO in the middle of EEH error handling.
>>>> The following fix in mlx5_pci_err_detected() is fixing the issue, Do you think its the right fix?
>>>> @@ -1847,6 +1847,7 @@ static pci_ers_result_t mlx5_pci_err_detected(struct pci_dev *pdev,
>>>>           mlx5_unload_one(dev, true);
>>>>           mlx5_drain_health_wq(dev);
>>>>           mlx5_pci_disable_device(dev);
>>>> +       cancel_delayed_work_sync(&clock->timer.overflow_work);
>>>>           res = state == pci_channel_io_perm_failure ?
>>>>                   PCI_ERS_RESULT_DISCONNECT : PCI_ERS_RESULT_NEED_RESET;
>> Hi Ganesh,
>> Thanks for pointing to this issue and its solution!
>> I would rather fix it in the work itself.
>> Please test this fix instead :
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
>> b/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
>> index 973babfaff25..2ad0bcc0f1b1 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
>> @@ -227,10 +227,15 @@ static void mlx5_timestamp_overflow(struct
>> work_struct *work)
>>          clock = container_of(timer, struct mlx5_clock, timer);
>>          mdev = container_of(clock, struct mlx5_core_dev, clock);
>> +       if (mdev->state == MLX5_DEVICE_STATE_INTERNAL_ERROR)
>> +              goto out;
>> +
>>          write_seqlock_irqsave(&clock->lock, flags);
>>          timecounter_read(&timer->tc);
>>          mlx5_update_clock_info_page(mdev);
>>          write_sequnlock_irqrestore(&clock->lock, flags);
>> +
>> +out:
>>          schedule_delayed_work(&timer->overflow_work,
>> timer->overflow_period);
>>   }
> Thanks Moshe, The fix looks clean and It worked.
> 

Great, thank you !