[PATCH v2 1/2] drm/amdkfd: Fix GWS queue count
Paul Menzel
pmenzel at molgen.mpg.de
Mon Apr 18 20:18:32 UTC 2022
Dear David,
Thank you for your patch.
Am 18.04.22 um 18:44 schrieb David Yat Sin:
> dqm->gws_queue_count and pdd->qpd.mapped_gws_queue needs to be updated
s/needs/need/
> each time the queue gets evicted.
Why?
Do you only change the case, when an element of the queue gets evicted?
Next time, a short note about the implementation would be nic.e
No Fixes tag?
Kind regards,
Paul
> Signed-off-by: David Yat Sin <david.yatsin at amd.com>
> ---
> .../drm/amd/amdkfd/kfd_device_queue_manager.c | 83 +++++++++----------
> 1 file changed, 37 insertions(+), 46 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 acf4f7975850..198672264492 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -130,19 +130,33 @@ void program_sh_mem_settings(struct device_queue_manager *dqm,
> }
>
> static void increment_queue_count(struct device_queue_manager *dqm,
> - enum kfd_queue_type type)
> + struct qcm_process_device *qpd,
> + struct queue *q)
> {
> dqm->active_queue_count++;
> - if (type == KFD_QUEUE_TYPE_COMPUTE || type == KFD_QUEUE_TYPE_DIQ)
> + if (q->properties.type == KFD_QUEUE_TYPE_COMPUTE ||
> + q->properties.type == KFD_QUEUE_TYPE_DIQ)
> dqm->active_cp_queue_count++;
> +
> + if (q->properties.is_gws) {
> + dqm->gws_queue_count++;
> + qpd->mapped_gws_queue = true;
> + }
> }
>
> static void decrement_queue_count(struct device_queue_manager *dqm,
> - enum kfd_queue_type type)
> + struct qcm_process_device *qpd,
> + struct queue *q)
> {
> dqm->active_queue_count--;
> - if (type == KFD_QUEUE_TYPE_COMPUTE || type == KFD_QUEUE_TYPE_DIQ)
> + if (q->properties.type == KFD_QUEUE_TYPE_COMPUTE ||
> + q->properties.type == KFD_QUEUE_TYPE_DIQ)
> dqm->active_cp_queue_count--;
> +
> + if (q->properties.is_gws) {
> + dqm->gws_queue_count--;
> + qpd->mapped_gws_queue = false;
> + }
> }
>
> /*
> @@ -412,7 +426,7 @@ static int create_queue_nocpsch(struct device_queue_manager *dqm,
> list_add(&q->list, &qpd->queues_list);
> qpd->queue_count++;
> if (q->properties.is_active)
> - increment_queue_count(dqm, q->properties.type);
> + increment_queue_count(dqm, qpd, q);
>
> /*
> * Unconditionally increment this counter, regardless of the queue's
> @@ -601,13 +615,8 @@ static int destroy_queue_nocpsch_locked(struct device_queue_manager *dqm,
> deallocate_vmid(dqm, qpd, q);
> }
> qpd->queue_count--;
> - if (q->properties.is_active) {
> - decrement_queue_count(dqm, q->properties.type);
> - if (q->properties.is_gws) {
> - dqm->gws_queue_count--;
> - qpd->mapped_gws_queue = false;
> - }
> - }
> + if (q->properties.is_active)
> + decrement_queue_count(dqm, qpd, q);
>
> return retval;
> }
> @@ -700,12 +709,11 @@ static int update_queue(struct device_queue_manager *dqm, struct queue *q,
> * dqm->active_queue_count to determine whether a new runlist must be
> * uploaded.
> */
> - if (q->properties.is_active && !prev_active)
> - increment_queue_count(dqm, q->properties.type);
> - else if (!q->properties.is_active && prev_active)
> - decrement_queue_count(dqm, q->properties.type);
> -
> - if (q->gws && !q->properties.is_gws) {
> + if (q->properties.is_active && !prev_active) {
> + increment_queue_count(dqm, &pdd->qpd, q);
> + } else if (!q->properties.is_active && prev_active) {
> + decrement_queue_count(dqm, &pdd->qpd, q);
> + } else if (q->gws && !q->properties.is_gws) {
> if (q->properties.is_active) {
> dqm->gws_queue_count++;
> pdd->qpd.mapped_gws_queue = true;
> @@ -767,11 +775,7 @@ static int evict_process_queues_nocpsch(struct device_queue_manager *dqm,
> mqd_mgr = dqm->mqd_mgrs[get_mqd_type_from_queue_type(
> q->properties.type)];
> q->properties.is_active = false;
> - decrement_queue_count(dqm, q->properties.type);
> - if (q->properties.is_gws) {
> - dqm->gws_queue_count--;
> - qpd->mapped_gws_queue = false;
> - }
> + decrement_queue_count(dqm, qpd, q);
>
> if (WARN_ONCE(!dqm->sched_running, "Evict when stopped\n"))
> continue;
> @@ -817,7 +821,7 @@ static int evict_process_queues_cpsch(struct device_queue_manager *dqm,
> continue;
>
> q->properties.is_active = false;
> - decrement_queue_count(dqm, q->properties.type);
> + decrement_queue_count(dqm, qpd, q);
> }
> pdd->last_evict_timestamp = get_jiffies_64();
> retval = execute_queues_cpsch(dqm,
> @@ -888,11 +892,7 @@ static int restore_process_queues_nocpsch(struct device_queue_manager *dqm,
> mqd_mgr = dqm->mqd_mgrs[get_mqd_type_from_queue_type(
> q->properties.type)];
> q->properties.is_active = true;
> - increment_queue_count(dqm, q->properties.type);
> - if (q->properties.is_gws) {
> - dqm->gws_queue_count++;
> - qpd->mapped_gws_queue = true;
> - }
> + increment_queue_count(dqm, qpd, q);
>
> if (WARN_ONCE(!dqm->sched_running, "Restore when stopped\n"))
> continue;
> @@ -950,7 +950,7 @@ static int restore_process_queues_cpsch(struct device_queue_manager *dqm,
> continue;
>
> q->properties.is_active = true;
> - increment_queue_count(dqm, q->properties.type);
> + increment_queue_count(dqm, &pdd->qpd, q);
> }
> retval = execute_queues_cpsch(dqm,
> KFD_UNMAP_QUEUES_FILTER_DYNAMIC_QUEUES, 0);
> @@ -1378,7 +1378,7 @@ static int create_kernel_queue_cpsch(struct device_queue_manager *dqm,
> dqm->total_queue_count);
>
> list_add(&kq->list, &qpd->priv_queue_list);
> - increment_queue_count(dqm, kq->queue->properties.type);
> + increment_queue_count(dqm, qpd, kq->queue);
> qpd->is_debug = true;
> execute_queues_cpsch(dqm, KFD_UNMAP_QUEUES_FILTER_DYNAMIC_QUEUES, 0);
> dqm_unlock(dqm);
> @@ -1392,7 +1392,7 @@ static void destroy_kernel_queue_cpsch(struct device_queue_manager *dqm,
> {
> dqm_lock(dqm);
> list_del(&kq->list);
> - decrement_queue_count(dqm, kq->queue->properties.type);
> + decrement_queue_count(dqm, qpd, kq->queue);
> qpd->is_debug = false;
> execute_queues_cpsch(dqm, KFD_UNMAP_QUEUES_FILTER_ALL_QUEUES, 0);
> /*
> @@ -1467,7 +1467,7 @@ static int create_queue_cpsch(struct device_queue_manager *dqm, struct queue *q,
> qpd->queue_count++;
>
> if (q->properties.is_active) {
> - increment_queue_count(dqm, q->properties.type);
> + increment_queue_count(dqm, qpd, q);
>
> execute_queues_cpsch(dqm,
> KFD_UNMAP_QUEUES_FILTER_DYNAMIC_QUEUES, 0);
> @@ -1683,15 +1683,11 @@ static int destroy_queue_cpsch(struct device_queue_manager *dqm,
> list_del(&q->list);
> qpd->queue_count--;
> if (q->properties.is_active) {
> - decrement_queue_count(dqm, q->properties.type);
> + decrement_queue_count(dqm, qpd, q);
> retval = execute_queues_cpsch(dqm,
> KFD_UNMAP_QUEUES_FILTER_DYNAMIC_QUEUES, 0);
> if (retval == -ETIME)
> qpd->reset_wavefronts = true;
> - if (q->properties.is_gws) {
> - dqm->gws_queue_count--;
> - qpd->mapped_gws_queue = false;
> - }
> }
>
> /*
> @@ -1932,7 +1928,7 @@ static int process_termination_cpsch(struct device_queue_manager *dqm,
> /* Clean all kernel queues */
> list_for_each_entry_safe(kq, kq_next, &qpd->priv_queue_list, list) {
> list_del(&kq->list);
> - decrement_queue_count(dqm, kq->queue->properties.type);
> + decrement_queue_count(dqm, qpd, kq->queue);
> qpd->is_debug = false;
> dqm->total_queue_count--;
> filter = KFD_UNMAP_QUEUES_FILTER_ALL_QUEUES;
> @@ -1945,13 +1941,8 @@ static int process_termination_cpsch(struct device_queue_manager *dqm,
> else if (q->properties.type == KFD_QUEUE_TYPE_SDMA_XGMI)
> deallocate_sdma_queue(dqm, q);
>
> - if (q->properties.is_active) {
> - decrement_queue_count(dqm, q->properties.type);
> - if (q->properties.is_gws) {
> - dqm->gws_queue_count--;
> - qpd->mapped_gws_queue = false;
> - }
> - }
> + if (q->properties.is_active)
> + decrement_queue_count(dqm, qpd, q);
>
> dqm->total_queue_count--;
> }
More information about the amd-gfx
mailing list