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

[Frederic Barrat <fbarrat@linux.ibm.com>]

> diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
> index c79ba1c699ad..28c28bceb063 100644
> --- a/drivers/misc/cxl/pci.c
> +++ b/drivers/misc/cxl/pci.c

> @@ -1932,14 +1935,20 @@ static pci_ers_result_t cxl_pci_error_detected(struct pci_dev *pdev,
>   	 *     * In slot_reset, free the old resources and allocate new ones.
>   	 *     * In resume, clear the flag to allow things to start.
>   	 */
> +
> +	/* Make sure no one else changes the afu list */
> +	spin_lock(&adapter->afu_list_lock);
> +
>   	for (i = 0; i < adapter->slices; i++) {
>   		afu = adapter->afu[i];
>   
>   		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.


> @@ -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 ...

   Fred