[PATCH] cxl: Wrap iterations over afu slices inside 'afu_list_lock'

Vaibhav Jain vaibhav at linux.ibm.com
Sat Jan 26 22:48:43 AEDT 2019


Thanks for reviewing this patch Fred,

Frederic Barrat <fbarrat at linux.ibm.com> writes:

>>   
>>   		afu_result = cxl_vphb_error_detected(afu, state);
>>   
>> -		cxl_context_detach_all(afu);
>> -		cxl_ops->afu_deactivate_mode(afu, afu->current_mode);
>> -		pci_deconfigure_afu(afu);
>> +		if (afu != NULL) {
>> +			cxl_context_detach_all(afu);
>> +			cxl_ops->afu_deactivate_mode(afu, afu->current_mode);
>> +			pci_deconfigure_afu(afu);
>> +		}
>
> I can see you're also checking if cxl_vphb_error_detected() is called 
> with a NULL afu from within the function, but why not move the call to 
> cxl_vphb_error_detected() within that "if (afu != NULL)...  " statement? 
> Otherwise, it looks suspicious when reading the code.
Yes, agree. However this was triggerring gcc compile warning
'maybe-uninitialized' for 'afu_result', hence removed the call to
cxl_vphb_error_detected() outside the branch. Have fixed this in v2 with
an explicit initialization of 'afu_result'.

>
>
>> @@ -2051,10 +2067,11 @@ static void cxl_pci_resume(struct pci_dev *pdev)
>>   	 * This is not the place to be checking if everything came back up
>>   	 * properly, because there's no return value: do that in slot_reset.
>>   	 */
>> +	spin_lock(&adapter->afu_list_lock);
>>   	for (i = 0; i < adapter->slices; i++) {
>>   		afu = adapter->afu[i];
>>   
>> -		if (afu->phb == NULL)
>> +		if (afu || afu->phb == NULL)
>>   			continue;
>
>
> if (afu == NULL ...
Thanks for catching this. Have fixed this in v2.
-- 
Vaibhav Jain <vaibhav at linux.ibm.com>
Linux Technology Center, IBM India Pvt. Ltd.



More information about the Linuxppc-dev mailing list