[02/11] powerpc/powernv/ioda: Protect PE list
diff mbox series

Message ID 20190909154600.19917-3-fbarrat@linux.ibm.com
State New
Headers show
Series
  • opencapi: enable card reset and link retraining
Related show

Checks

Context Check Description
snowpatch_ozlabs/checkpatch success total: 0 errors, 0 warnings, 0 checks, 20 lines checked
snowpatch_ozlabs/apply_patch success Successfully applied on branch next (c317052c95bef1f977b023158e5aa929215f443d)

Commit Message

Frederic Barrat Sept. 9, 2019, 3:45 p.m. UTC
Protect the PHB's list of PE. Probably not needed as long as it was
populated during PHB creation, but it feels right and will become
required once we can add/remove opencapi devices on hotplug.

Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
---
 arch/powerpc/platforms/powernv/pci-ioda.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Alastair D'Silva Sept. 10, 2019, 12:34 a.m. UTC | #1
On Mon, 2019-09-09 at 17:45 +0200, Frederic Barrat wrote:
> Protect the PHB's list of PE. Probably not needed as long as it was
> populated during PHB creation, but it feels right and will become
> required once we can add/remove opencapi devices on hotplug.
> 
> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
> ---
>  arch/powerpc/platforms/powernv/pci-ioda.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c
> b/arch/powerpc/platforms/powernv/pci-ioda.c
> index 92767f006f20..3dbbf5365c1c 100644
> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
> @@ -1080,8 +1080,9 @@ static struct pnv_ioda_pe
> *pnv_ioda_setup_dev_PE(struct pci_dev *dev)
>  	}
>  
>  	/* Put PE to the list */
> +	mutex_lock(&phb->ioda.pe_list_mutex);
>  	list_add_tail(&pe->list, &phb->ioda.pe_list);
> -
> +	mutex_unlock(&phb->ioda.pe_list_mutex);
>  	return pe;
>  }
>  
> @@ -3513,7 +3514,10 @@ static void pnv_ioda_release_pe(struct
> pnv_ioda_pe *pe)
>  	struct pnv_phb *phb = pe->phb;
>  	struct pnv_ioda_pe *slave, *tmp;
>  
> +	mutex_lock(&phb->ioda.pe_list_mutex);
>  	list_del(&pe->list);
> +	mutex_unlock(&phb->ioda.pe_list_mutex);
> +
>  	switch (phb->type) {
>  	case PNV_PHB_IODA1:
>  		pnv_pci_ioda1_release_pe_dma(pe);

Hmm, the ioda.pe_list_mutex muxtex exists, and is inited, but there are
no other users. It's position & naming in the struct suggests it
belongs to ioda.pe_list, rather than pnv_ioda_pe.list (as suggested by
the lock/unlock around the list del).

Do the other accessors of ioda.pe_list also need mutex protection?
pnv_ioda_setup_bus_PE()
pnv_pci_dma_bus_setup()
pnv_pci_init_ioda_phb()
pnv_pci_ioda_setup_PEs()

If not, perhaps the metux should be removed from ioda and replaced with
pe.list_mutex instead.

Patch
diff mbox series

diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index 92767f006f20..3dbbf5365c1c 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -1080,8 +1080,9 @@  static struct pnv_ioda_pe *pnv_ioda_setup_dev_PE(struct pci_dev *dev)
 	}
 
 	/* Put PE to the list */
+	mutex_lock(&phb->ioda.pe_list_mutex);
 	list_add_tail(&pe->list, &phb->ioda.pe_list);
-
+	mutex_unlock(&phb->ioda.pe_list_mutex);
 	return pe;
 }
 
@@ -3513,7 +3514,10 @@  static void pnv_ioda_release_pe(struct pnv_ioda_pe *pe)
 	struct pnv_phb *phb = pe->phb;
 	struct pnv_ioda_pe *slave, *tmp;
 
+	mutex_lock(&phb->ioda.pe_list_mutex);
 	list_del(&pe->list);
+	mutex_unlock(&phb->ioda.pe_list_mutex);
+
 	switch (phb->type) {
 	case PNV_PHB_IODA1:
 		pnv_pci_ioda1_release_pe_dma(pe);