On Sat, Sep 10, 2016 at 4:31 AM, Edward O'Callaghan
<funfunc...@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 <funfunc...@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.gab...@gmail.com>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
