[PATCH 2/2] drm/amdkfd: Walk thorugh list with dqm lock hold

[Felix Kuehling]

On 2021-06-16 4:35 a.m., xinhui pan wrote:
> To avoid any list corruption.
>
> Signed-off-by: xinhui pan <xinhui.pan at amd.com>
> ---
>   .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c    | 12 ++++++++----
>   1 file changed, 8 insertions(+), 4 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 c24ab8f17eb6..1f84de861ec6 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -1704,7 +1704,7 @@ static int process_termination_cpsch(struct device_queue_manager *dqm,
>   		struct qcm_process_device *qpd)
>   {
>   	int retval;
> -	struct queue *q, *next;
> +	struct queue *q;
>   	struct kernel_queue *kq, *kq_next;
>   	struct mqd_manager *mqd_mgr;
>   	struct device_process_node *cur, *next_dpn;
> @@ -1739,8 +1739,6 @@ static int process_termination_cpsch(struct device_queue_manager *dqm,
>   				qpd->mapped_gws_queue = false;
>   			}
>   		}
> -
> -		dqm->total_queue_count--;

I think this should stay here. This is only used to check the maximum 
user queue limit per-device, which is a HW limitation. As far as the HW 
is concerned, the queues are destroyed after the call to 
execute_queues_cpsch. So there is no need to delay this update.


>   	}
>   
>   	/* Unregister process */
> @@ -1772,13 +1770,19 @@ static int process_termination_cpsch(struct device_queue_manager *dqm,
>   	/* Lastly, free mqd resources.
>   	 * Do free_mqd() after dqm_unlock to avoid circular locking.
>   	 */
> -	list_for_each_entry_safe(q, next, &qpd->queues_list, list) {
> +	dqm_lock(dqm);

Instead of taking the dqm lock again, just move this up a couple of 
lines before the dqm_unlock call.

Regards,
   Felix


> +	while (!list_empty(&qpd->queues_list)) {
> +		q = list_first_entry(&qpd->queues_list, struct queue, list);
>   		mqd_mgr = dqm->mqd_mgrs[get_mqd_type_from_queue_type(
>   				q->properties.type)];
>   		list_del(&q->list);
>   		qpd->queue_count--;
> +		dqm->total_queue_count--;
> +		dqm_unlock(dqm);
>   		mqd_mgr->free_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
> +		dqm_lock(dqm);
>   	}
> +	dqm_unlock(dqm);
>   
>   	return retval;
>   }