[6/7] drm/amdkfd: Reuse function to find a process through pasid

Submitted by Edward O'Callaghan on Sept. 10, 2016, 1:31 a.m.

Details

Message ID 1473471099-16914-7-git-send-email-funfunctor@folklore1984.net
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in AMD X.Org drivers

Not browsing as part of any series.

Commit Message

Edward O'Callaghan Sept. 10, 2016, 1:31 a.m.
The kfd_lookup_process_by_pasid() is just for that purpose,
so use it instead of repeating the code.

Signed-off-by: Edward O'Callaghan <funfunctor@folklore1984.net>
---
 drivers/gpu/drm/amd/amdkfd/kfd_process.c | 58 +++++++++++++-------------------
 1 file changed, 23 insertions(+), 35 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index ef2266a..a94dddc 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -404,56 +404,44 @@  void kfd_unbind_process_from_device(struct kfd_dev *dev, unsigned int pasid)
 {
 	struct kfd_process *p;
 	struct kfd_process_device *pdd;
-	int idx, i;
 
 	BUG_ON(dev == NULL);
 
-	idx = srcu_read_lock(&kfd_processes_srcu);
-
 	/*
 	 * Look for the process that matches the pasid. If there is no such
 	 * process, we either released it in amdkfd's own notifier, or there
 	 * is a bug. Unfortunately, there is no way to tell...
 	 */
-	hash_for_each_rcu(kfd_processes_table, i, p, kfd_processes)
-		if (p->pasid == pasid) {
+	p = kfd_lookup_process_by_pasid(pasid);
+	BUG_ON(!p);
 
-			srcu_read_unlock(&kfd_processes_srcu, idx);
+	pr_debug("Unbinding process %d from IOMMU\n", pasid);
 
-			pr_debug("Unbinding process %d from IOMMU\n", pasid);
-
-			mutex_lock(&p->mutex);
+	if ((dev->dbgmgr) && (dev->dbgmgr->pasid == p->pasid))
+		kfd_dbgmgr_destroy(dev->dbgmgr);
 
-			if ((dev->dbgmgr) && (dev->dbgmgr->pasid == p->pasid))
-				kfd_dbgmgr_destroy(dev->dbgmgr);
-
-			pdd = kfd_get_process_device_data(dev, p);
-
-			if (!pdd) {
-				mutex_unlock(&p->mutex);
-				return;
-			}
-
-			if (pdd->reset_wavefronts) {
-				dbgdev_wave_reset_wavefronts(pdd->dev, p);
-				pdd->reset_wavefronts = false;
-			}
+	pdd = kfd_get_process_device_data(dev, p);
 
-			/*
-			 * Just mark pdd as unbound, because we still need it
-			 * to call amd_iommu_unbind_pasid() in when the
-			 * process exits.
-			 * We don't call amd_iommu_unbind_pasid() here
-			 * because the IOMMU called us.
-			 */
-			pdd->bound = false;
+	if (!pdd) {
+		mutex_unlock(&p->mutex);
+		return;
+	}
 
-			mutex_unlock(&p->mutex);
+	if (pdd->reset_wavefronts) {
+		dbgdev_wave_reset_wavefronts(pdd->dev, p);
+		pdd->reset_wavefronts = false;
+	}
 
-			return;
-		}
+	/*
+	 * Just mark pdd as unbound, because we still need it
+	 * to call amd_iommu_unbind_pasid() in when the
+	 * process exits.
+	 * We don't call amd_iommu_unbind_pasid() here
+	 * because the IOMMU called us.
+	 */
+	pdd->bound = false;
 
-	srcu_read_unlock(&kfd_processes_srcu, idx);
+	mutex_unlock(&p->mutex);
 }
 
 struct kfd_process_device *kfd_get_first_process_device_data(struct kfd_process *p)

Comments

On Sat, Sep 10, 2016 at 4:31 AM, Edward O'Callaghan
<funfunctor@folklore1984.net> wrote:
> The kfd_lookup_process_by_pasid() is just for that purpose,
> so use it instead of repeating the code.
>
> Signed-off-by: Edward O'Callaghan <funfunctor@folklore1984.net>
> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_process.c | 58 +++++++++++++-------------------
>  1 file changed, 23 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> index ef2266a..a94dddc 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> @@ -404,56 +404,44 @@ void kfd_unbind_process_from_device(struct kfd_dev *dev, unsigned int pasid)
>  {
>         struct kfd_process *p;
>         struct kfd_process_device *pdd;
> -       int idx, i;
>
>         BUG_ON(dev == NULL);
>
> -       idx = srcu_read_lock(&kfd_processes_srcu);
> -
>         /*
>          * Look for the process that matches the pasid. If there is no such
>          * process, we either released it in amdkfd's own notifier, or there
>          * is a bug. Unfortunately, there is no way to tell...
>          */
> -       hash_for_each_rcu(kfd_processes_table, i, p, kfd_processes)
> -               if (p->pasid == pasid) {
> +       p = kfd_lookup_process_by_pasid(pasid);
> +       BUG_ON(!p);
Don't BUG_ON here. Look at the comment above, there is a valid
scenario where p might be NULL. If p is NULL, just exit the function.

>
> -                       srcu_read_unlock(&kfd_processes_srcu, idx);
> +       pr_debug("Unbinding process %d from IOMMU\n", pasid);
>
> -                       pr_debug("Unbinding process %d from IOMMU\n", pasid);
> -
> -                       mutex_lock(&p->mutex);
> +       if ((dev->dbgmgr) && (dev->dbgmgr->pasid == p->pasid))
> +               kfd_dbgmgr_destroy(dev->dbgmgr);
>
> -                       if ((dev->dbgmgr) && (dev->dbgmgr->pasid == p->pasid))
> -                               kfd_dbgmgr_destroy(dev->dbgmgr);
> -
> -                       pdd = kfd_get_process_device_data(dev, p);
> -
> -                       if (!pdd) {
> -                               mutex_unlock(&p->mutex);
> -                               return;
> -                       }
> -
> -                       if (pdd->reset_wavefronts) {
> -                               dbgdev_wave_reset_wavefronts(pdd->dev, p);
> -                               pdd->reset_wavefronts = false;
> -                       }
> +       pdd = kfd_get_process_device_data(dev, p);
>
> -                       /*
> -                        * Just mark pdd as unbound, because we still need it
> -                        * to call amd_iommu_unbind_pasid() in when the
> -                        * process exits.
> -                        * We don't call amd_iommu_unbind_pasid() here
> -                        * because the IOMMU called us.
> -                        */
> -                       pdd->bound = false;
> +       if (!pdd) {
> +               mutex_unlock(&p->mutex);
> +               return;
> +       }
>
> -                       mutex_unlock(&p->mutex);
> +       if (pdd->reset_wavefronts) {
> +               dbgdev_wave_reset_wavefronts(pdd->dev, p);
> +               pdd->reset_wavefronts = false;
> +       }
>
> -                       return;
> -               }
> +       /*
> +        * Just mark pdd as unbound, because we still need it
> +        * to call amd_iommu_unbind_pasid() in when the
> +        * process exits.
> +        * We don't call amd_iommu_unbind_pasid() here
> +        * because the IOMMU called us.
> +        */
> +       pdd->bound = false;
>
> -       srcu_read_unlock(&kfd_processes_srcu, idx);
> +       mutex_unlock(&p->mutex);
>  }
>
>  struct kfd_process_device *kfd_get_first_process_device_data(struct kfd_process *p)
> --
> 2.7.4
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Assuming the above comment is fixed, this patch is:
Reviewed-by: Oded Gabbay <oded.gabbay@gmail.com>