Re: [PATCH v5] PCI hotplug: rpaphp: Error out on busy status from get-sensor-state

[Michael Ellerman <mpe@ellerman.id.au>]

Nathan Lynch <nathanl@linux.ibm.com> writes:
> Mahesh Salgaonkar <mahesh@linux.ibm.com> writes:
>> When certain PHB HW failure causes phyp to recover PHB, it marks the PE
>> state as temporarily unavailable until recovery is complete. This also
>> triggers an EEH handler in Linux which needs to notify drivers, and perform
>> recovery. But before notifying the driver about the pci error it uses
                                                       PCI

>> get_adapter_state()->get-sesnor-state() operation of the hotplug_slot to
                              ^
                              typo

>> determine if the slot contains a device or not. if the slot is empty, the
>> recovery is skipped entirely.
>>
>> However on certain PHB failures, the rtas call get-sesnor-state() returns
                                                        ^
                                                        typo

>> extended busy error (9902) until PHB is recovered by phyp. Once PHB is
>> recovered, the get-sensor-state() returns success with correct presence
>> status. The rtas call interface rtas_get_sensor() loops over the rtas call
               RTAS

>> on extended delay return code (9902) until the return value is either
>> success (0) or error (-1). This causes the EEH handler to get stuck for ~6
>> seconds before it could notify that the pci error has been detected and
>> stop any active operations. Hence with running I/O traffic, during this 6
>> seconds, the network driver continues its operation and hits a timeout
>> (netdev watchdog). On timeouts, network driver go into ffdc capture mode
>> and reset path assuming the PCI device is in fatal condition. This
>> sometimes causes EEH recovery to fail. This impacts the ssh connection and
>> leads to the system being inaccessible.
>>
>> ------------
>> [52732.244731] DEBUG: ibm_read_slot_reset_state2()
>> [52732.244762] DEBUG: ret = 0, rets[0]=5, rets[1]=1, rets[2]=4000, rets[3]=>
>> [52732.244798] DEBUG: in eeh_slot_presence_check
>> [52732.244804] DEBUG: error state check
>> [52732.244807] DEBUG: Is slot hotpluggable
>> [52732.244810] DEBUG: hotpluggable ops ?
>> [52732.244953] DEBUG: Calling ops->get_adapter_status
>> [52732.244958] DEBUG: calling rpaphp_get_sensor_state
>> [52736.564262] ------------[ cut here ]------------
>> [52736.564299] NETDEV WATCHDOG: enP64p1s0f3 (tg3): transmit queue 0 timed o>
>> [52736.564324] WARNING: CPU: 1442 PID: 0 at net/sched/sch_generic.c:478 dev>
>> [...]
>> [52736.564505] NIP [c000000000c32368] dev_watchdog+0x438/0x440
>> [52736.564513] LR [c000000000c32364] dev_watchdog+0x434/0x440
>> ------------
>>
>> To avoid this issue, fix the pci hotplug driver (rpaphp) to return an error
>> if the slot presence state can not be detected immediately while pe is in
                                                                    PE
>> EEH recovery state. Current implementation uses rtas_get_sensor() API which
>> blocks the slot check state until rtas call returns success. Change
>> rpaphp_get_sensor_state() to invoke rtas_call(get-sensor-state) directly
>> only if the respective pe is in EEH recovery state, and take actions based
>> on rtas return status.
>>
>> In normal cases (non-EEH case) rpaphp_get_sensor_state() will continue to
>> invoke rtas_get_sensor() as it was earlier with no change in existing
>> behavior.
>>
>> Signed-off-by: Mahesh Salgaonkar <mahesh@linux.ibm.com>
>>
>> ---
>> Change in v5:
>> - Fixup #define macros with parentheses around the values.
>>
>> Change in V4:
>> - Error out on sensor busy only if pe is going through EEH recovery instead
>>   of always error out.
>>
>> Change in V3:
>> - Invoke rtas_call(get-sensor-state) directly from
>>   rpaphp_get_sensor_state() directly and do special handling.
>> - See v2 at
>>   https://lists.ozlabs.org/pipermail/linuxppc-dev/2021-November/237336.html
>>
>> Change in V2:
>> - Alternate approach to fix the EEH issue instead of delaying slot presence
>>   check proposed at
>>   https://lists.ozlabs.org/pipermail/linuxppc-dev/2021-November/236956.html
>>
>> Also refer:
>> https://lists.ozlabs.org/pipermail/linuxppc-dev/2021-November/237027.html
>
> Sorry for the long delay. I think it looks OK now. Does this need to go
> to the PCI list/maintainer?

Yes it needs to be Cc'ed to the PCI list/maintainer, even if we end up
merging it via the powerpc tree.

cheers