[PATCH 1/6] drm/amdkfd: Only initialize sdma vm for sdma queues

[PATCH 1/6] drm/amdkfd: Only initialize sdma vm for sdma queues

[Zeng, Oak]

Don't do the same for compute queues

Change-Id: Id5f743ca10c2b761590bfe18cab2f802d3c04d2d
Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index c18355d..00ca3dd 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -1209,8 +1209,9 @@ static int create_queue_cpsch(struct device_queue_manager *dqm, struct queue *q,
 	 * updates the is_evicted flag but is a no-op otherwise.
 	 */
 	q->properties.is_evicted = !!qpd->evicted;
-
-	dqm->asic_ops.init_sdma_vm(dqm, q, qpd);
+	if (q->properties.type == KFD_QUEUE_TYPE_SDMA ||
+		q->properties.type == KFD_QUEUE_TYPE_SDMA_XGMI)
+		dqm->asic_ops.init_sdma_vm(dqm, q, qpd);
 	q->properties.tba_addr = qpd->tba_addr;
 	q->properties.tma_addr = qpd->tma_addr;
 	retval = mqd_mgr->init_mqd(mqd_mgr, &q->mqd, &q->mqd_mem_obj,
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 2/6] drm/amdkfd: Only load sdma mqd when queue is active

[Zeng, Oak]

Also calls load_mqd with current->mm struct. The mm
struct is used to read back user wptr of the queue.

Change-Id: I0f6d085878358dcd3a413054dbe61d1ca0fdf66d
Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index 00ca3dd..dab6ef4 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -995,8 +995,11 @@ static int create_sdma_queue_nocpsch(struct device_queue_manager *dqm,
 	if (retval)
 		goto out_deallocate_doorbell;
 
+	if (!q->properties.is_active)
+		return 0;
+
 	retval = mqd_mgr->load_mqd(mqd_mgr, q->mqd, 0, 0, &q->properties,
-				NULL);
+				current->mm);
 	if (retval)
 		goto out_uninit_mqd;
 
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 3/6] drm/amdkfd: Refactor create_queue_nocpsch

[Zeng, Oak]

This is prepare work to fix a circular lock dependency.
No logic change

Change-Id: I4e0ee918260e7780de972dd71f4ce787b4f6dde9
Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>
---
 .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c  | 171 +++++++--------------
 1 file changed, 57 insertions(+), 114 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index dab6ef4..3c042eb 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -42,10 +42,6 @@
 static int set_pasid_vmid_mapping(struct device_queue_manager *dqm,
 					unsigned int pasid, unsigned int vmid);
 
-static int create_compute_queue_nocpsch(struct device_queue_manager *dqm,
-					struct queue *q,
-					struct qcm_process_device *qpd);
-
 static int execute_queues_cpsch(struct device_queue_manager *dqm,
 				enum kfd_unmap_queues_filter filter,
 				uint32_t filter_param);
@@ -55,13 +51,14 @@ static int unmap_queues_cpsch(struct device_queue_manager *dqm,
 
 static int map_queues_cpsch(struct device_queue_manager *dqm);
 
-static int create_sdma_queue_nocpsch(struct device_queue_manager *dqm,
-					struct queue *q,
-					struct qcm_process_device *qpd);
-
 static void deallocate_sdma_queue(struct device_queue_manager *dqm,
 				struct queue *q);
 
+static inline void deallocate_hqd(struct device_queue_manager *dqm,
+				struct queue *q);
+static int allocate_hqd(struct device_queue_manager *dqm, struct queue *q);
+static int allocate_sdma_queue(struct device_queue_manager *dqm,
+				struct queue *q);
 static void kfd_process_hw_exception(struct work_struct *work);
 
 static inline
@@ -223,6 +220,9 @@ static int allocate_vmid(struct device_queue_manager *dqm,
 	/* invalidate the VM context after pasid and vmid mapping is set up */
 	kfd_flush_tlb(qpd_to_pdd(qpd));
 
+	dqm->dev->kfd2kgd->set_scratch_backing_va(
+		dqm->dev->kgd, qpd->sh_hidden_private_base, qpd->vmid);
+
 	return 0;
 }
 
@@ -269,6 +269,7 @@ static int create_queue_nocpsch(struct device_queue_manager *dqm,
 				struct queue *q,
 				struct qcm_process_device *qpd)
 {
+	struct mqd_manager *mqd_mgr;
 	int retval;
 
 	print_queue(q);
@@ -298,18 +299,41 @@ static int create_queue_nocpsch(struct device_queue_manager *dqm,
 	q->properties.tba_addr = qpd->tba_addr;
 	q->properties.tma_addr = qpd->tma_addr;
 
-	if (q->properties.type == KFD_QUEUE_TYPE_COMPUTE)
-		retval = create_compute_queue_nocpsch(dqm, q, qpd);
-	else if (q->properties.type == KFD_QUEUE_TYPE_SDMA ||
-			q->properties.type == KFD_QUEUE_TYPE_SDMA_XGMI)
-		retval = create_sdma_queue_nocpsch(dqm, q, qpd);
-	else
-		retval = -EINVAL;
+	mqd_mgr = dqm->mqd_mgrs[get_mqd_type_from_queue_type(
+			q->properties.type)];
+	if (q->properties.type == KFD_QUEUE_TYPE_COMPUTE) {
+		retval = allocate_hqd(dqm, q);
+		if (retval)
+			goto deallocate_vmid;
+		pr_debug("Loading mqd to hqd on pipe %d, queue %d\n",
+			q->pipe, q->queue);
+	} else if (q->properties.type == KFD_QUEUE_TYPE_SDMA ||
+		q->properties.type == KFD_QUEUE_TYPE_SDMA_XGMI) {
+		retval = allocate_sdma_queue(dqm, q);
+		if (retval)
+			goto deallocate_vmid;
+		dqm->asic_ops.init_sdma_vm(dqm, q, qpd);
+	}
 
-	if (retval) {
-		if (list_empty(&qpd->queues_list))
-			deallocate_vmid(dqm, qpd, q);
-		goto out_unlock;
+	retval = allocate_doorbell(qpd, q);
+	if (retval)
+		goto out_deallocate_hqd;
+
+	retval = mqd_mgr->init_mqd(mqd_mgr, &q->mqd, &q->mqd_mem_obj,
+				&q->gart_mqd_addr, &q->properties);
+	if (retval)
+		goto out_deallocate_doorbell;
+
+	if (q->properties.is_active) {
+
+		if (WARN(q->process->mm != current->mm,
+					"should only run in user thread"))
+			retval = -EFAULT;
+		else
+			retval = mqd_mgr->load_mqd(mqd_mgr, q->mqd, q->pipe,
+					q->queue, &q->properties, current->mm);
+		if (retval)
+			goto out_uninit_mqd;
 	}
 
 	list_add(&q->list, &qpd->queues_list);
@@ -329,7 +353,21 @@ static int create_queue_nocpsch(struct device_queue_manager *dqm,
 	dqm->total_queue_count++;
 	pr_debug("Total of %d queues are accountable so far\n",
 			dqm->total_queue_count);
+	goto out_unlock;
 
+out_uninit_mqd:
+	mqd_mgr->uninit_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
+out_deallocate_doorbell:
+	deallocate_doorbell(qpd, q);
+out_deallocate_hqd:
+	if (q->properties.type == KFD_QUEUE_TYPE_COMPUTE)
+		deallocate_hqd(dqm, q);
+	else if (q->properties.type == KFD_QUEUE_TYPE_SDMA ||
+		q->properties.type == KFD_QUEUE_TYPE_SDMA_XGMI)
+		deallocate_sdma_queue(dqm, q);
+deallocate_vmid:
+	if (list_empty(&qpd->queues_list))
+		deallocate_vmid(dqm, qpd, q);
 out_unlock:
 	dqm_unlock(dqm);
 	return retval;
@@ -375,58 +413,6 @@ static inline void deallocate_hqd(struct device_queue_manager *dqm,
 	dqm->allocated_queues[q->pipe] |= (1 << q->queue);
 }
 
-static int create_compute_queue_nocpsch(struct device_queue_manager *dqm,
-					struct queue *q,
-					struct qcm_process_device *qpd)
-{
-	struct mqd_manager *mqd_mgr;
-	int retval;
-
-	mqd_mgr = dqm->mqd_mgrs[KFD_MQD_TYPE_COMPUTE];
-
-	retval = allocate_hqd(dqm, q);
-	if (retval)
-		return retval;
-
-	retval = allocate_doorbell(qpd, q);
-	if (retval)
-		goto out_deallocate_hqd;
-
-	retval = mqd_mgr->init_mqd(mqd_mgr, &q->mqd, &q->mqd_mem_obj,
-				&q->gart_mqd_addr, &q->properties);
-	if (retval)
-		goto out_deallocate_doorbell;
-
-	pr_debug("Loading mqd to hqd on pipe %d, queue %d\n",
-			q->pipe, q->queue);
-
-	dqm->dev->kfd2kgd->set_scratch_backing_va(
-			dqm->dev->kgd, qpd->sh_hidden_private_base, qpd->vmid);
-
-	if (!q->properties.is_active)
-		return 0;
-
-	if (WARN(q->process->mm != current->mm,
-		 "should only run in user thread"))
-		retval = -EFAULT;
-	else
-		retval = mqd_mgr->load_mqd(mqd_mgr, q->mqd, q->pipe, q->queue,
-					   &q->properties, current->mm);
-	if (retval)
-		goto out_uninit_mqd;
-
-	return 0;
-
-out_uninit_mqd:
-	mqd_mgr->uninit_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
-out_deallocate_doorbell:
-	deallocate_doorbell(qpd, q);
-out_deallocate_hqd:
-	deallocate_hqd(dqm, q);
-
-	return retval;
-}
-
 /* Access to DQM has to be locked before calling destroy_queue_nocpsch_locked
  * to avoid asynchronized access
  */
@@ -972,49 +958,6 @@ static void deallocate_sdma_queue(struct device_queue_manager *dqm,
 	}
 }
 
-static int create_sdma_queue_nocpsch(struct device_queue_manager *dqm,
-					struct queue *q,
-					struct qcm_process_device *qpd)
-{
-	struct mqd_manager *mqd_mgr;
-	int retval;
-
-	mqd_mgr = dqm->mqd_mgrs[KFD_MQD_TYPE_SDMA];
-
-	retval = allocate_sdma_queue(dqm, q);
-	if (retval)
-		return retval;
-
-	retval = allocate_doorbell(qpd, q);
-	if (retval)
-		goto out_deallocate_sdma_queue;
-
-	dqm->asic_ops.init_sdma_vm(dqm, q, qpd);
-	retval = mqd_mgr->init_mqd(mqd_mgr, &q->mqd, &q->mqd_mem_obj,
-				&q->gart_mqd_addr, &q->properties);
-	if (retval)
-		goto out_deallocate_doorbell;
-
-	if (!q->properties.is_active)
-		return 0;
-
-	retval = mqd_mgr->load_mqd(mqd_mgr, q->mqd, 0, 0, &q->properties,
-				current->mm);
-	if (retval)
-		goto out_uninit_mqd;
-
-	return 0;
-
-out_uninit_mqd:
-	mqd_mgr->uninit_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
-out_deallocate_doorbell:
-	deallocate_doorbell(qpd, q);
-out_deallocate_sdma_queue:
-	deallocate_sdma_queue(dqm, q);
-
-	return retval;
-}
-
 /*
  * Device Queue Manager implementation for cp scheduler
  */
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/6] drm/amdkfd: Refactor create_queue_nocpsch

[Kuehling, Felix]

Patches 1-3 are Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>

On 2019-06-05 12:06 p.m., Zeng, Oak wrote:
> This is prepare work to fix a circular lock dependency.
> No logic change
>
> Change-Id: I4e0ee918260e7780de972dd71f4ce787b4f6dde9
> Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>
> ---
>   .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c  | 171 +++++++--------------
>   1 file changed, 57 insertions(+), 114 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> index dab6ef4..3c042eb 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -42,10 +42,6 @@
>   static int set_pasid_vmid_mapping(struct device_queue_manager *dqm,
>   					unsigned int pasid, unsigned int vmid);
>   
> -static int create_compute_queue_nocpsch(struct device_queue_manager *dqm,
> -					struct queue *q,
> -					struct qcm_process_device *qpd);
> -
>   static int execute_queues_cpsch(struct device_queue_manager *dqm,
>   				enum kfd_unmap_queues_filter filter,
>   				uint32_t filter_param);
> @@ -55,13 +51,14 @@ static int unmap_queues_cpsch(struct device_queue_manager *dqm,
>   
>   static int map_queues_cpsch(struct device_queue_manager *dqm);
>   
> -static int create_sdma_queue_nocpsch(struct device_queue_manager *dqm,
> -					struct queue *q,
> -					struct qcm_process_device *qpd);
> -
>   static void deallocate_sdma_queue(struct device_queue_manager *dqm,
>   				struct queue *q);
>   
> +static inline void deallocate_hqd(struct device_queue_manager *dqm,
> +				struct queue *q);
> +static int allocate_hqd(struct device_queue_manager *dqm, struct queue *q);
> +static int allocate_sdma_queue(struct device_queue_manager *dqm,
> +				struct queue *q);
>   static void kfd_process_hw_exception(struct work_struct *work);
>   
>   static inline
> @@ -223,6 +220,9 @@ static int allocate_vmid(struct device_queue_manager *dqm,
>   	/* invalidate the VM context after pasid and vmid mapping is set up */
>   	kfd_flush_tlb(qpd_to_pdd(qpd));
>   
> +	dqm->dev->kfd2kgd->set_scratch_backing_va(
> +		dqm->dev->kgd, qpd->sh_hidden_private_base, qpd->vmid);
> +
>   	return 0;
>   }
>   
> @@ -269,6 +269,7 @@ static int create_queue_nocpsch(struct device_queue_manager *dqm,
>   				struct queue *q,
>   				struct qcm_process_device *qpd)
>   {
> +	struct mqd_manager *mqd_mgr;
>   	int retval;
>   
>   	print_queue(q);
> @@ -298,18 +299,41 @@ static int create_queue_nocpsch(struct device_queue_manager *dqm,
>   	q->properties.tba_addr = qpd->tba_addr;
>   	q->properties.tma_addr = qpd->tma_addr;
>   
> -	if (q->properties.type == KFD_QUEUE_TYPE_COMPUTE)
> -		retval = create_compute_queue_nocpsch(dqm, q, qpd);
> -	else if (q->properties.type == KFD_QUEUE_TYPE_SDMA ||
> -			q->properties.type == KFD_QUEUE_TYPE_SDMA_XGMI)
> -		retval = create_sdma_queue_nocpsch(dqm, q, qpd);
> -	else
> -		retval = -EINVAL;
> +	mqd_mgr = dqm->mqd_mgrs[get_mqd_type_from_queue_type(
> +			q->properties.type)];
> +	if (q->properties.type == KFD_QUEUE_TYPE_COMPUTE) {
> +		retval = allocate_hqd(dqm, q);
> +		if (retval)
> +			goto deallocate_vmid;
> +		pr_debug("Loading mqd to hqd on pipe %d, queue %d\n",
> +			q->pipe, q->queue);
> +	} else if (q->properties.type == KFD_QUEUE_TYPE_SDMA ||
> +		q->properties.type == KFD_QUEUE_TYPE_SDMA_XGMI) {
> +		retval = allocate_sdma_queue(dqm, q);
> +		if (retval)
> +			goto deallocate_vmid;
> +		dqm->asic_ops.init_sdma_vm(dqm, q, qpd);
> +	}
>   
> -	if (retval) {
> -		if (list_empty(&qpd->queues_list))
> -			deallocate_vmid(dqm, qpd, q);
> -		goto out_unlock;
> +	retval = allocate_doorbell(qpd, q);
> +	if (retval)
> +		goto out_deallocate_hqd;
> +
> +	retval = mqd_mgr->init_mqd(mqd_mgr, &q->mqd, &q->mqd_mem_obj,
> +				&q->gart_mqd_addr, &q->properties);
> +	if (retval)
> +		goto out_deallocate_doorbell;
> +
> +	if (q->properties.is_active) {
> +
> +		if (WARN(q->process->mm != current->mm,
> +					"should only run in user thread"))
> +			retval = -EFAULT;
> +		else
> +			retval = mqd_mgr->load_mqd(mqd_mgr, q->mqd, q->pipe,
> +					q->queue, &q->properties, current->mm);
> +		if (retval)
> +			goto out_uninit_mqd;
>   	}
>   
>   	list_add(&q->list, &qpd->queues_list);
> @@ -329,7 +353,21 @@ static int create_queue_nocpsch(struct device_queue_manager *dqm,
>   	dqm->total_queue_count++;
>   	pr_debug("Total of %d queues are accountable so far\n",
>   			dqm->total_queue_count);
> +	goto out_unlock;
>   
> +out_uninit_mqd:
> +	mqd_mgr->uninit_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
> +out_deallocate_doorbell:
> +	deallocate_doorbell(qpd, q);
> +out_deallocate_hqd:
> +	if (q->properties.type == KFD_QUEUE_TYPE_COMPUTE)
> +		deallocate_hqd(dqm, q);
> +	else if (q->properties.type == KFD_QUEUE_TYPE_SDMA ||
> +		q->properties.type == KFD_QUEUE_TYPE_SDMA_XGMI)
> +		deallocate_sdma_queue(dqm, q);
> +deallocate_vmid:
> +	if (list_empty(&qpd->queues_list))
> +		deallocate_vmid(dqm, qpd, q);
>   out_unlock:
>   	dqm_unlock(dqm);
>   	return retval;
> @@ -375,58 +413,6 @@ static inline void deallocate_hqd(struct device_queue_manager *dqm,
>   	dqm->allocated_queues[q->pipe] |= (1 << q->queue);
>   }
>   
> -static int create_compute_queue_nocpsch(struct device_queue_manager *dqm,
> -					struct queue *q,
> -					struct qcm_process_device *qpd)
> -{
> -	struct mqd_manager *mqd_mgr;
> -	int retval;
> -
> -	mqd_mgr = dqm->mqd_mgrs[KFD_MQD_TYPE_COMPUTE];
> -
> -	retval = allocate_hqd(dqm, q);
> -	if (retval)
> -		return retval;
> -
> -	retval = allocate_doorbell(qpd, q);
> -	if (retval)
> -		goto out_deallocate_hqd;
> -
> -	retval = mqd_mgr->init_mqd(mqd_mgr, &q->mqd, &q->mqd_mem_obj,
> -				&q->gart_mqd_addr, &q->properties);
> -	if (retval)
> -		goto out_deallocate_doorbell;
> -
> -	pr_debug("Loading mqd to hqd on pipe %d, queue %d\n",
> -			q->pipe, q->queue);
> -
> -	dqm->dev->kfd2kgd->set_scratch_backing_va(
> -			dqm->dev->kgd, qpd->sh_hidden_private_base, qpd->vmid);
> -
> -	if (!q->properties.is_active)
> -		return 0;
> -
> -	if (WARN(q->process->mm != current->mm,
> -		 "should only run in user thread"))
> -		retval = -EFAULT;
> -	else
> -		retval = mqd_mgr->load_mqd(mqd_mgr, q->mqd, q->pipe, q->queue,
> -					   &q->properties, current->mm);
> -	if (retval)
> -		goto out_uninit_mqd;
> -
> -	return 0;
> -
> -out_uninit_mqd:
> -	mqd_mgr->uninit_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
> -out_deallocate_doorbell:
> -	deallocate_doorbell(qpd, q);
> -out_deallocate_hqd:
> -	deallocate_hqd(dqm, q);
> -
> -	return retval;
> -}
> -
>   /* Access to DQM has to be locked before calling destroy_queue_nocpsch_locked
>    * to avoid asynchronized access
>    */
> @@ -972,49 +958,6 @@ static void deallocate_sdma_queue(struct device_queue_manager *dqm,
>   	}
>   }
>   
> -static int create_sdma_queue_nocpsch(struct device_queue_manager *dqm,
> -					struct queue *q,
> -					struct qcm_process_device *qpd)
> -{
> -	struct mqd_manager *mqd_mgr;
> -	int retval;
> -
> -	mqd_mgr = dqm->mqd_mgrs[KFD_MQD_TYPE_SDMA];
> -
> -	retval = allocate_sdma_queue(dqm, q);
> -	if (retval)
> -		return retval;
> -
> -	retval = allocate_doorbell(qpd, q);
> -	if (retval)
> -		goto out_deallocate_sdma_queue;
> -
> -	dqm->asic_ops.init_sdma_vm(dqm, q, qpd);
> -	retval = mqd_mgr->init_mqd(mqd_mgr, &q->mqd, &q->mqd_mem_obj,
> -				&q->gart_mqd_addr, &q->properties);
> -	if (retval)
> -		goto out_deallocate_doorbell;
> -
> -	if (!q->properties.is_active)
> -		return 0;
> -
> -	retval = mqd_mgr->load_mqd(mqd_mgr, q->mqd, 0, 0, &q->properties,
> -				current->mm);
> -	if (retval)
> -		goto out_uninit_mqd;
> -
> -	return 0;
> -
> -out_uninit_mqd:
> -	mqd_mgr->uninit_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
> -out_deallocate_doorbell:
> -	deallocate_doorbell(qpd, q);
> -out_deallocate_sdma_queue:
> -	deallocate_sdma_queue(dqm, q);
> -
> -	return retval;
> -}
> -
>   /*
>    * Device Queue Manager implementation for cp scheduler
>    */
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 4/6] drm/amdkfd: Separate mqd allocation and initialization

[Zeng, Oak]

Introduce a new mqd allocation interface and split the original
init_mqd function into two functions: allocate_mqd and init_mqd.
This is preparation work to fix a circular lock dependency.

Change-Id: I26e53ee1abcdd688ad11d35b433da77e3fa1bee7
Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>
---
 .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c  | 16 ++++++++---
 drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c      |  8 +++++-
 drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.c       |  2 +-
 drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.h       |  8 ++++--
 drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c   | 31 ++++++++--------------
 drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c    | 31 ++++++++--------------
 drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c    | 31 ++++++++--------------
 7 files changed, 59 insertions(+), 68 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index 3c042eb..787b5be 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -319,10 +319,13 @@ static int create_queue_nocpsch(struct device_queue_manager *dqm,
 	if (retval)
 		goto out_deallocate_hqd;
 
-	retval = mqd_mgr->init_mqd(mqd_mgr, &q->mqd, &q->mqd_mem_obj,
+	q->mqd_mem_obj = mqd_mgr->allocate_mqd(mqd_mgr->dev, &q->properties);
+	if (!q->mqd_mem_obj)
+		goto out_deallocate_doorbell;
+	retval = mqd_mgr->init_mqd(mqd_mgr, &q->mqd, q->mqd_mem_obj,
 				&q->gart_mqd_addr, &q->properties);
 	if (retval)
-		goto out_deallocate_doorbell;
+		goto out_uninit_mqd;
 
 	if (q->properties.is_active) {
 
@@ -1160,10 +1163,13 @@ static int create_queue_cpsch(struct device_queue_manager *dqm, struct queue *q,
 		dqm->asic_ops.init_sdma_vm(dqm, q, qpd);
 	q->properties.tba_addr = qpd->tba_addr;
 	q->properties.tma_addr = qpd->tma_addr;
-	retval = mqd_mgr->init_mqd(mqd_mgr, &q->mqd, &q->mqd_mem_obj,
+	q->mqd_mem_obj = mqd_mgr->allocate_mqd(mqd_mgr->dev, &q->properties);
+	if (!q->mqd_mem_obj)
+		goto out_deallocate_doorbell;
+	retval = mqd_mgr->init_mqd(mqd_mgr, &q->mqd, q->mqd_mem_obj,
 				&q->gart_mqd_addr, &q->properties);
 	if (retval)
-		goto out_deallocate_doorbell;
+		goto out_deallocate_mqd;
 
 	dqm_lock(dqm);
 
@@ -1191,6 +1197,8 @@ static int create_queue_cpsch(struct device_queue_manager *dqm, struct queue *q,
 	dqm_unlock(dqm);
 	return retval;
 
+out_deallocate_mqd:
+	mqd_mgr->uninit_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
 out_deallocate_doorbell:
 	deallocate_doorbell(qpd, q);
 out_deallocate_sdma_queue:
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
index 1cc03b3..64c8ffe 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
@@ -132,8 +132,12 @@ static bool initialize(struct kernel_queue *kq, struct kfd_dev *dev,
 	kq->queue->device = dev;
 	kq->queue->process = kfd_get_process(current);
 
+	kq->queue->mqd_mem_obj = kq->mqd_mgr->allocate_mqd(kq->mqd_mgr->dev,
+					&kq->queue->properties);
+	if (!kq->queue->mqd_mem_obj)
+		goto err_allocate_mqd;
 	retval = kq->mqd_mgr->init_mqd(kq->mqd_mgr, &kq->queue->mqd,
-					&kq->queue->mqd_mem_obj,
+					kq->queue->mqd_mem_obj,
 					&kq->queue->gart_mqd_addr,
 					&kq->queue->properties);
 	if (retval != 0)
@@ -165,6 +169,8 @@ static bool initialize(struct kernel_queue *kq, struct kfd_dev *dev,
 	return true;
 err_alloc_fence:
 err_init_mqd:
+	kq->mqd_mgr->uninit_mqd(kq->mqd_mgr, kq->queue->mqd, kq->queue->mqd_mem_obj);
+err_allocate_mqd:
 	uninit_queue(kq->queue);
 err_init_queue:
 	kfd_gtt_sa_free(dev, kq->wptr_mem);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.c
index cc04b362..b4f305a 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.c
@@ -45,7 +45,7 @@ int pipe_priority_map[] = {
 	KFD_PIPE_PRIORITY_CS_HIGH
 };
 
-struct kfd_mem_obj *allocate_hiq_mqd(struct kfd_dev *dev)
+struct kfd_mem_obj *allocate_hiq_mqd(struct kfd_dev *dev, struct queue_properties *q)
 {
 	struct kfd_mem_obj *mqd_mem_obj = NULL;
 
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.h b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.h
index 66b8c67..8667990 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.h
@@ -64,8 +64,11 @@
  */
 extern int pipe_priority_map[];
 struct mqd_manager {
+	struct kfd_mem_obj*	(*allocate_mqd)(struct kfd_dev *kfd,
+		struct queue_properties *q);
+
 	int	(*init_mqd)(struct mqd_manager *mm, void **mqd,
-			struct kfd_mem_obj **mqd_mem_obj, uint64_t *gart_addr,
+			struct kfd_mem_obj *mqd_mem_obj, uint64_t *gart_addr,
 			struct queue_properties *q);
 
 	int	(*load_mqd)(struct mqd_manager *mm, void *mqd,
@@ -102,7 +105,8 @@ struct mqd_manager {
 	uint32_t mqd_size;
 };
 
-struct kfd_mem_obj *allocate_hiq_mqd(struct kfd_dev *dev);
+struct kfd_mem_obj *allocate_hiq_mqd(struct kfd_dev *dev,
+				struct queue_properties *q);
 
 struct kfd_mem_obj *allocate_sdma_mqd(struct kfd_dev *dev,
 					struct queue_properties *q);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c
index e911438..bbe808d 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c
@@ -77,9 +77,6 @@ static struct kfd_mem_obj *allocate_mqd(struct kfd_dev *kfd,
 {
 	struct kfd_mem_obj *mqd_mem_obj;
 
-	if (q->type == KFD_QUEUE_TYPE_HIQ)
-		return allocate_hiq_mqd(kfd);
-
 	if (kfd_gtt_sa_allocate(kfd, sizeof(struct cik_mqd),
 			&mqd_mem_obj))
 		return NULL;
@@ -88,20 +85,15 @@ static struct kfd_mem_obj *allocate_mqd(struct kfd_dev *kfd,
 }
 
 static int init_mqd(struct mqd_manager *mm, void **mqd,
-		struct kfd_mem_obj **mqd_mem_obj, uint64_t *gart_addr,
+		struct kfd_mem_obj *mqd_mem_obj, uint64_t *gart_addr,
 		struct queue_properties *q)
 {
 	uint64_t addr;
 	struct cik_mqd *m;
 	int retval;
-	struct kfd_dev *kfd = mm->dev;
-
-	*mqd_mem_obj = allocate_mqd(kfd, q);
-	if (!*mqd_mem_obj)
-		return -ENOMEM;
 
-	m = (struct cik_mqd *) (*mqd_mem_obj)->cpu_ptr;
-	addr = (*mqd_mem_obj)->gpu_addr;
+	m = (struct cik_mqd *) mqd_mem_obj->cpu_ptr;
+	addr = mqd_mem_obj->gpu_addr;
 
 	memset(m, 0, ALIGN(sizeof(struct cik_mqd), 256));
 
@@ -150,24 +142,19 @@ static int init_mqd(struct mqd_manager *mm, void **mqd,
 }
 
 static int init_mqd_sdma(struct mqd_manager *mm, void **mqd,
-			struct kfd_mem_obj **mqd_mem_obj, uint64_t *gart_addr,
+			struct kfd_mem_obj *mqd_mem_obj, uint64_t *gart_addr,
 			struct queue_properties *q)
 {
 	int retval;
 	struct cik_sdma_rlc_registers *m;
-	struct kfd_dev *dev = mm->dev;
-
-	*mqd_mem_obj = allocate_sdma_mqd(dev, q);
-	if (!*mqd_mem_obj)
-		return -ENOMEM;
 
-	m = (struct cik_sdma_rlc_registers *) (*mqd_mem_obj)->cpu_ptr;
+	m = (struct cik_sdma_rlc_registers *) mqd_mem_obj->cpu_ptr;
 
 	memset(m, 0, sizeof(struct cik_sdma_rlc_registers));
 
 	*mqd = m;
 	if (gart_addr)
-		*gart_addr = (*mqd_mem_obj)->gpu_addr;
+		*gart_addr = mqd_mem_obj->gpu_addr;
 
 	retval = mm->update_mqd(mm, m, q);
 
@@ -327,7 +314,7 @@ static bool is_occupied_sdma(struct mqd_manager *mm, void *mqd,
  */
 
 static int init_mqd_hiq(struct mqd_manager *mm, void **mqd,
-		struct kfd_mem_obj **mqd_mem_obj, uint64_t *gart_addr,
+		struct kfd_mem_obj *mqd_mem_obj, uint64_t *gart_addr,
 		struct queue_properties *q)
 {
 	return init_mqd(mm, mqd, mqd_mem_obj, gart_addr, q);
@@ -399,6 +386,7 @@ struct mqd_manager *mqd_manager_init_cik(enum KFD_MQD_TYPE type,
 	switch (type) {
 	case KFD_MQD_TYPE_CP:
 	case KFD_MQD_TYPE_COMPUTE:
+		mqd->allocate_mqd = allocate_mqd;
 		mqd->init_mqd = init_mqd;
 		mqd->uninit_mqd = uninit_mqd;
 		mqd->load_mqd = load_mqd;
@@ -411,6 +399,7 @@ struct mqd_manager *mqd_manager_init_cik(enum KFD_MQD_TYPE type,
 #endif
 		break;
 	case KFD_MQD_TYPE_HIQ:
+		mqd->allocate_mqd = allocate_hiq_mqd;
 		mqd->init_mqd = init_mqd_hiq;
 		mqd->uninit_mqd = uninit_mqd_hiq_sdma;
 		mqd->load_mqd = load_mqd;
@@ -423,6 +412,7 @@ struct mqd_manager *mqd_manager_init_cik(enum KFD_MQD_TYPE type,
 #endif
 		break;
 	case KFD_MQD_TYPE_DIQ:
+		mqd->allocate_mqd = allocate_hiq_mqd;
 		mqd->init_mqd = init_mqd_hiq;
 		mqd->uninit_mqd = uninit_mqd;
 		mqd->load_mqd = load_mqd;
@@ -435,6 +425,7 @@ struct mqd_manager *mqd_manager_init_cik(enum KFD_MQD_TYPE type,
 #endif
 		break;
 	case KFD_MQD_TYPE_SDMA:
+		mqd->allocate_mqd = allocate_sdma_mqd;
 		mqd->init_mqd = init_mqd_sdma;
 		mqd->uninit_mqd = uninit_mqd_hiq_sdma;
 		mqd->load_mqd = load_mqd_sdma;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
index 818944b..572cc6a 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
@@ -79,9 +79,6 @@ static struct kfd_mem_obj *allocate_mqd(struct kfd_dev *kfd,
 	int retval;
 	struct kfd_mem_obj *mqd_mem_obj = NULL;
 
-	if (q->type == KFD_QUEUE_TYPE_HIQ)
-		return allocate_hiq_mqd(kfd);
-
 	/* From V9,  for CWSR, the control stack is located on the next page
 	 * boundary after the mqd, we will use the gtt allocation function
 	 * instead of sub-allocation function.
@@ -111,20 +108,15 @@ static struct kfd_mem_obj *allocate_mqd(struct kfd_dev *kfd,
 }
 
 static int init_mqd(struct mqd_manager *mm, void **mqd,
-			struct kfd_mem_obj **mqd_mem_obj, uint64_t *gart_addr,
+			struct kfd_mem_obj *mqd_mem_obj, uint64_t *gart_addr,
 			struct queue_properties *q)
 {
 	int retval;
 	uint64_t addr;
 	struct v9_mqd *m;
-	struct kfd_dev *kfd = mm->dev;
 
-	*mqd_mem_obj = allocate_mqd(kfd, q);
-	if (!*mqd_mem_obj)
-		return -ENOMEM;
-
-	m = (struct v9_mqd *) (*mqd_mem_obj)->cpu_ptr;
-	addr = (*mqd_mem_obj)->gpu_addr;
+	m = (struct v9_mqd *) mqd_mem_obj->cpu_ptr;
+	addr = mqd_mem_obj->gpu_addr;
 
 	memset(m, 0, sizeof(struct v9_mqd));
 
@@ -312,7 +304,7 @@ static int get_wave_state(struct mqd_manager *mm, void *mqd,
 }
 
 static int init_mqd_hiq(struct mqd_manager *mm, void **mqd,
-			struct kfd_mem_obj **mqd_mem_obj, uint64_t *gart_addr,
+			struct kfd_mem_obj *mqd_mem_obj, uint64_t *gart_addr,
 			struct queue_properties *q)
 {
 	struct v9_mqd *m;
@@ -345,24 +337,19 @@ static int update_mqd_hiq(struct mqd_manager *mm, void *mqd,
 }
 
 static int init_mqd_sdma(struct mqd_manager *mm, void **mqd,
-		struct kfd_mem_obj **mqd_mem_obj, uint64_t *gart_addr,
+		struct kfd_mem_obj *mqd_mem_obj, uint64_t *gart_addr,
 		struct queue_properties *q)
 {
 	int retval;
 	struct v9_sdma_mqd *m;
-	struct kfd_dev *dev = mm->dev;
-
-	*mqd_mem_obj = allocate_sdma_mqd(dev, q);
-	if (!*mqd_mem_obj)
-		return -ENOMEM;
 
-	m = (struct v9_sdma_mqd *) (*mqd_mem_obj)->cpu_ptr;
+	m = (struct v9_sdma_mqd *) mqd_mem_obj->cpu_ptr;
 
 	memset(m, 0, sizeof(struct v9_sdma_mqd));
 
 	*mqd = m;
 	if (gart_addr)
-		*gart_addr = (*mqd_mem_obj)->gpu_addr;
+		*gart_addr = mqd_mem_obj->gpu_addr;
 
 	retval = mm->update_mqd(mm, m, q);
 
@@ -462,6 +449,7 @@ struct mqd_manager *mqd_manager_init_v9(enum KFD_MQD_TYPE type,
 	switch (type) {
 	case KFD_MQD_TYPE_CP:
 	case KFD_MQD_TYPE_COMPUTE:
+		mqd->allocate_mqd = allocate_mqd;
 		mqd->init_mqd = init_mqd;
 		mqd->uninit_mqd = uninit_mqd;
 		mqd->load_mqd = load_mqd;
@@ -475,6 +463,7 @@ struct mqd_manager *mqd_manager_init_v9(enum KFD_MQD_TYPE type,
 #endif
 		break;
 	case KFD_MQD_TYPE_HIQ:
+		mqd->allocate_mqd = allocate_hiq_mqd;
 		mqd->init_mqd = init_mqd_hiq;
 		mqd->uninit_mqd = uninit_mqd_hiq_sdma;
 		mqd->load_mqd = load_mqd;
@@ -487,6 +476,7 @@ struct mqd_manager *mqd_manager_init_v9(enum KFD_MQD_TYPE type,
 #endif
 		break;
 	case KFD_MQD_TYPE_DIQ:
+		mqd->allocate_mqd = allocate_hiq_mqd;
 		mqd->init_mqd = init_mqd_hiq;
 		mqd->uninit_mqd = uninit_mqd;
 		mqd->load_mqd = load_mqd;
@@ -499,6 +489,7 @@ struct mqd_manager *mqd_manager_init_v9(enum KFD_MQD_TYPE type,
 #endif
 		break;
 	case KFD_MQD_TYPE_SDMA:
+		mqd->allocate_mqd = allocate_sdma_mqd;
 		mqd->init_mqd = init_mqd_sdma;
 		mqd->uninit_mqd = uninit_mqd_hiq_sdma;
 		mqd->load_mqd = load_mqd_sdma;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c
index 00e6a59..431899f 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c
@@ -80,9 +80,6 @@ static struct kfd_mem_obj *allocate_mqd(struct kfd_dev *kfd,
 {
 	struct kfd_mem_obj *mqd_mem_obj;
 
-	if (q->type == KFD_QUEUE_TYPE_HIQ)
-		return allocate_hiq_mqd(kfd);
-
 	if (kfd_gtt_sa_allocate(kfd, sizeof(struct vi_mqd),
 			&mqd_mem_obj))
 		return NULL;
@@ -91,20 +88,15 @@ static struct kfd_mem_obj *allocate_mqd(struct kfd_dev *kfd,
 }
 
 static int init_mqd(struct mqd_manager *mm, void **mqd,
-			struct kfd_mem_obj **mqd_mem_obj, uint64_t *gart_addr,
+			struct kfd_mem_obj *mqd_mem_obj, uint64_t *gart_addr,
 			struct queue_properties *q)
 {
 	int retval;
 	uint64_t addr;
 	struct vi_mqd *m;
-	struct kfd_dev *kfd = mm->dev;
-
-	*mqd_mem_obj = allocate_mqd(kfd, q);
-	if (!*mqd_mem_obj)
-		return -ENOMEM;
 
-	m = (struct vi_mqd *) (*mqd_mem_obj)->cpu_ptr;
-	addr = (*mqd_mem_obj)->gpu_addr;
+	m = (struct vi_mqd *) mqd_mem_obj->cpu_ptr;
+	addr = mqd_mem_obj->gpu_addr;
 
 	memset(m, 0, sizeof(struct vi_mqd));
 
@@ -310,7 +302,7 @@ static int get_wave_state(struct mqd_manager *mm, void *mqd,
 }
 
 static int init_mqd_hiq(struct mqd_manager *mm, void **mqd,
-			struct kfd_mem_obj **mqd_mem_obj, uint64_t *gart_addr,
+			struct kfd_mem_obj *mqd_mem_obj, uint64_t *gart_addr,
 			struct queue_properties *q)
 {
 	struct vi_mqd *m;
@@ -342,24 +334,19 @@ static int update_mqd_hiq(struct mqd_manager *mm, void *mqd,
 }
 
 static int init_mqd_sdma(struct mqd_manager *mm, void **mqd,
-		struct kfd_mem_obj **mqd_mem_obj, uint64_t *gart_addr,
+		struct kfd_mem_obj *mqd_mem_obj, uint64_t *gart_addr,
 		struct queue_properties *q)
 {
 	int retval;
 	struct vi_sdma_mqd *m;
-	struct kfd_dev *dev = mm->dev;
-
-	*mqd_mem_obj = allocate_sdma_mqd(dev, q);
-	if (!*mqd_mem_obj)
-		return -ENOMEM;
 
-	m = (struct vi_sdma_mqd *) (*mqd_mem_obj)->cpu_ptr;
+	m = (struct vi_sdma_mqd *) mqd_mem_obj->cpu_ptr;
 
 	memset(m, 0, sizeof(struct vi_sdma_mqd));
 
 	*mqd = m;
 	if (gart_addr)
-		*gart_addr = (*mqd_mem_obj)->gpu_addr;
+		*gart_addr = mqd_mem_obj->gpu_addr;
 
 	retval = mm->update_mqd(mm, m, q);
 
@@ -458,6 +445,7 @@ struct mqd_manager *mqd_manager_init_vi(enum KFD_MQD_TYPE type,
 	switch (type) {
 	case KFD_MQD_TYPE_CP:
 	case KFD_MQD_TYPE_COMPUTE:
+		mqd->allocate_mqd = allocate_mqd;
 		mqd->init_mqd = init_mqd;
 		mqd->uninit_mqd = uninit_mqd;
 		mqd->load_mqd = load_mqd;
@@ -471,6 +459,7 @@ struct mqd_manager *mqd_manager_init_vi(enum KFD_MQD_TYPE type,
 #endif
 		break;
 	case KFD_MQD_TYPE_HIQ:
+		mqd->allocate_mqd = allocate_hiq_mqd;
 		mqd->init_mqd = init_mqd_hiq;
 		mqd->uninit_mqd = uninit_mqd_hiq_sdma;
 		mqd->load_mqd = load_mqd;
@@ -483,6 +472,7 @@ struct mqd_manager *mqd_manager_init_vi(enum KFD_MQD_TYPE type,
 #endif
 		break;
 	case KFD_MQD_TYPE_DIQ:
+		mqd->allocate_mqd = allocate_hiq_mqd;
 		mqd->init_mqd = init_mqd_hiq;
 		mqd->uninit_mqd = uninit_mqd;
 		mqd->load_mqd = load_mqd;
@@ -495,6 +485,7 @@ struct mqd_manager *mqd_manager_init_vi(enum KFD_MQD_TYPE type,
 #endif
 		break;
 	case KFD_MQD_TYPE_SDMA:
+		mqd->allocate_mqd = allocate_sdma_mqd;
 		mqd->init_mqd = init_mqd_sdma;
 		mqd->uninit_mqd = uninit_mqd_hiq_sdma;
 		mqd->load_mqd = load_mqd_sdma;
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 4/6] drm/amdkfd: Separate mqd allocation and initialization

[Kuehling, Felix]

Nice. Surprisingly this actually resulted in slightly fewer lines of 
code. Some suggestions:

Let's also rename uninit_mqd to free_mqd, as it is the counterpart to 
allocate_mqd, not init_mqd.

And unless I'm missing something, init_mqd and update_mqd cannot fail 
now that allocate_mqd has been separated from them. That means you could 
change them to void functions, and it may simplify some of the error 
handling if you have fewer potential failure cases to consider.

Regards,
   Felix

On 2019-06-05 12:06 p.m., Zeng, Oak wrote:
> Introduce a new mqd allocation interface and split the original
> init_mqd function into two functions: allocate_mqd and init_mqd.
> This is preparation work to fix a circular lock dependency.
>
> Change-Id: I26e53ee1abcdd688ad11d35b433da77e3fa1bee7
> Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>
> ---
>   .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c  | 16 ++++++++---
>   drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c      |  8 +++++-
>   drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.c       |  2 +-
>   drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.h       |  8 ++++--
>   drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c   | 31 ++++++++--------------
>   drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c    | 31 ++++++++--------------
>   drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c    | 31 ++++++++--------------
>   7 files changed, 59 insertions(+), 68 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> index 3c042eb..787b5be 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -319,10 +319,13 @@ static int create_queue_nocpsch(struct device_queue_manager *dqm,
>   	if (retval)
>   		goto out_deallocate_hqd;
>   
> -	retval = mqd_mgr->init_mqd(mqd_mgr, &q->mqd, &q->mqd_mem_obj,
> +	q->mqd_mem_obj = mqd_mgr->allocate_mqd(mqd_mgr->dev, &q->properties);
> +	if (!q->mqd_mem_obj)
> +		goto out_deallocate_doorbell;
> +	retval = mqd_mgr->init_mqd(mqd_mgr, &q->mqd, q->mqd_mem_obj,
>   				&q->gart_mqd_addr, &q->properties);
>   	if (retval)
> -		goto out_deallocate_doorbell;
> +		goto out_uninit_mqd;
>   
>   	if (q->properties.is_active) {
>   
> @@ -1160,10 +1163,13 @@ static int create_queue_cpsch(struct device_queue_manager *dqm, struct queue *q,
>   		dqm->asic_ops.init_sdma_vm(dqm, q, qpd);
>   	q->properties.tba_addr = qpd->tba_addr;
>   	q->properties.tma_addr = qpd->tma_addr;
> -	retval = mqd_mgr->init_mqd(mqd_mgr, &q->mqd, &q->mqd_mem_obj,
> +	q->mqd_mem_obj = mqd_mgr->allocate_mqd(mqd_mgr->dev, &q->properties);
> +	if (!q->mqd_mem_obj)
> +		goto out_deallocate_doorbell;
> +	retval = mqd_mgr->init_mqd(mqd_mgr, &q->mqd, q->mqd_mem_obj,
>   				&q->gart_mqd_addr, &q->properties);
>   	if (retval)
> -		goto out_deallocate_doorbell;
> +		goto out_deallocate_mqd;
>   
>   	dqm_lock(dqm);
>   
> @@ -1191,6 +1197,8 @@ static int create_queue_cpsch(struct device_queue_manager *dqm, struct queue *q,
>   	dqm_unlock(dqm);
>   	return retval;
>   
> +out_deallocate_mqd:
> +	mqd_mgr->uninit_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
>   out_deallocate_doorbell:
>   	deallocate_doorbell(qpd, q);
>   out_deallocate_sdma_queue:
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
> index 1cc03b3..64c8ffe 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
> @@ -132,8 +132,12 @@ static bool initialize(struct kernel_queue *kq, struct kfd_dev *dev,
>   	kq->queue->device = dev;
>   	kq->queue->process = kfd_get_process(current);
>   
> +	kq->queue->mqd_mem_obj = kq->mqd_mgr->allocate_mqd(kq->mqd_mgr->dev,
> +					&kq->queue->properties);
> +	if (!kq->queue->mqd_mem_obj)
> +		goto err_allocate_mqd;
>   	retval = kq->mqd_mgr->init_mqd(kq->mqd_mgr, &kq->queue->mqd,
> -					&kq->queue->mqd_mem_obj,
> +					kq->queue->mqd_mem_obj,
>   					&kq->queue->gart_mqd_addr,
>   					&kq->queue->properties);
>   	if (retval != 0)
> @@ -165,6 +169,8 @@ static bool initialize(struct kernel_queue *kq, struct kfd_dev *dev,
>   	return true;
>   err_alloc_fence:
>   err_init_mqd:
> +	kq->mqd_mgr->uninit_mqd(kq->mqd_mgr, kq->queue->mqd, kq->queue->mqd_mem_obj);
> +err_allocate_mqd:
>   	uninit_queue(kq->queue);
>   err_init_queue:
>   	kfd_gtt_sa_free(dev, kq->wptr_mem);
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.c
> index cc04b362..b4f305a 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.c
> @@ -45,7 +45,7 @@ int pipe_priority_map[] = {
>   	KFD_PIPE_PRIORITY_CS_HIGH
>   };
>   
> -struct kfd_mem_obj *allocate_hiq_mqd(struct kfd_dev *dev)
> +struct kfd_mem_obj *allocate_hiq_mqd(struct kfd_dev *dev, struct queue_properties *q)
>   {
>   	struct kfd_mem_obj *mqd_mem_obj = NULL;
>   
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.h b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.h
> index 66b8c67..8667990 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.h
> @@ -64,8 +64,11 @@
>    */
>   extern int pipe_priority_map[];
>   struct mqd_manager {
> +	struct kfd_mem_obj*	(*allocate_mqd)(struct kfd_dev *kfd,
> +		struct queue_properties *q);
> +
>   	int	(*init_mqd)(struct mqd_manager *mm, void **mqd,
> -			struct kfd_mem_obj **mqd_mem_obj, uint64_t *gart_addr,
> +			struct kfd_mem_obj *mqd_mem_obj, uint64_t *gart_addr,
>   			struct queue_properties *q);
>   
>   	int	(*load_mqd)(struct mqd_manager *mm, void *mqd,
> @@ -102,7 +105,8 @@ struct mqd_manager {
>   	uint32_t mqd_size;
>   };
>   
> -struct kfd_mem_obj *allocate_hiq_mqd(struct kfd_dev *dev);
> +struct kfd_mem_obj *allocate_hiq_mqd(struct kfd_dev *dev,
> +				struct queue_properties *q);
>   
>   struct kfd_mem_obj *allocate_sdma_mqd(struct kfd_dev *dev,
>   					struct queue_properties *q);
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c
> index e911438..bbe808d 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c
> @@ -77,9 +77,6 @@ static struct kfd_mem_obj *allocate_mqd(struct kfd_dev *kfd,
>   {
>   	struct kfd_mem_obj *mqd_mem_obj;
>   
> -	if (q->type == KFD_QUEUE_TYPE_HIQ)
> -		return allocate_hiq_mqd(kfd);
> -
>   	if (kfd_gtt_sa_allocate(kfd, sizeof(struct cik_mqd),
>   			&mqd_mem_obj))
>   		return NULL;
> @@ -88,20 +85,15 @@ static struct kfd_mem_obj *allocate_mqd(struct kfd_dev *kfd,
>   }
>   
>   static int init_mqd(struct mqd_manager *mm, void **mqd,
> -		struct kfd_mem_obj **mqd_mem_obj, uint64_t *gart_addr,
> +		struct kfd_mem_obj *mqd_mem_obj, uint64_t *gart_addr,
>   		struct queue_properties *q)
>   {
>   	uint64_t addr;
>   	struct cik_mqd *m;
>   	int retval;
> -	struct kfd_dev *kfd = mm->dev;
> -
> -	*mqd_mem_obj = allocate_mqd(kfd, q);
> -	if (!*mqd_mem_obj)
> -		return -ENOMEM;
>   
> -	m = (struct cik_mqd *) (*mqd_mem_obj)->cpu_ptr;
> -	addr = (*mqd_mem_obj)->gpu_addr;
> +	m = (struct cik_mqd *) mqd_mem_obj->cpu_ptr;
> +	addr = mqd_mem_obj->gpu_addr;
>   
>   	memset(m, 0, ALIGN(sizeof(struct cik_mqd), 256));
>   
> @@ -150,24 +142,19 @@ static int init_mqd(struct mqd_manager *mm, void **mqd,
>   }
>   
>   static int init_mqd_sdma(struct mqd_manager *mm, void **mqd,
> -			struct kfd_mem_obj **mqd_mem_obj, uint64_t *gart_addr,
> +			struct kfd_mem_obj *mqd_mem_obj, uint64_t *gart_addr,
>   			struct queue_properties *q)
>   {
>   	int retval;
>   	struct cik_sdma_rlc_registers *m;
> -	struct kfd_dev *dev = mm->dev;
> -
> -	*mqd_mem_obj = allocate_sdma_mqd(dev, q);
> -	if (!*mqd_mem_obj)
> -		return -ENOMEM;
>   
> -	m = (struct cik_sdma_rlc_registers *) (*mqd_mem_obj)->cpu_ptr;
> +	m = (struct cik_sdma_rlc_registers *) mqd_mem_obj->cpu_ptr;
>   
>   	memset(m, 0, sizeof(struct cik_sdma_rlc_registers));
>   
>   	*mqd = m;
>   	if (gart_addr)
> -		*gart_addr = (*mqd_mem_obj)->gpu_addr;
> +		*gart_addr = mqd_mem_obj->gpu_addr;
>   
>   	retval = mm->update_mqd(mm, m, q);
>   
> @@ -327,7 +314,7 @@ static bool is_occupied_sdma(struct mqd_manager *mm, void *mqd,
>    */
>   
>   static int init_mqd_hiq(struct mqd_manager *mm, void **mqd,
> -		struct kfd_mem_obj **mqd_mem_obj, uint64_t *gart_addr,
> +		struct kfd_mem_obj *mqd_mem_obj, uint64_t *gart_addr,
>   		struct queue_properties *q)
>   {
>   	return init_mqd(mm, mqd, mqd_mem_obj, gart_addr, q);
> @@ -399,6 +386,7 @@ struct mqd_manager *mqd_manager_init_cik(enum KFD_MQD_TYPE type,
>   	switch (type) {
>   	case KFD_MQD_TYPE_CP:
>   	case KFD_MQD_TYPE_COMPUTE:
> +		mqd->allocate_mqd = allocate_mqd;
>   		mqd->init_mqd = init_mqd;
>   		mqd->uninit_mqd = uninit_mqd;
>   		mqd->load_mqd = load_mqd;
> @@ -411,6 +399,7 @@ struct mqd_manager *mqd_manager_init_cik(enum KFD_MQD_TYPE type,
>   #endif
>   		break;
>   	case KFD_MQD_TYPE_HIQ:
> +		mqd->allocate_mqd = allocate_hiq_mqd;
>   		mqd->init_mqd = init_mqd_hiq;
>   		mqd->uninit_mqd = uninit_mqd_hiq_sdma;
>   		mqd->load_mqd = load_mqd;
> @@ -423,6 +412,7 @@ struct mqd_manager *mqd_manager_init_cik(enum KFD_MQD_TYPE type,
>   #endif
>   		break;
>   	case KFD_MQD_TYPE_DIQ:
> +		mqd->allocate_mqd = allocate_hiq_mqd;
>   		mqd->init_mqd = init_mqd_hiq;
>   		mqd->uninit_mqd = uninit_mqd;
>   		mqd->load_mqd = load_mqd;
> @@ -435,6 +425,7 @@ struct mqd_manager *mqd_manager_init_cik(enum KFD_MQD_TYPE type,
>   #endif
>   		break;
>   	case KFD_MQD_TYPE_SDMA:
> +		mqd->allocate_mqd = allocate_sdma_mqd;
>   		mqd->init_mqd = init_mqd_sdma;
>   		mqd->uninit_mqd = uninit_mqd_hiq_sdma;
>   		mqd->load_mqd = load_mqd_sdma;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
> index 818944b..572cc6a 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
> @@ -79,9 +79,6 @@ static struct kfd_mem_obj *allocate_mqd(struct kfd_dev *kfd,
>   	int retval;
>   	struct kfd_mem_obj *mqd_mem_obj = NULL;
>   
> -	if (q->type == KFD_QUEUE_TYPE_HIQ)
> -		return allocate_hiq_mqd(kfd);
> -
>   	/* From V9,  for CWSR, the control stack is located on the next page
>   	 * boundary after the mqd, we will use the gtt allocation function
>   	 * instead of sub-allocation function.
> @@ -111,20 +108,15 @@ static struct kfd_mem_obj *allocate_mqd(struct kfd_dev *kfd,
>   }
>   
>   static int init_mqd(struct mqd_manager *mm, void **mqd,
> -			struct kfd_mem_obj **mqd_mem_obj, uint64_t *gart_addr,
> +			struct kfd_mem_obj *mqd_mem_obj, uint64_t *gart_addr,
>   			struct queue_properties *q)
>   {
>   	int retval;
>   	uint64_t addr;
>   	struct v9_mqd *m;
> -	struct kfd_dev *kfd = mm->dev;
>   
> -	*mqd_mem_obj = allocate_mqd(kfd, q);
> -	if (!*mqd_mem_obj)
> -		return -ENOMEM;
> -
> -	m = (struct v9_mqd *) (*mqd_mem_obj)->cpu_ptr;
> -	addr = (*mqd_mem_obj)->gpu_addr;
> +	m = (struct v9_mqd *) mqd_mem_obj->cpu_ptr;
> +	addr = mqd_mem_obj->gpu_addr;
>   
>   	memset(m, 0, sizeof(struct v9_mqd));
>   
> @@ -312,7 +304,7 @@ static int get_wave_state(struct mqd_manager *mm, void *mqd,
>   }
>   
>   static int init_mqd_hiq(struct mqd_manager *mm, void **mqd,
> -			struct kfd_mem_obj **mqd_mem_obj, uint64_t *gart_addr,
> +			struct kfd_mem_obj *mqd_mem_obj, uint64_t *gart_addr,
>   			struct queue_properties *q)
>   {
>   	struct v9_mqd *m;
> @@ -345,24 +337,19 @@ static int update_mqd_hiq(struct mqd_manager *mm, void *mqd,
>   }
>   
>   static int init_mqd_sdma(struct mqd_manager *mm, void **mqd,
> -		struct kfd_mem_obj **mqd_mem_obj, uint64_t *gart_addr,
> +		struct kfd_mem_obj *mqd_mem_obj, uint64_t *gart_addr,
>   		struct queue_properties *q)
>   {
>   	int retval;
>   	struct v9_sdma_mqd *m;
> -	struct kfd_dev *dev = mm->dev;
> -
> -	*mqd_mem_obj = allocate_sdma_mqd(dev, q);
> -	if (!*mqd_mem_obj)
> -		return -ENOMEM;
>   
> -	m = (struct v9_sdma_mqd *) (*mqd_mem_obj)->cpu_ptr;
> +	m = (struct v9_sdma_mqd *) mqd_mem_obj->cpu_ptr;
>   
>   	memset(m, 0, sizeof(struct v9_sdma_mqd));
>   
>   	*mqd = m;
>   	if (gart_addr)
> -		*gart_addr = (*mqd_mem_obj)->gpu_addr;
> +		*gart_addr = mqd_mem_obj->gpu_addr;
>   
>   	retval = mm->update_mqd(mm, m, q);
>   
> @@ -462,6 +449,7 @@ struct mqd_manager *mqd_manager_init_v9(enum KFD_MQD_TYPE type,
>   	switch (type) {
>   	case KFD_MQD_TYPE_CP:
>   	case KFD_MQD_TYPE_COMPUTE:
> +		mqd->allocate_mqd = allocate_mqd;
>   		mqd->init_mqd = init_mqd;
>   		mqd->uninit_mqd = uninit_mqd;
>   		mqd->load_mqd = load_mqd;
> @@ -475,6 +463,7 @@ struct mqd_manager *mqd_manager_init_v9(enum KFD_MQD_TYPE type,
>   #endif
>   		break;
>   	case KFD_MQD_TYPE_HIQ:
> +		mqd->allocate_mqd = allocate_hiq_mqd;
>   		mqd->init_mqd = init_mqd_hiq;
>   		mqd->uninit_mqd = uninit_mqd_hiq_sdma;
>   		mqd->load_mqd = load_mqd;
> @@ -487,6 +476,7 @@ struct mqd_manager *mqd_manager_init_v9(enum KFD_MQD_TYPE type,
>   #endif
>   		break;
>   	case KFD_MQD_TYPE_DIQ:
> +		mqd->allocate_mqd = allocate_hiq_mqd;
>   		mqd->init_mqd = init_mqd_hiq;
>   		mqd->uninit_mqd = uninit_mqd;
>   		mqd->load_mqd = load_mqd;
> @@ -499,6 +489,7 @@ struct mqd_manager *mqd_manager_init_v9(enum KFD_MQD_TYPE type,
>   #endif
>   		break;
>   	case KFD_MQD_TYPE_SDMA:
> +		mqd->allocate_mqd = allocate_sdma_mqd;
>   		mqd->init_mqd = init_mqd_sdma;
>   		mqd->uninit_mqd = uninit_mqd_hiq_sdma;
>   		mqd->load_mqd = load_mqd_sdma;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c
> index 00e6a59..431899f 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c
> @@ -80,9 +80,6 @@ static struct kfd_mem_obj *allocate_mqd(struct kfd_dev *kfd,
>   {
>   	struct kfd_mem_obj *mqd_mem_obj;
>   
> -	if (q->type == KFD_QUEUE_TYPE_HIQ)
> -		return allocate_hiq_mqd(kfd);
> -
>   	if (kfd_gtt_sa_allocate(kfd, sizeof(struct vi_mqd),
>   			&mqd_mem_obj))
>   		return NULL;
> @@ -91,20 +88,15 @@ static struct kfd_mem_obj *allocate_mqd(struct kfd_dev *kfd,
>   }
>   
>   static int init_mqd(struct mqd_manager *mm, void **mqd,
> -			struct kfd_mem_obj **mqd_mem_obj, uint64_t *gart_addr,
> +			struct kfd_mem_obj *mqd_mem_obj, uint64_t *gart_addr,
>   			struct queue_properties *q)
>   {
>   	int retval;
>   	uint64_t addr;
>   	struct vi_mqd *m;
> -	struct kfd_dev *kfd = mm->dev;
> -
> -	*mqd_mem_obj = allocate_mqd(kfd, q);
> -	if (!*mqd_mem_obj)
> -		return -ENOMEM;
>   
> -	m = (struct vi_mqd *) (*mqd_mem_obj)->cpu_ptr;
> -	addr = (*mqd_mem_obj)->gpu_addr;
> +	m = (struct vi_mqd *) mqd_mem_obj->cpu_ptr;
> +	addr = mqd_mem_obj->gpu_addr;
>   
>   	memset(m, 0, sizeof(struct vi_mqd));
>   
> @@ -310,7 +302,7 @@ static int get_wave_state(struct mqd_manager *mm, void *mqd,
>   }
>   
>   static int init_mqd_hiq(struct mqd_manager *mm, void **mqd,
> -			struct kfd_mem_obj **mqd_mem_obj, uint64_t *gart_addr,
> +			struct kfd_mem_obj *mqd_mem_obj, uint64_t *gart_addr,
>   			struct queue_properties *q)
>   {
>   	struct vi_mqd *m;
> @@ -342,24 +334,19 @@ static int update_mqd_hiq(struct mqd_manager *mm, void *mqd,
>   }
>   
>   static int init_mqd_sdma(struct mqd_manager *mm, void **mqd,
> -		struct kfd_mem_obj **mqd_mem_obj, uint64_t *gart_addr,
> +		struct kfd_mem_obj *mqd_mem_obj, uint64_t *gart_addr,
>   		struct queue_properties *q)
>   {
>   	int retval;
>   	struct vi_sdma_mqd *m;
> -	struct kfd_dev *dev = mm->dev;
> -
> -	*mqd_mem_obj = allocate_sdma_mqd(dev, q);
> -	if (!*mqd_mem_obj)
> -		return -ENOMEM;
>   
> -	m = (struct vi_sdma_mqd *) (*mqd_mem_obj)->cpu_ptr;
> +	m = (struct vi_sdma_mqd *) mqd_mem_obj->cpu_ptr;
>   
>   	memset(m, 0, sizeof(struct vi_sdma_mqd));
>   
>   	*mqd = m;
>   	if (gart_addr)
> -		*gart_addr = (*mqd_mem_obj)->gpu_addr;
> +		*gart_addr = mqd_mem_obj->gpu_addr;
>   
>   	retval = mm->update_mqd(mm, m, q);
>   
> @@ -458,6 +445,7 @@ struct mqd_manager *mqd_manager_init_vi(enum KFD_MQD_TYPE type,
>   	switch (type) {
>   	case KFD_MQD_TYPE_CP:
>   	case KFD_MQD_TYPE_COMPUTE:
> +		mqd->allocate_mqd = allocate_mqd;
>   		mqd->init_mqd = init_mqd;
>   		mqd->uninit_mqd = uninit_mqd;
>   		mqd->load_mqd = load_mqd;
> @@ -471,6 +459,7 @@ struct mqd_manager *mqd_manager_init_vi(enum KFD_MQD_TYPE type,
>   #endif
>   		break;
>   	case KFD_MQD_TYPE_HIQ:
> +		mqd->allocate_mqd = allocate_hiq_mqd;
>   		mqd->init_mqd = init_mqd_hiq;
>   		mqd->uninit_mqd = uninit_mqd_hiq_sdma;
>   		mqd->load_mqd = load_mqd;
> @@ -483,6 +472,7 @@ struct mqd_manager *mqd_manager_init_vi(enum KFD_MQD_TYPE type,
>   #endif
>   		break;
>   	case KFD_MQD_TYPE_DIQ:
> +		mqd->allocate_mqd = allocate_hiq_mqd;
>   		mqd->init_mqd = init_mqd_hiq;
>   		mqd->uninit_mqd = uninit_mqd;
>   		mqd->load_mqd = load_mqd;
> @@ -495,6 +485,7 @@ struct mqd_manager *mqd_manager_init_vi(enum KFD_MQD_TYPE type,
>   #endif
>   		break;
>   	case KFD_MQD_TYPE_SDMA:
> +		mqd->allocate_mqd = allocate_sdma_mqd;
>   		mqd->init_mqd = init_mqd_sdma;
>   		mqd->uninit_mqd = uninit_mqd_hiq_sdma;
>   		mqd->load_mqd = load_mqd_sdma;
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 5/6] drm/amdkfd: Fix a circular lock dependency

[Zeng, Oak]

The idea to break the circular lock dependency is to move allocate_mqd
out of dqm lock protection. See callstack #1 below.

[   59.510149] [drm] Initialized amdgpu 3.30.0 20150101 for 0000:04:00.0 on minor 0

[  513.604034] ======================================================
[  513.604205] WARNING: possible circular locking dependency detected
[  513.604375] 4.18.0-kfd-root #2 Tainted: G        W
[  513.604530] ------------------------------------------------------
[  513.604699] kswapd0/611 is trying to acquire lock:
[  513.604840] 00000000d254022e (&dqm->lock_hidden){+.+.}, at: evict_process_queues_nocpsch+0x26/0x140 [amdgpu]
[  513.605150]
               but task is already holding lock:
[  513.605307] 00000000961547fc (&anon_vma->rwsem){++++}, at: page_lock_anon_vma_read+0xe4/0x250
[  513.605540]
               which lock already depends on the new lock.

[  513.605747]
               the existing dependency chain (in reverse order) is:
[  513.605944]
               -> #4 (&anon_vma->rwsem){++++}:
[  513.606106]        __vma_adjust+0x147/0x7f0
[  513.606231]        __split_vma+0x179/0x190
[  513.606353]        mprotect_fixup+0x217/0x260
[  513.606553]        do_mprotect_pkey+0x211/0x380
[  513.606752]        __x64_sys_mprotect+0x1b/0x20
[  513.606954]        do_syscall_64+0x50/0x1a0
[  513.607149]        entry_SYSCALL_64_after_hwframe+0x49/0xbe
[  513.607380]
               -> #3 (&mapping->i_mmap_rwsem){++++}:
[  513.607678]        rmap_walk_file+0x1f0/0x280
[  513.607887]        page_referenced+0xdd/0x180
[  513.608081]        shrink_page_list+0x853/0xcb0
[  513.608279]        shrink_inactive_list+0x33b/0x700
[  513.608483]        shrink_node_memcg+0x37a/0x7f0
[  513.608682]        shrink_node+0xd8/0x490
[  513.608869]        balance_pgdat+0x18b/0x3b0
[  513.609062]        kswapd+0x203/0x5c0
[  513.609241]        kthread+0x100/0x140
[  513.609420]        ret_from_fork+0x24/0x30
[  513.609607]
               -> #2 (fs_reclaim){+.+.}:
[  513.609883]        kmem_cache_alloc_trace+0x34/0x2e0
[  513.610093]        reservation_object_reserve_shared+0x139/0x300
[  513.610326]        ttm_bo_init_reserved+0x291/0x480 [ttm]
[  513.610567]        amdgpu_bo_do_create+0x1d2/0x650 [amdgpu]
[  513.610811]        amdgpu_bo_create+0x40/0x1f0 [amdgpu]
[  513.611041]        amdgpu_bo_create_reserved+0x249/0x2d0 [amdgpu]
[  513.611290]        amdgpu_bo_create_kernel+0x12/0x70 [amdgpu]
[  513.611584]        amdgpu_ttm_init+0x2cb/0x560 [amdgpu]
[  513.611823]        gmc_v9_0_sw_init+0x400/0x750 [amdgpu]
[  513.612491]        amdgpu_device_init+0x14eb/0x1990 [amdgpu]
[  513.612730]        amdgpu_driver_load_kms+0x78/0x290 [amdgpu]
[  513.612958]        drm_dev_register+0x111/0x1a0
[  513.613171]        amdgpu_pci_probe+0x11c/0x1e0 [amdgpu]
[  513.613389]        local_pci_probe+0x3f/0x90
[  513.613581]        pci_device_probe+0x102/0x1c0
[  513.613779]        driver_probe_device+0x2a7/0x480
[  513.613984]        __driver_attach+0x10a/0x110
[  513.614179]        bus_for_each_dev+0x67/0xc0
[  513.614372]        bus_add_driver+0x1eb/0x260
[  513.614565]        driver_register+0x5b/0xe0
[  513.614756]        do_one_initcall+0xac/0x357
[  513.614952]        do_init_module+0x5b/0x213
[  513.615145]        load_module+0x2542/0x2d30
[  513.615337]        __do_sys_finit_module+0xd2/0x100
[  513.615541]        do_syscall_64+0x50/0x1a0
[  513.615731]        entry_SYSCALL_64_after_hwframe+0x49/0xbe
[  513.615963]
               -> #1 (reservation_ww_class_mutex){+.+.}:
[  513.616293]        amdgpu_amdkfd_alloc_gtt_mem+0xcf/0x2c0 [amdgpu]
[  513.616554]        init_mqd+0x223/0x260 [amdgpu]
[  513.616779]        create_queue_nocpsch+0x4d9/0x600 [amdgpu]
[  513.617031]        pqm_create_queue+0x37c/0x520 [amdgpu]
[  513.617270]        kfd_ioctl_create_queue+0x2f9/0x650 [amdgpu]
[  513.617522]        kfd_ioctl+0x202/0x350 [amdgpu]
[  513.617724]        do_vfs_ioctl+0x9f/0x6c0
[  513.617914]        ksys_ioctl+0x66/0x70
[  513.618095]        __x64_sys_ioctl+0x16/0x20
[  513.618286]        do_syscall_64+0x50/0x1a0
[  513.618476]        entry_SYSCALL_64_after_hwframe+0x49/0xbe
[  513.618695]
               -> #0 (&dqm->lock_hidden){+.+.}:
[  513.618984]        __mutex_lock+0x98/0x970
[  513.619197]        evict_process_queues_nocpsch+0x26/0x140 [amdgpu]
[  513.619459]        kfd_process_evict_queues+0x3b/0xb0 [amdgpu]
[  513.619710]        kgd2kfd_quiesce_mm+0x1c/0x40 [amdgpu]
[  513.620103]        amdgpu_amdkfd_evict_userptr+0x38/0x70 [amdgpu]
[  513.620363]        amdgpu_mn_invalidate_range_start_hsa+0xa6/0xc0 [amdgpu]
[  513.620614]        __mmu_notifier_invalidate_range_start+0x70/0xb0
[  513.620851]        try_to_unmap_one+0x7fc/0x8f0
[  513.621049]        rmap_walk_anon+0x121/0x290
[  513.621242]        try_to_unmap+0x93/0xf0
[  513.621428]        shrink_page_list+0x606/0xcb0
[  513.621625]        shrink_inactive_list+0x33b/0x700
[  513.621835]        shrink_node_memcg+0x37a/0x7f0
[  513.622034]        shrink_node+0xd8/0x490
[  513.622219]        balance_pgdat+0x18b/0x3b0
[  513.622410]        kswapd+0x203/0x5c0
[  513.622589]        kthread+0x100/0x140
[  513.622769]        ret_from_fork+0x24/0x30
[  513.622957]
               other info that might help us debug this:

[  513.623354] Chain exists of:
                 &dqm->lock_hidden --> &mapping->i_mmap_rwsem --> &anon_vma->rwsem

[  513.623900]  Possible unsafe locking scenario:

[  513.624189]        CPU0                    CPU1
[  513.624397]        ----                    ----
[  513.624594]   lock(&anon_vma->rwsem);
[  513.624771]                                lock(&mapping->i_mmap_rwsem);
[  513.625020]                                lock(&anon_vma->rwsem);
[  513.625253]   lock(&dqm->lock_hidden);
[  513.625433]
                *** DEADLOCK ***

[  513.625783] 3 locks held by kswapd0/611:
[  513.625967]  #0: 00000000f14edf84 (fs_reclaim){+.+.}, at: __fs_reclaim_acquire+0x5/0x30
[  513.626309]  #1: 00000000961547fc (&anon_vma->rwsem){++++}, at: page_lock_anon_vma_read+0xe4/0x250
[  513.626671]  #2: 0000000067b5cd12 (srcu){....}, at: __mmu_notifier_invalidate_range_start+0x5/0xb0
[  513.627037]
               stack backtrace:
[  513.627292] CPU: 0 PID: 611 Comm: kswapd0 Tainted: G        W         4.18.0-kfd-root #2
[  513.627632] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
[  513.627990] Call Trace:
[  513.628143]  dump_stack+0x7c/0xbb
[  513.628315]  print_circular_bug.isra.37+0x21b/0x228
[  513.628581]  __lock_acquire+0xf7d/0x1470
[  513.628782]  ? unwind_next_frame+0x6c/0x4f0
[  513.628974]  ? lock_acquire+0xec/0x1e0
[  513.629154]  lock_acquire+0xec/0x1e0
[  513.629357]  ? evict_process_queues_nocpsch+0x26/0x140 [amdgpu]
[  513.629587]  __mutex_lock+0x98/0x970
[  513.629790]  ? evict_process_queues_nocpsch+0x26/0x140 [amdgpu]
[  513.630047]  ? evict_process_queues_nocpsch+0x26/0x140 [amdgpu]
[  513.630309]  ? evict_process_queues_nocpsch+0x26/0x140 [amdgpu]
[  513.630562]  evict_process_queues_nocpsch+0x26/0x140 [amdgpu]
[  513.630816]  kfd_process_evict_queues+0x3b/0xb0 [amdgpu]
[  513.631057]  kgd2kfd_quiesce_mm+0x1c/0x40 [amdgpu]
[  513.631288]  amdgpu_amdkfd_evict_userptr+0x38/0x70 [amdgpu]
[  513.631536]  amdgpu_mn_invalidate_range_start_hsa+0xa6/0xc0 [amdgpu]
[  513.632076]  __mmu_notifier_invalidate_range_start+0x70/0xb0
[  513.632299]  try_to_unmap_one+0x7fc/0x8f0
[  513.632487]  ? page_lock_anon_vma_read+0x68/0x250
[  513.632690]  rmap_walk_anon+0x121/0x290
[  513.632875]  try_to_unmap+0x93/0xf0
[  513.633050]  ? page_remove_rmap+0x330/0x330
[  513.633239]  ? rcu_read_unlock+0x60/0x60
[  513.633422]  ? page_get_anon_vma+0x160/0x160
[  513.633613]  shrink_page_list+0x606/0xcb0
[  513.633800]  shrink_inactive_list+0x33b/0x700
[  513.633997]  shrink_node_memcg+0x37a/0x7f0
[  513.634186]  ? shrink_node+0xd8/0x490
[  513.634363]  shrink_node+0xd8/0x490
[  513.634537]  balance_pgdat+0x18b/0x3b0
[  513.634718]  kswapd+0x203/0x5c0
[  513.634887]  ? wait_woken+0xb0/0xb0
[  513.635062]  kthread+0x100/0x140
[  513.635231]  ? balance_pgdat+0x3b0/0x3b0
[  513.635414]  ? kthread_delayed_work_timer_fn+0x80/0x80
[  513.635626]  ret_from_fork+0x24/0x30
[  513.636042] Evicting PASID 32768 queues
[  513.936236] Restoring PASID 32768 queues
[  524.708912] Evicting PASID 32768 queues
[  524.999875] Restoring PASID 32768 queues

Change-Id: I334c8c9329be12e468ea7aabc878842ec003bd8e
Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>
---
 .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c   | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index 787b5be..6b1a2ee 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -274,6 +274,12 @@ static int create_queue_nocpsch(struct device_queue_manager *dqm,
 
 	print_queue(q);
 
+	mqd_mgr = dqm->mqd_mgrs[get_mqd_type_from_queue_type(
+			q->properties.type)];
+	q->mqd_mem_obj = mqd_mgr->allocate_mqd(mqd_mgr->dev, &q->properties);
+	if (!q->mqd_mem_obj)
+		return -ENOMEM;
+
 	dqm_lock(dqm);
 
 	if (dqm->total_queue_count >= max_num_of_queues_per_device) {
@@ -299,8 +305,6 @@ static int create_queue_nocpsch(struct device_queue_manager *dqm,
 	q->properties.tba_addr = qpd->tba_addr;
 	q->properties.tma_addr = qpd->tma_addr;
 
-	mqd_mgr = dqm->mqd_mgrs[get_mqd_type_from_queue_type(
-			q->properties.type)];
 	if (q->properties.type == KFD_QUEUE_TYPE_COMPUTE) {
 		retval = allocate_hqd(dqm, q);
 		if (retval)
@@ -319,13 +323,10 @@ static int create_queue_nocpsch(struct device_queue_manager *dqm,
 	if (retval)
 		goto out_deallocate_hqd;
 
-	q->mqd_mem_obj = mqd_mgr->allocate_mqd(mqd_mgr->dev, &q->properties);
-	if (!q->mqd_mem_obj)
-		goto out_deallocate_doorbell;
 	retval = mqd_mgr->init_mqd(mqd_mgr, &q->mqd, q->mqd_mem_obj,
 				&q->gart_mqd_addr, &q->properties);
 	if (retval)
-		goto out_uninit_mqd;
+		goto out_deallocate_doorbell;
 
 	if (q->properties.is_active) {
 
@@ -336,7 +337,7 @@ static int create_queue_nocpsch(struct device_queue_manager *dqm,
 			retval = mqd_mgr->load_mqd(mqd_mgr, q->mqd, q->pipe,
 					q->queue, &q->properties, current->mm);
 		if (retval)
-			goto out_uninit_mqd;
+			goto out_deallocate_doorbell;
 	}
 
 	list_add(&q->list, &qpd->queues_list);
@@ -356,10 +357,9 @@ static int create_queue_nocpsch(struct device_queue_manager *dqm,
 	dqm->total_queue_count++;
 	pr_debug("Total of %d queues are accountable so far\n",
 			dqm->total_queue_count);
-	goto out_unlock;
+	dqm_unlock(dqm);
+	return retval;
 
-out_uninit_mqd:
-	mqd_mgr->uninit_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
 out_deallocate_doorbell:
 	deallocate_doorbell(qpd, q);
 out_deallocate_hqd:
@@ -373,6 +373,7 @@ static int create_queue_nocpsch(struct device_queue_manager *dqm,
 		deallocate_vmid(dqm, qpd, q);
 out_unlock:
 	dqm_unlock(dqm);
+	mqd_mgr->uninit_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
 	return retval;
 }
 
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 5/6] drm/amdkfd: Fix a circular lock dependency

[Kuehling, Felix]

This patch looks good to me, but it'll probably change a little if you 
implement my suggestions for patch 4.

Regards,
   Felix

On 2019-06-05 12:06 p.m., Zeng, Oak wrote:
> The idea to break the circular lock dependency is to move allocate_mqd
> out of dqm lock protection. See callstack #1 below.
>
> [   59.510149] [drm] Initialized amdgpu 3.30.0 20150101 for 0000:04:00.0 on minor 0
>
> [  513.604034] ======================================================
> [  513.604205] WARNING: possible circular locking dependency detected
> [  513.604375] 4.18.0-kfd-root #2 Tainted: G        W
> [  513.604530] ------------------------------------------------------
> [  513.604699] kswapd0/611 is trying to acquire lock:
> [  513.604840] 00000000d254022e (&dqm->lock_hidden){+.+.}, at: evict_process_queues_nocpsch+0x26/0x140 [amdgpu]
> [  513.605150]
>                 but task is already holding lock:
> [  513.605307] 00000000961547fc (&anon_vma->rwsem){++++}, at: page_lock_anon_vma_read+0xe4/0x250
> [  513.605540]
>                 which lock already depends on the new lock.
>
> [  513.605747]
>                 the existing dependency chain (in reverse order) is:
> [  513.605944]
>                 -> #4 (&anon_vma->rwsem){++++}:
> [  513.606106]        __vma_adjust+0x147/0x7f0
> [  513.606231]        __split_vma+0x179/0x190
> [  513.606353]        mprotect_fixup+0x217/0x260
> [  513.606553]        do_mprotect_pkey+0x211/0x380
> [  513.606752]        __x64_sys_mprotect+0x1b/0x20
> [  513.606954]        do_syscall_64+0x50/0x1a0
> [  513.607149]        entry_SYSCALL_64_after_hwframe+0x49/0xbe
> [  513.607380]
>                 -> #3 (&mapping->i_mmap_rwsem){++++}:
> [  513.607678]        rmap_walk_file+0x1f0/0x280
> [  513.607887]        page_referenced+0xdd/0x180
> [  513.608081]        shrink_page_list+0x853/0xcb0
> [  513.608279]        shrink_inactive_list+0x33b/0x700
> [  513.608483]        shrink_node_memcg+0x37a/0x7f0
> [  513.608682]        shrink_node+0xd8/0x490
> [  513.608869]        balance_pgdat+0x18b/0x3b0
> [  513.609062]        kswapd+0x203/0x5c0
> [  513.609241]        kthread+0x100/0x140
> [  513.609420]        ret_from_fork+0x24/0x30
> [  513.609607]
>                 -> #2 (fs_reclaim){+.+.}:
> [  513.609883]        kmem_cache_alloc_trace+0x34/0x2e0
> [  513.610093]        reservation_object_reserve_shared+0x139/0x300
> [  513.610326]        ttm_bo_init_reserved+0x291/0x480 [ttm]
> [  513.610567]        amdgpu_bo_do_create+0x1d2/0x650 [amdgpu]
> [  513.610811]        amdgpu_bo_create+0x40/0x1f0 [amdgpu]
> [  513.611041]        amdgpu_bo_create_reserved+0x249/0x2d0 [amdgpu]
> [  513.611290]        amdgpu_bo_create_kernel+0x12/0x70 [amdgpu]
> [  513.611584]        amdgpu_ttm_init+0x2cb/0x560 [amdgpu]
> [  513.611823]        gmc_v9_0_sw_init+0x400/0x750 [amdgpu]
> [  513.612491]        amdgpu_device_init+0x14eb/0x1990 [amdgpu]
> [  513.612730]        amdgpu_driver_load_kms+0x78/0x290 [amdgpu]
> [  513.612958]        drm_dev_register+0x111/0x1a0
> [  513.613171]        amdgpu_pci_probe+0x11c/0x1e0 [amdgpu]
> [  513.613389]        local_pci_probe+0x3f/0x90
> [  513.613581]        pci_device_probe+0x102/0x1c0
> [  513.613779]        driver_probe_device+0x2a7/0x480
> [  513.613984]        __driver_attach+0x10a/0x110
> [  513.614179]        bus_for_each_dev+0x67/0xc0
> [  513.614372]        bus_add_driver+0x1eb/0x260
> [  513.614565]        driver_register+0x5b/0xe0
> [  513.614756]        do_one_initcall+0xac/0x357
> [  513.614952]        do_init_module+0x5b/0x213
> [  513.615145]        load_module+0x2542/0x2d30
> [  513.615337]        __do_sys_finit_module+0xd2/0x100
> [  513.615541]        do_syscall_64+0x50/0x1a0
> [  513.615731]        entry_SYSCALL_64_after_hwframe+0x49/0xbe
> [  513.615963]
>                 -> #1 (reservation_ww_class_mutex){+.+.}:
> [  513.616293]        amdgpu_amdkfd_alloc_gtt_mem+0xcf/0x2c0 [amdgpu]
> [  513.616554]        init_mqd+0x223/0x260 [amdgpu]
> [  513.616779]        create_queue_nocpsch+0x4d9/0x600 [amdgpu]
> [  513.617031]        pqm_create_queue+0x37c/0x520 [amdgpu]
> [  513.617270]        kfd_ioctl_create_queue+0x2f9/0x650 [amdgpu]
> [  513.617522]        kfd_ioctl+0x202/0x350 [amdgpu]
> [  513.617724]        do_vfs_ioctl+0x9f/0x6c0
> [  513.617914]        ksys_ioctl+0x66/0x70
> [  513.618095]        __x64_sys_ioctl+0x16/0x20
> [  513.618286]        do_syscall_64+0x50/0x1a0
> [  513.618476]        entry_SYSCALL_64_after_hwframe+0x49/0xbe
> [  513.618695]
>                 -> #0 (&dqm->lock_hidden){+.+.}:
> [  513.618984]        __mutex_lock+0x98/0x970
> [  513.619197]        evict_process_queues_nocpsch+0x26/0x140 [amdgpu]
> [  513.619459]        kfd_process_evict_queues+0x3b/0xb0 [amdgpu]
> [  513.619710]        kgd2kfd_quiesce_mm+0x1c/0x40 [amdgpu]
> [  513.620103]        amdgpu_amdkfd_evict_userptr+0x38/0x70 [amdgpu]
> [  513.620363]        amdgpu_mn_invalidate_range_start_hsa+0xa6/0xc0 [amdgpu]
> [  513.620614]        __mmu_notifier_invalidate_range_start+0x70/0xb0
> [  513.620851]        try_to_unmap_one+0x7fc/0x8f0
> [  513.621049]        rmap_walk_anon+0x121/0x290
> [  513.621242]        try_to_unmap+0x93/0xf0
> [  513.621428]        shrink_page_list+0x606/0xcb0
> [  513.621625]        shrink_inactive_list+0x33b/0x700
> [  513.621835]        shrink_node_memcg+0x37a/0x7f0
> [  513.622034]        shrink_node+0xd8/0x490
> [  513.622219]        balance_pgdat+0x18b/0x3b0
> [  513.622410]        kswapd+0x203/0x5c0
> [  513.622589]        kthread+0x100/0x140
> [  513.622769]        ret_from_fork+0x24/0x30
> [  513.622957]
>                 other info that might help us debug this:
>
> [  513.623354] Chain exists of:
>                   &dqm->lock_hidden --> &mapping->i_mmap_rwsem --> &anon_vma->rwsem
>
> [  513.623900]  Possible unsafe locking scenario:
>
> [  513.624189]        CPU0                    CPU1
> [  513.624397]        ----                    ----
> [  513.624594]   lock(&anon_vma->rwsem);
> [  513.624771]                                lock(&mapping->i_mmap_rwsem);
> [  513.625020]                                lock(&anon_vma->rwsem);
> [  513.625253]   lock(&dqm->lock_hidden);
> [  513.625433]
>                  *** DEADLOCK ***
>
> [  513.625783] 3 locks held by kswapd0/611:
> [  513.625967]  #0: 00000000f14edf84 (fs_reclaim){+.+.}, at: __fs_reclaim_acquire+0x5/0x30
> [  513.626309]  #1: 00000000961547fc (&anon_vma->rwsem){++++}, at: page_lock_anon_vma_read+0xe4/0x250
> [  513.626671]  #2: 0000000067b5cd12 (srcu){....}, at: __mmu_notifier_invalidate_range_start+0x5/0xb0
> [  513.627037]
>                 stack backtrace:
> [  513.627292] CPU: 0 PID: 611 Comm: kswapd0 Tainted: G        W         4.18.0-kfd-root #2
> [  513.627632] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
> [  513.627990] Call Trace:
> [  513.628143]  dump_stack+0x7c/0xbb
> [  513.628315]  print_circular_bug.isra.37+0x21b/0x228
> [  513.628581]  __lock_acquire+0xf7d/0x1470
> [  513.628782]  ? unwind_next_frame+0x6c/0x4f0
> [  513.628974]  ? lock_acquire+0xec/0x1e0
> [  513.629154]  lock_acquire+0xec/0x1e0
> [  513.629357]  ? evict_process_queues_nocpsch+0x26/0x140 [amdgpu]
> [  513.629587]  __mutex_lock+0x98/0x970
> [  513.629790]  ? evict_process_queues_nocpsch+0x26/0x140 [amdgpu]
> [  513.630047]  ? evict_process_queues_nocpsch+0x26/0x140 [amdgpu]
> [  513.630309]  ? evict_process_queues_nocpsch+0x26/0x140 [amdgpu]
> [  513.630562]  evict_process_queues_nocpsch+0x26/0x140 [amdgpu]
> [  513.630816]  kfd_process_evict_queues+0x3b/0xb0 [amdgpu]
> [  513.631057]  kgd2kfd_quiesce_mm+0x1c/0x40 [amdgpu]
> [  513.631288]  amdgpu_amdkfd_evict_userptr+0x38/0x70 [amdgpu]
> [  513.631536]  amdgpu_mn_invalidate_range_start_hsa+0xa6/0xc0 [amdgpu]
> [  513.632076]  __mmu_notifier_invalidate_range_start+0x70/0xb0
> [  513.632299]  try_to_unmap_one+0x7fc/0x8f0
> [  513.632487]  ? page_lock_anon_vma_read+0x68/0x250
> [  513.632690]  rmap_walk_anon+0x121/0x290
> [  513.632875]  try_to_unmap+0x93/0xf0
> [  513.633050]  ? page_remove_rmap+0x330/0x330
> [  513.633239]  ? rcu_read_unlock+0x60/0x60
> [  513.633422]  ? page_get_anon_vma+0x160/0x160
> [  513.633613]  shrink_page_list+0x606/0xcb0
> [  513.633800]  shrink_inactive_list+0x33b/0x700
> [  513.633997]  shrink_node_memcg+0x37a/0x7f0
> [  513.634186]  ? shrink_node+0xd8/0x490
> [  513.634363]  shrink_node+0xd8/0x490
> [  513.634537]  balance_pgdat+0x18b/0x3b0
> [  513.634718]  kswapd+0x203/0x5c0
> [  513.634887]  ? wait_woken+0xb0/0xb0
> [  513.635062]  kthread+0x100/0x140
> [  513.635231]  ? balance_pgdat+0x3b0/0x3b0
> [  513.635414]  ? kthread_delayed_work_timer_fn+0x80/0x80
> [  513.635626]  ret_from_fork+0x24/0x30
> [  513.636042] Evicting PASID 32768 queues
> [  513.936236] Restoring PASID 32768 queues
> [  524.708912] Evicting PASID 32768 queues
> [  524.999875] Restoring PASID 32768 queues
>
> Change-Id: I334c8c9329be12e468ea7aabc878842ec003bd8e
> Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>
> ---
>   .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c   | 21 +++++++++++----------
>   1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> index 787b5be..6b1a2ee 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -274,6 +274,12 @@ static int create_queue_nocpsch(struct device_queue_manager *dqm,
>   
>   	print_queue(q);
>   
> +	mqd_mgr = dqm->mqd_mgrs[get_mqd_type_from_queue_type(
> +			q->properties.type)];
> +	q->mqd_mem_obj = mqd_mgr->allocate_mqd(mqd_mgr->dev, &q->properties);
> +	if (!q->mqd_mem_obj)
> +		return -ENOMEM;
> +
>   	dqm_lock(dqm);
>   
>   	if (dqm->total_queue_count >= max_num_of_queues_per_device) {
> @@ -299,8 +305,6 @@ static int create_queue_nocpsch(struct device_queue_manager *dqm,
>   	q->properties.tba_addr = qpd->tba_addr;
>   	q->properties.tma_addr = qpd->tma_addr;
>   
> -	mqd_mgr = dqm->mqd_mgrs[get_mqd_type_from_queue_type(
> -			q->properties.type)];
>   	if (q->properties.type == KFD_QUEUE_TYPE_COMPUTE) {
>   		retval = allocate_hqd(dqm, q);
>   		if (retval)
> @@ -319,13 +323,10 @@ static int create_queue_nocpsch(struct device_queue_manager *dqm,
>   	if (retval)
>   		goto out_deallocate_hqd;
>   
> -	q->mqd_mem_obj = mqd_mgr->allocate_mqd(mqd_mgr->dev, &q->properties);
> -	if (!q->mqd_mem_obj)
> -		goto out_deallocate_doorbell;
>   	retval = mqd_mgr->init_mqd(mqd_mgr, &q->mqd, q->mqd_mem_obj,
>   				&q->gart_mqd_addr, &q->properties);
>   	if (retval)
> -		goto out_uninit_mqd;
> +		goto out_deallocate_doorbell;
>   
>   	if (q->properties.is_active) {
>   
> @@ -336,7 +337,7 @@ static int create_queue_nocpsch(struct device_queue_manager *dqm,
>   			retval = mqd_mgr->load_mqd(mqd_mgr, q->mqd, q->pipe,
>   					q->queue, &q->properties, current->mm);
>   		if (retval)
> -			goto out_uninit_mqd;
> +			goto out_deallocate_doorbell;
>   	}
>   
>   	list_add(&q->list, &qpd->queues_list);
> @@ -356,10 +357,9 @@ static int create_queue_nocpsch(struct device_queue_manager *dqm,
>   	dqm->total_queue_count++;
>   	pr_debug("Total of %d queues are accountable so far\n",
>   			dqm->total_queue_count);
> -	goto out_unlock;
> +	dqm_unlock(dqm);
> +	return retval;
>   
> -out_uninit_mqd:
> -	mqd_mgr->uninit_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
>   out_deallocate_doorbell:
>   	deallocate_doorbell(qpd, q);
>   out_deallocate_hqd:
> @@ -373,6 +373,7 @@ static int create_queue_nocpsch(struct device_queue_manager *dqm,
>   		deallocate_vmid(dqm, qpd, q);
>   out_unlock:
>   	dqm_unlock(dqm);
> +	mqd_mgr->uninit_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
>   	return retval;
>   }
>   
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 6/6] drm/amdkfd: Fix sdma queue allocate race condition

[Zeng, Oak]

SDMA queue allocation requires the dqm lock as it modify
the global dqm members. Introduce functions to allocate/deallocate
in locked/unlocked circumstance.

Change-Id: Id3084524c5f65d9629b12cf6b4862a7516945cb1
Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>
---
 .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c  | 34 ++++++++++++++++------
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index 6b1a2ee..52e4ede 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -53,6 +53,8 @@ static int map_queues_cpsch(struct device_queue_manager *dqm);
 
 static void deallocate_sdma_queue(struct device_queue_manager *dqm,
 				struct queue *q);
+static void deallocate_sdma_queue_locked(struct device_queue_manager *dqm,
+				struct queue *q);
 
 static inline void deallocate_hqd(struct device_queue_manager *dqm,
 				struct queue *q);
@@ -434,10 +436,10 @@ static int destroy_queue_nocpsch_locked(struct device_queue_manager *dqm,
 		deallocate_hqd(dqm, q);
 	} else if (q->properties.type == KFD_QUEUE_TYPE_SDMA) {
 		dqm->sdma_queue_count--;
-		deallocate_sdma_queue(dqm, q);
+		deallocate_sdma_queue_locked(dqm, q);
 	} else if (q->properties.type == KFD_QUEUE_TYPE_SDMA_XGMI) {
 		dqm->xgmi_sdma_queue_count--;
-		deallocate_sdma_queue(dqm, q);
+		deallocate_sdma_queue_locked(dqm, q);
 	} else {
 		pr_debug("q->properties.type %d is invalid\n",
 				q->properties.type);
@@ -914,9 +916,12 @@ static int allocate_sdma_queue(struct device_queue_manager *dqm,
 {
 	int bit;
 
+	dqm_lock(dqm);
 	if (q->properties.type == KFD_QUEUE_TYPE_SDMA) {
-		if (dqm->sdma_bitmap == 0)
+		if (dqm->sdma_bitmap == 0) {
+			dqm_unlock(dqm);
 			return -ENOMEM;
+		}
 		bit = __ffs64(dqm->sdma_bitmap);
 		dqm->sdma_bitmap &= ~(1ULL << bit);
 		q->sdma_id = bit;
@@ -925,8 +930,10 @@ static int allocate_sdma_queue(struct device_queue_manager *dqm,
 		q->properties.sdma_queue_id = q->sdma_id /
 				get_num_sdma_engines(dqm);
 	} else if (q->properties.type == KFD_QUEUE_TYPE_SDMA_XGMI) {
-		if (dqm->xgmi_sdma_bitmap == 0)
+		if (dqm->xgmi_sdma_bitmap == 0) {
+			dqm_unlock(dqm);
 			return -ENOMEM;
+		}
 		bit = __ffs64(dqm->xgmi_sdma_bitmap);
 		dqm->xgmi_sdma_bitmap &= ~(1ULL << bit);
 		q->sdma_id = bit;
@@ -942,13 +949,14 @@ static int allocate_sdma_queue(struct device_queue_manager *dqm,
 				get_num_xgmi_sdma_engines(dqm);
 	}
 
+	dqm_unlock(dqm);
 	pr_debug("SDMA engine id: %d\n", q->properties.sdma_engine_id);
 	pr_debug("SDMA queue id: %d\n", q->properties.sdma_queue_id);
 
 	return 0;
 }
 
-static void deallocate_sdma_queue(struct device_queue_manager *dqm,
+static void deallocate_sdma_queue_locked(struct device_queue_manager *dqm,
 				struct queue *q)
 {
 	if (q->properties.type == KFD_QUEUE_TYPE_SDMA) {
@@ -962,6 +970,14 @@ static void deallocate_sdma_queue(struct device_queue_manager *dqm,
 	}
 }
 
+static void deallocate_sdma_queue(struct device_queue_manager *dqm,
+				struct queue *q)
+{
+	dqm_lock(dqm);
+	deallocate_sdma_queue_locked(dqm, q);
+	dqm_unlock(dqm);
+}
+
 /*
  * Device Queue Manager implementation for cp scheduler
  */
@@ -1356,10 +1372,10 @@ static int destroy_queue_cpsch(struct device_queue_manager *dqm,
 
 	if (q->properties.type == KFD_QUEUE_TYPE_SDMA) {
 		dqm->sdma_queue_count--;
-		deallocate_sdma_queue(dqm, q);
+		deallocate_sdma_queue_locked(dqm, q);
 	} else if (q->properties.type == KFD_QUEUE_TYPE_SDMA_XGMI) {
 		dqm->xgmi_sdma_queue_count--;
-		deallocate_sdma_queue(dqm, q);
+		deallocate_sdma_queue_locked(dqm, q);
 	}
 
 	list_del(&q->list);
@@ -1585,10 +1601,10 @@ static int process_termination_cpsch(struct device_queue_manager *dqm,
 	list_for_each_entry(q, &qpd->queues_list, list) {
 		if (q->properties.type == KFD_QUEUE_TYPE_SDMA) {
 			dqm->sdma_queue_count--;
-			deallocate_sdma_queue(dqm, q);
+			deallocate_sdma_queue_locked(dqm, q);
 		} else if (q->properties.type == KFD_QUEUE_TYPE_SDMA_XGMI) {
 			dqm->xgmi_sdma_queue_count--;
-			deallocate_sdma_queue(dqm, q);
+			deallocate_sdma_queue_locked(dqm, q);
 		}
 
 		if (q->properties.is_active)
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 6/6] drm/amdkfd: Fix sdma queue allocate race condition

[Kuehling, Felix]

I think the simpler way to fix this, is to restructure 
create_queue_cpsch similar to the nocpsch version where we allocate the 
MQD early and take the DQM lock right after that. That way you don't 
need locked and unlocked variants of allocate_sdma_queue and 
deallocate_sdma_queue.

Regards,
   Felix


On 2019-06-05 12:06 p.m., Zeng, Oak wrote:
> SDMA queue allocation requires the dqm lock as it modify
> the global dqm members. Introduce functions to allocate/deallocate
> in locked/unlocked circumstance.
>
> Change-Id: Id3084524c5f65d9629b12cf6b4862a7516945cb1
> Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>
> ---
>   .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c  | 34 ++++++++++++++++------
>   1 file changed, 25 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> index 6b1a2ee..52e4ede 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -53,6 +53,8 @@ static int map_queues_cpsch(struct device_queue_manager *dqm);
>   
>   static void deallocate_sdma_queue(struct device_queue_manager *dqm,
>   				struct queue *q);
> +static void deallocate_sdma_queue_locked(struct device_queue_manager *dqm,
> +				struct queue *q);
>   
>   static inline void deallocate_hqd(struct device_queue_manager *dqm,
>   				struct queue *q);
> @@ -434,10 +436,10 @@ static int destroy_queue_nocpsch_locked(struct device_queue_manager *dqm,
>   		deallocate_hqd(dqm, q);
>   	} else if (q->properties.type == KFD_QUEUE_TYPE_SDMA) {
>   		dqm->sdma_queue_count--;
> -		deallocate_sdma_queue(dqm, q);
> +		deallocate_sdma_queue_locked(dqm, q);
>   	} else if (q->properties.type == KFD_QUEUE_TYPE_SDMA_XGMI) {
>   		dqm->xgmi_sdma_queue_count--;
> -		deallocate_sdma_queue(dqm, q);
> +		deallocate_sdma_queue_locked(dqm, q);
>   	} else {
>   		pr_debug("q->properties.type %d is invalid\n",
>   				q->properties.type);
> @@ -914,9 +916,12 @@ static int allocate_sdma_queue(struct device_queue_manager *dqm,
>   {
>   	int bit;
>   
> +	dqm_lock(dqm);
>   	if (q->properties.type == KFD_QUEUE_TYPE_SDMA) {
> -		if (dqm->sdma_bitmap == 0)
> +		if (dqm->sdma_bitmap == 0) {
> +			dqm_unlock(dqm);
>   			return -ENOMEM;
> +		}
>   		bit = __ffs64(dqm->sdma_bitmap);
>   		dqm->sdma_bitmap &= ~(1ULL << bit);
>   		q->sdma_id = bit;
> @@ -925,8 +930,10 @@ static int allocate_sdma_queue(struct device_queue_manager *dqm,
>   		q->properties.sdma_queue_id = q->sdma_id /
>   				get_num_sdma_engines(dqm);
>   	} else if (q->properties.type == KFD_QUEUE_TYPE_SDMA_XGMI) {
> -		if (dqm->xgmi_sdma_bitmap == 0)
> +		if (dqm->xgmi_sdma_bitmap == 0) {
> +			dqm_unlock(dqm);
>   			return -ENOMEM;
> +		}
>   		bit = __ffs64(dqm->xgmi_sdma_bitmap);
>   		dqm->xgmi_sdma_bitmap &= ~(1ULL << bit);
>   		q->sdma_id = bit;
> @@ -942,13 +949,14 @@ static int allocate_sdma_queue(struct device_queue_manager *dqm,
>   				get_num_xgmi_sdma_engines(dqm);
>   	}
>   
> +	dqm_unlock(dqm);
>   	pr_debug("SDMA engine id: %d\n", q->properties.sdma_engine_id);
>   	pr_debug("SDMA queue id: %d\n", q->properties.sdma_queue_id);
>   
>   	return 0;
>   }
>   
> -static void deallocate_sdma_queue(struct device_queue_manager *dqm,
> +static void deallocate_sdma_queue_locked(struct device_queue_manager *dqm,
>   				struct queue *q)
>   {
>   	if (q->properties.type == KFD_QUEUE_TYPE_SDMA) {
> @@ -962,6 +970,14 @@ static void deallocate_sdma_queue(struct device_queue_manager *dqm,
>   	}
>   }
>   
> +static void deallocate_sdma_queue(struct device_queue_manager *dqm,
> +				struct queue *q)
> +{
> +	dqm_lock(dqm);
> +	deallocate_sdma_queue_locked(dqm, q);
> +	dqm_unlock(dqm);
> +}
> +
>   /*
>    * Device Queue Manager implementation for cp scheduler
>    */
> @@ -1356,10 +1372,10 @@ static int destroy_queue_cpsch(struct device_queue_manager *dqm,
>   
>   	if (q->properties.type == KFD_QUEUE_TYPE_SDMA) {
>   		dqm->sdma_queue_count--;
> -		deallocate_sdma_queue(dqm, q);
> +		deallocate_sdma_queue_locked(dqm, q);
>   	} else if (q->properties.type == KFD_QUEUE_TYPE_SDMA_XGMI) {
>   		dqm->xgmi_sdma_queue_count--;
> -		deallocate_sdma_queue(dqm, q);
> +		deallocate_sdma_queue_locked(dqm, q);
>   	}
>   
>   	list_del(&q->list);
> @@ -1585,10 +1601,10 @@ static int process_termination_cpsch(struct device_queue_manager *dqm,
>   	list_for_each_entry(q, &qpd->queues_list, list) {
>   		if (q->properties.type == KFD_QUEUE_TYPE_SDMA) {
>   			dqm->sdma_queue_count--;
> -			deallocate_sdma_queue(dqm, q);
> +			deallocate_sdma_queue_locked(dqm, q);
>   		} else if (q->properties.type == KFD_QUEUE_TYPE_SDMA_XGMI) {
>   			dqm->xgmi_sdma_queue_count--;
> -			deallocate_sdma_queue(dqm, q);
> +			deallocate_sdma_queue_locked(dqm, q);
>   		}
>   
>   		if (q->properties.is_active)
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH 1/6] drm/amdkfd: Only initialize sdma vm for sdma queues

[Zeng, Oak]

Hi Felix,

I rebased this series to the latest drm-next code base.

Regards,
Oak

-----Original Message-----
From: Zeng, Oak <Oak.Zeng@amd.com> 
Sent: Wednesday, June 5, 2019 12:07 PM
To: amd-gfx@lists.freedesktop.org
Cc: Kuehling, Felix <Felix.Kuehling@amd.com>; Zhao, Yong <Yong.Zhao@amd.com>; Liu, Alex <Alex.Liu@amd.com>; Freehill, Chris <Chris.Freehill@amd.com>; Zeng, Oak <Oak.Zeng@amd.com>
Subject: [PATCH 1/6] drm/amdkfd: Only initialize sdma vm for sdma queues

Don't do the same for compute queues

Change-Id: Id5f743ca10c2b761590bfe18cab2f802d3c04d2d
Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index c18355d..00ca3dd 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -1209,8 +1209,9 @@ static int create_queue_cpsch(struct device_queue_manager *dqm, struct queue *q,
 	 * updates the is_evicted flag but is a no-op otherwise.
 	 */
 	q->properties.is_evicted = !!qpd->evicted;
-
-	dqm->asic_ops.init_sdma_vm(dqm, q, qpd);
+	if (q->properties.type == KFD_QUEUE_TYPE_SDMA ||
+		q->properties.type == KFD_QUEUE_TYPE_SDMA_XGMI)
+		dqm->asic_ops.init_sdma_vm(dqm, q, qpd);
 	q->properties.tba_addr = qpd->tba_addr;
 	q->properties.tma_addr = qpd->tma_addr;
 	retval = mqd_mgr->init_mqd(mqd_mgr, &q->mqd, &q->mqd_mem_obj,
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 1/6] drm/amdkfd: Only initialize sdma vm for sdma queues

[Zeng, Oak]

Don't do the same for compute queues

Change-Id: Id5f743ca10c2b761590bfe18cab2f802d3c04d2d
Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index ece35c7..e5cbf21 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -1206,7 +1206,9 @@ static int create_queue_cpsch(struct device_queue_manager *dqm, struct queue *q,
 		q->properties.is_evicted = (q->properties.queue_size > 0 &&
 					    q->properties.queue_percent > 0 &&
 					    q->properties.queue_address != 0);
-	dqm->asic_ops.init_sdma_vm(dqm, q, qpd);
+	if (q->properties.type == KFD_QUEUE_TYPE_SDMA ||
+		q->properties.type == KFD_QUEUE_TYPE_SDMA_XGMI)
+		dqm->asic_ops.init_sdma_vm(dqm, q, qpd);
 	q->properties.tba_addr = qpd->tba_addr;
 	q->properties.tma_addr = qpd->tma_addr;
 	retval = mqd_mgr->init_mqd(mqd_mgr, &q->mqd, &q->mqd_mem_obj,
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 2/6] drm/amdkfd: Only load sdma mqd when queue is active

[Zeng, Oak]

Also calls load_mqd with current->mm struct. The mm
struct is used to read back user wptr of the queue.

Change-Id: I0f6d085878358dcd3a413054dbe61d1ca0fdf66d
Signed-off-by: Oak Zeng <Oak.Zeng@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index e5cbf21..dc1a70b 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -990,8 +990,11 @@ static int create_sdma_queue_nocpsch(struct device_queue_manager *dqm,
 	if (retval)
 		goto out_deallocate_doorbell;
 
+	if (!q->properties.is_active)
+		return 0;
+
 	retval = mqd_mgr->load_mqd(mqd_mgr, q->mqd, 0, 0, &q->properties,
-				NULL);
+				current->mm);
 	if (retval)
 		goto out_uninit_mqd;
 
-- 
2.7.4

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx