Re: [PATCH v3 10/16] PCI/DOE: Deduplicate mailbox flushing

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Fri, 10 Feb 2023 21:25:10 +0100
Lukas Wunner <lukas@wunner.de> wrote:

> When a DOE mailbox is torn down, its workqueue is flushed once in
> pci_doe_flush_mb() through a call to flush_workqueue() and subsequently
> flushed once more in pci_doe_destroy_workqueue() through a call to
> destroy_workqueue().
> 
> Deduplicate by dropping flush_workqueue() from pci_doe_flush_mb().
> 
> Rename pci_doe_flush_mb() to pci_doe_cancel_tasks() to more aptly
> describe what it now does.
> 
> Tested-by: Ira Weiny <ira.weiny@intel.com>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
LGTM

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
>  Changes v2 -> v3:
>  * Newly added patch in v3
> 
>  drivers/pci/doe.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/doe.c b/drivers/pci/doe.c
> index afb53bc1b4aa..291cd7a46a39 100644
> --- a/drivers/pci/doe.c
> +++ b/drivers/pci/doe.c
> @@ -429,7 +429,7 @@ static void pci_doe_destroy_workqueue(void *mb)
>  	destroy_workqueue(doe_mb->work_queue);
>  }
>  
> -static void pci_doe_flush_mb(void *mb)
> +static void pci_doe_cancel_tasks(void *mb)
>  {
>  	struct pci_doe_mb *doe_mb = mb;
>  
> @@ -439,9 +439,6 @@ static void pci_doe_flush_mb(void *mb)
>  	/* Cancel an in progress work item, if necessary */
>  	set_bit(PCI_DOE_FLAG_CANCEL, &doe_mb->flags);
>  	wake_up(&doe_mb->wq);
> -
> -	/* Flush all work items */
> -	flush_workqueue(doe_mb->work_queue);
>  }
>  
>  /**
> @@ -498,9 +495,9 @@ struct pci_doe_mb *pcim_doe_create_mb(struct pci_dev *pdev, u16 cap_offset)
>  
>  	/*
>  	 * The state machine and the mailbox should be in sync now;
> -	 * Set up mailbox flush prior to using the mailbox to query protocols.
> +	 * Set up cancel tasks prior to using the mailbox to query protocols.
>  	 */
> -	rc = devm_add_action_or_reset(dev, pci_doe_flush_mb, doe_mb);
> +	rc = devm_add_action_or_reset(dev, pci_doe_cancel_tasks, doe_mb);
>  	if (rc)
>  		return ERR_PTR(rc);
>