Re: [PATCH v2] Bugfix: powerpc/eeh: Wrong place to call pci_get_slot()

[Mike Qiu <qiudayu@linux.vnet.ibm.com>]

On 07/15/2014 01:07 PM, Gavin Shan wrote:
> On Mon, Jul 14, 2014 at 10:33:48PM -0400, Mike Qiu wrote:
>> pci_get_slot() is called with hold of PCI bus semaphore and it's not
>> safe to be called in interrupt context. However, we possibly checks
>> EEH error and calls the function in interrupt context. To avoid using
>> pci_get_slot(), we turn into device tree for fetching location code.
>> Otherwise, we might run into WARN_ON() as following messages indicate:
>>
>> WARNING: at drivers/pci/search.c:223
>> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 3.16.0-rc3+ #72
>> task: c000000001367af0 ti: c000000001444000 task.ti: c000000001444000
>> NIP: c000000000497b70 LR: c000000000037530 CTR: 000000003003d114
>> REGS: c000000001446fa0 TRAP: 0700   Not tainted  (3.16.0-rc3+)
>> MSR: 9000000000029032 <SF,HV,EE,ME,IR,DR,RI>  CR: 48002422  XER: 20000000
>> CFAR: c00000000003752c SOFTE: 0
>>    :
>> NIP [c000000000497b70] .pci_get_slot+0x40/0x110
>> LR [c000000000037530] .eeh_pe_loc_get+0x150/0x190
>> Call Trace:
>>    .of_get_property+0x30/0x60 (unreliable)
>>    .eeh_pe_loc_get+0x150/0x190
>>    .eeh_dev_check_failure+0x1b4/0x550
>>    .eeh_check_failure+0x90/0xf0
>>    .lpfc_sli_check_eratt+0x504/0x7c0 [lpfc]
>>    .lpfc_poll_eratt+0x64/0x100 [lpfc]
>>    .call_timer_fn+0x64/0x190
>>    .run_timer_softirq+0x2cc/0x3e0
>>
>> Signed-off-by: Mike Qiu <qiudayu@linux.vnet.ibm.com>
>> ---
>> Changelog[v2]:
>> 	Check the child device_node of root bus for root port
>> 	directly instead of search pdev from device-tree and
>> 	then translate it to device-node
>>
> I run into following warning with your patch. Please test the attached
> one. If no problem found, send that one please.
>
> arch/powerpc/kernel/eeh_pe.c: In function 'eeh_pe_loc_get':
> arch/powerpc/kernel/eeh_pe.c:806:18: warning: unused variable 'pdev'

OK, I will remove unused variable.

Thanks,
Mike
> Thanks,
> Gavin
>
>> arch/powerpc/kernel/eeh_pe.c | 24 ++++++------------------
>> 1 file changed, 6 insertions(+), 18 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
>> index fbd01eb..f96c10f 100644
>> --- a/arch/powerpc/kernel/eeh_pe.c
>> +++ b/arch/powerpc/kernel/eeh_pe.c
>> @@ -802,7 +802,6 @@ void eeh_pe_restore_bars(struct eeh_pe *pe)
>>   */
>> const char *eeh_pe_loc_get(struct eeh_pe *pe)
>> {
>> -	struct pci_controller *hose;
>> 	struct pci_bus *bus = eeh_pe_bus_get(pe);
>> 	struct pci_dev *pdev;
>> 	struct device_node *dn;
>> @@ -811,29 +810,20 @@ const char *eeh_pe_loc_get(struct eeh_pe *pe)
>> 	if (!bus)
>> 		return "N/A";
>>
>> +	dn = pci_bus_to_OF_node(bus);
>> 	/* PHB PE or root PE ? */
>> -	if (pci_is_root_bus(bus)) {
>> -		hose = pci_bus_to_host(bus);
>> -		loc = of_get_property(hose->dn,
>> -				"ibm,loc-code", NULL);
>> +	if (dn && pci_is_root_bus(bus)) {
>> +		loc = of_get_property(dn, "ibm,loc-code", NULL);
>> 		if (loc)
>> 			return loc;
>> -		loc = of_get_property(hose->dn,
>> -				"ibm,io-base-loc-code", NULL);
>> +		loc = of_get_property(dn, "ibm,io-base-loc-code", NULL);
>> 		if (loc)
>> 			return loc;
>>
>> -		pdev = pci_get_slot(bus, 0x0);
>> -	} else {
>> -		pdev = bus->self;
>> -	}
>> -
>> -	if (!pdev) {
>> -		loc = "N/A";
>> -		goto out;
>> +		/* Check the root port */
>> +		dn = dn->child;
>> 	}
>>
>> -	dn = pci_device_to_OF_node(pdev);
>> 	if (!dn) {
>> 		loc = "N/A";
>> 		goto out;
>> @@ -846,8 +836,6 @@ const char *eeh_pe_loc_get(struct eeh_pe *pe)
>> 		loc = "N/A";
>>
>> out:
>> -	if (pci_is_root_bus(bus) && pdev)
>> -		pci_dev_put(pdev);
>> 	return loc;
>> }
>>
>> -- 
>> 1.8.1.4
>>