[PATCH 1/2] drm/amdkfd: Add an optional argument into update queue operation

Felix Kuehling felix.kuehling at amd.com
Thu Oct 21 16:45:44 UTC 2021


Am 2021-10-15 um 4:48 a.m. schrieb Lang Yu:
> Currently, queue is updated with data stored in queue_properties.
> And all allocated resource in queue_properties will not be freed
> until the queue is destroyed.
>
> But some properties(e.g., cu mask) bring some memory management
> headaches(e.g., memory leak) and make code complex. Actually they
> don't have to persist in queue_properties.
>
> So add an argument into update queue to pass such properties and
> remove them from queue_properties.
>
> Signed-off-by: Lang Yu <lang.yu at amd.com>
> ---
>  .../drm/amd/amdkfd/kfd_device_queue_manager.c |  4 ++--
>  .../drm/amd/amdkfd/kfd_device_queue_manager.h |  2 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.h  |  2 +-
>  .../gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c  | 18 +++++++--------
>  .../gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c  |  8 +++----
>  .../gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c   |  8 +++----
>  .../gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c   | 22 +++++++++----------
>  .../amd/amdkfd/kfd_process_queue_manager.c    |  6 ++---
>  8 files changed, 35 insertions(+), 35 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 f8fce9d05f50..7f6f4937eedb 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -557,7 +557,7 @@ static int destroy_queue_nocpsch(struct device_queue_manager *dqm,
>  	return retval;
>  }
>  
> -static int update_queue(struct device_queue_manager *dqm, struct queue *q)
> +static int update_queue(struct device_queue_manager *dqm, struct queue *q, void *args)

Please don't use a void * here. If you don't want to declare the struct
queue_update_info in this patch, you can just declare it as an abstract
type:

    struct queue_update_info;

You can cast NULL to (struct queue_update_info *) without requiring the
structure definition.

Regards,
  Felix


>  {
>  	int retval = 0;
>  	struct mqd_manager *mqd_mgr;
> @@ -605,7 +605,7 @@ static int update_queue(struct device_queue_manager *dqm, struct queue *q)
>  		}
>  	}
>  
> -	mqd_mgr->update_mqd(mqd_mgr, q->mqd, &q->properties);
> +	mqd_mgr->update_mqd(mqd_mgr, q->mqd, &q->properties, args);
>  
>  	/*
>  	 * check active state vs. the previous state and modify
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
> index c8719682c4da..08cfc2a2fdbb 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
> @@ -93,7 +93,7 @@ struct device_queue_manager_ops {
>  				struct queue *q);
>  
>  	int	(*update_queue)(struct device_queue_manager *dqm,
> -				struct queue *q);
> +				struct queue *q, void *args);
>  
>  	int	(*register_process)(struct device_queue_manager *dqm,
>  					struct qcm_process_device *qpd);
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.h b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.h
> index 6e6918ccedfd..6ddf93629b8c 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.h
> @@ -80,7 +80,7 @@ struct mqd_manager {
>  				struct mm_struct *mms);
>  
>  	void	(*update_mqd)(struct mqd_manager *mm, void *mqd,
> -				struct queue_properties *q);
> +				struct queue_properties *q, void *args);
>  
>  	int	(*destroy_mqd)(struct mqd_manager *mm, void *mqd,
>  				enum kfd_preempt_type type,
> 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 064914e1e8d6..8bb2fd4cba41 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c
> @@ -135,7 +135,7 @@ static void init_mqd(struct mqd_manager *mm, void **mqd,
>  	*mqd = m;
>  	if (gart_addr)
>  		*gart_addr = addr;
> -	mm->update_mqd(mm, m, q);
> +	mm->update_mqd(mm, m, q, NULL);
>  }
>  
>  static void init_mqd_sdma(struct mqd_manager *mm, void **mqd,
> @@ -152,7 +152,7 @@ static void init_mqd_sdma(struct mqd_manager *mm, void **mqd,
>  	if (gart_addr)
>  		*gart_addr = mqd_mem_obj->gpu_addr;
>  
> -	mm->update_mqd(mm, m, q);
> +	mm->update_mqd(mm, m, q, NULL);
>  }
>  
>  static void free_mqd(struct mqd_manager *mm, void *mqd,
> @@ -185,7 +185,7 @@ static int load_mqd_sdma(struct mqd_manager *mm, void *mqd,
>  }
>  
>  static void __update_mqd(struct mqd_manager *mm, void *mqd,
> -			struct queue_properties *q, unsigned int atc_bit)
> +			struct queue_properties *q, void *args, unsigned int atc_bit)
>  {
>  	struct cik_mqd *m;
>  
> @@ -221,9 +221,9 @@ static void __update_mqd(struct mqd_manager *mm, void *mqd,
>  }
>  
>  static void update_mqd(struct mqd_manager *mm, void *mqd,
> -			struct queue_properties *q)
> +			struct queue_properties *q, void *args)
>  {
> -	__update_mqd(mm, mqd, q, 1);
> +	__update_mqd(mm, mqd, q, args, 1);
>  }
>  
>  static uint32_t read_doorbell_id(void *mqd)
> @@ -234,13 +234,13 @@ static uint32_t read_doorbell_id(void *mqd)
>  }
>  
>  static void update_mqd_hawaii(struct mqd_manager *mm, void *mqd,
> -			struct queue_properties *q)
> +			struct queue_properties *q, void *args)
>  {
> -	__update_mqd(mm, mqd, q, 0);
> +	__update_mqd(mm, mqd, q, args, 0);
>  }
>  
>  static void update_mqd_sdma(struct mqd_manager *mm, void *mqd,
> -				struct queue_properties *q)
> +				struct queue_properties *q, void *args)
>  {
>  	struct cik_sdma_rlc_registers *m;
>  
> @@ -318,7 +318,7 @@ static void init_mqd_hiq(struct mqd_manager *mm, void **mqd,
>  }
>  
>  static void update_mqd_hiq(struct mqd_manager *mm, void *mqd,
> -				struct queue_properties *q)
> +				struct queue_properties *q, void *args)
>  {
>  	struct cik_mqd *m;
>  
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c
> index c7fb59ca597f..6d468b6c8d7d 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c
> @@ -136,7 +136,7 @@ static void init_mqd(struct mqd_manager *mm, void **mqd,
>  	*mqd = m;
>  	if (gart_addr)
>  		*gart_addr = addr;
> -	mm->update_mqd(mm, m, q);
> +	mm->update_mqd(mm, m, q, NULL);
>  }
>  
>  static int load_mqd(struct mqd_manager *mm, void *mqd,
> @@ -162,7 +162,7 @@ static int hiq_load_mqd_kiq(struct mqd_manager *mm, void *mqd,
>  }
>  
>  static void update_mqd(struct mqd_manager *mm, void *mqd,
> -		      struct queue_properties *q)
> +		      struct queue_properties *q, void *args)
>  {
>  	struct v10_compute_mqd *m;
>  
> @@ -311,7 +311,7 @@ static void init_mqd_sdma(struct mqd_manager *mm, void **mqd,
>  	if (gart_addr)
>  		*gart_addr = mqd_mem_obj->gpu_addr;
>  
> -	mm->update_mqd(mm, m, q);
> +	mm->update_mqd(mm, m, q, NULL);
>  }
>  
>  static int load_mqd_sdma(struct mqd_manager *mm, void *mqd,
> @@ -326,7 +326,7 @@ static int load_mqd_sdma(struct mqd_manager *mm, void *mqd,
>  #define SDMA_RLC_DUMMY_DEFAULT 0xf
>  
>  static void update_mqd_sdma(struct mqd_manager *mm, void *mqd,
> -		struct queue_properties *q)
> +		struct queue_properties *q, void *args)
>  {
>  	struct v10_sdma_mqd *m;
>  
> 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 7f4e102ff4bd..f6c10b1b5f8b 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
> @@ -188,7 +188,7 @@ static void init_mqd(struct mqd_manager *mm, void **mqd,
>  	*mqd = m;
>  	if (gart_addr)
>  		*gart_addr = addr;
> -	mm->update_mqd(mm, m, q);
> +	mm->update_mqd(mm, m, q, NULL);
>  }
>  
>  static int load_mqd(struct mqd_manager *mm, void *mqd,
> @@ -212,7 +212,7 @@ static int hiq_load_mqd_kiq(struct mqd_manager *mm, void *mqd,
>  }
>  
>  static void update_mqd(struct mqd_manager *mm, void *mqd,
> -		      struct queue_properties *q)
> +		      struct queue_properties *q, void *args)
>  {
>  	struct v9_mqd *m;
>  
> @@ -366,7 +366,7 @@ static void init_mqd_sdma(struct mqd_manager *mm, void **mqd,
>  	if (gart_addr)
>  		*gart_addr = mqd_mem_obj->gpu_addr;
>  
> -	mm->update_mqd(mm, m, q);
> +	mm->update_mqd(mm, m, q, NULL);
>  }
>  
>  static int load_mqd_sdma(struct mqd_manager *mm, void *mqd,
> @@ -381,7 +381,7 @@ static int load_mqd_sdma(struct mqd_manager *mm, void *mqd,
>  #define SDMA_RLC_DUMMY_DEFAULT 0xf
>  
>  static void update_mqd_sdma(struct mqd_manager *mm, void *mqd,
> -		struct queue_properties *q)
> +		struct queue_properties *q, void *args)
>  {
>  	struct v9_sdma_mqd *m;
>  
> 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 33dbd22d290f..bddd6d8fdf32 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c
> @@ -150,7 +150,7 @@ static void init_mqd(struct mqd_manager *mm, void **mqd,
>  	*mqd = m;
>  	if (gart_addr)
>  		*gart_addr = addr;
> -	mm->update_mqd(mm, m, q);
> +	mm->update_mqd(mm, m, q, NULL);
>  }
>  
>  static int load_mqd(struct mqd_manager *mm, void *mqd,
> @@ -167,8 +167,8 @@ static int load_mqd(struct mqd_manager *mm, void *mqd,
>  }
>  
>  static void __update_mqd(struct mqd_manager *mm, void *mqd,
> -			struct queue_properties *q, unsigned int mtype,
> -			unsigned int atc_bit)
> +			struct queue_properties *q, void *args,
> +			unsigned int mtype, unsigned int atc_bit)
>  {
>  	struct vi_mqd *m;
>  
> @@ -238,9 +238,9 @@ static void __update_mqd(struct mqd_manager *mm, void *mqd,
>  
>  
>  static void update_mqd(struct mqd_manager *mm, void *mqd,
> -			struct queue_properties *q)
> +			struct queue_properties *q, void *args)
>  {
> -	__update_mqd(mm, mqd, q, MTYPE_CC, 1);
> +	__update_mqd(mm, mqd, q, args, MTYPE_CC, 1);
>  }
>  
>  static uint32_t read_doorbell_id(void *mqd)
> @@ -251,9 +251,9 @@ static uint32_t read_doorbell_id(void *mqd)
>  }
>  
>  static void update_mqd_tonga(struct mqd_manager *mm, void *mqd,
> -			struct queue_properties *q)
> +			struct queue_properties *q, void *args)
>  {
> -	__update_mqd(mm, mqd, q, MTYPE_UC, 0);
> +	__update_mqd(mm, mqd, q, args, MTYPE_UC, 0);
>  }
>  
>  static int destroy_mqd(struct mqd_manager *mm, void *mqd,
> @@ -317,9 +317,9 @@ static void init_mqd_hiq(struct mqd_manager *mm, void **mqd,
>  }
>  
>  static void update_mqd_hiq(struct mqd_manager *mm, void *mqd,
> -			struct queue_properties *q)
> +			struct queue_properties *q, void *args)
>  {
> -	__update_mqd(mm, mqd, q, MTYPE_UC, 0);
> +	__update_mqd(mm, mqd, q, args, MTYPE_UC, 0);
>  }
>  
>  static void init_mqd_sdma(struct mqd_manager *mm, void **mqd,
> @@ -336,7 +336,7 @@ static void init_mqd_sdma(struct mqd_manager *mm, void **mqd,
>  	if (gart_addr)
>  		*gart_addr = mqd_mem_obj->gpu_addr;
>  
> -	mm->update_mqd(mm, m, q);
> +	mm->update_mqd(mm, m, q, NULL);
>  }
>  
>  static int load_mqd_sdma(struct mqd_manager *mm, void *mqd,
> @@ -349,7 +349,7 @@ static int load_mqd_sdma(struct mqd_manager *mm, void *mqd,
>  }
>  
>  static void update_mqd_sdma(struct mqd_manager *mm, void *mqd,
> -		struct queue_properties *q)
> +		struct queue_properties *q, void *args)
>  {
>  	struct vi_sdma_mqd *m;
>  
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> index 243dd1efcdbf..37529592457d 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> @@ -121,7 +121,7 @@ int pqm_set_gws(struct process_queue_manager *pqm, unsigned int qid,
>  	pdd->qpd.num_gws = gws ? amdgpu_amdkfd_get_num_gws(dev->kgd) : 0;
>  
>  	return pqn->q->device->dqm->ops.update_queue(pqn->q->device->dqm,
> -							pqn->q);
> +							pqn->q, NULL);
>  }
>  
>  void kfd_process_dequeue_from_all_devices(struct kfd_process *p)
> @@ -429,7 +429,7 @@ int pqm_update_queue(struct process_queue_manager *pqm, unsigned int qid,
>  	pqn->q->properties.priority = p->priority;
>  
>  	retval = pqn->q->device->dqm->ops.update_queue(pqn->q->device->dqm,
> -							pqn->q);
> +							pqn->q, NULL);
>  	if (retval != 0)
>  		return retval;
>  
> @@ -457,7 +457,7 @@ int pqm_set_cu_mask(struct process_queue_manager *pqm, unsigned int qid,
>  	pqn->q->properties.cu_mask = p->cu_mask;
>  
>  	retval = pqn->q->device->dqm->ops.update_queue(pqn->q->device->dqm,
> -							pqn->q);
> +							pqn->q, NULL);
>  	if (retval != 0)
>  		return retval;
>  


More information about the amd-gfx mailing list