[PATCH 19/24] drm/amdkfd: enable pc sampling stop

[Yat Sin, David]

[AMD Official Use Only - General]



From: Zhu, James <James.Zhu at amd.com>
Sent: Monday, November 13, 2023 10:20 AM
To: Yat Sin, David <David.YatSin at amd.com>; Zhu, James <James.Zhu at amd.com>; amd-gfx at lists.freedesktop.org
Cc: Kuehling, Felix <Felix.Kuehling at amd.com>; Greathouse, Joseph <Joseph.Greathouse at amd.com>
Subject: Re: [PATCH 19/24] drm/amdkfd: enable pc sampling stop



On 2023-11-10 14:07, Yat Sin, David wrote:

[AMD Official Use Only - General]



-----Original Message-----

From: Zhu, James <James.Zhu at amd.com><mailto:James.Zhu at amd.com>

Sent: Friday, November 3, 2023 9:12 AM

To: amd-gfx at lists.freedesktop.org<mailto:amd-gfx at lists.freedesktop.org>

Cc: Kuehling, Felix <Felix.Kuehling at amd.com><mailto:Felix.Kuehling at amd.com>; Greathouse, Joseph

<Joseph.Greathouse at amd.com><mailto:Joseph.Greathouse at amd.com>; Yat Sin, David <David.YatSin at amd.com><mailto:David.YatSin at amd.com>; Zhu,

James <James.Zhu at amd.com><mailto:James.Zhu at amd.com>

Subject: [PATCH 19/24] drm/amdkfd: enable pc sampling stop



Enable pc sampling stop.



Signed-off-by: James Zhu <James.Zhu at amd.com><mailto:James.Zhu at amd.com>

---

 drivers/gpu/drm/amd/amdkfd/kfd_pc_sampling.c | 28 +++++++++++++++++--

-

 drivers/gpu/drm/amd/amdkfd/kfd_priv.h        |  2 ++

 2 files changed, 27 insertions(+), 3 deletions(-)



diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_pc_sampling.c

b/drivers/gpu/drm/amd/amdkfd/kfd_pc_sampling.c

index 33d003ca0093..2c4ac5b4cc4b 100644

--- a/drivers/gpu/drm/amd/amdkfd/kfd_pc_sampling.c

+++ b/drivers/gpu/drm/amd/amdkfd/kfd_pc_sampling.c

@@ -108,10 +108,32 @@ static int kfd_pc_sample_start(struct

kfd_process_device *pdd,

      return 0;

 }



-static int kfd_pc_sample_stop(struct kfd_process_device *pdd)

+static int kfd_pc_sample_stop(struct kfd_process_device *pdd,

+                                     struct pc_sampling_entry *pcs_entry)

 {

-     return -EINVAL;

+     bool pc_sampling_stop = false;

+

+     pcs_entry->enabled = false;

+     mutex_lock(&pdd->dev->pcs_data.mutex);

For the START/STOP/DESTROY ioctls, I think we can hold pdd->dev->pcs_data.mutex through the whole IOCTL. Then we would not have to deal with the intermediate states where the START/STOP/DESTROY are happening at the same time.
[JZ] pdd->dev->pcs_data.mutex is per device, not per process. In the future, also it will share protection within different pc sampling methods on the same devices. So I don't think a bigger lock here is good idea.
[David] I think the CREATE/START/STOP/DESTROY actions are not time critical. So if two processes are using the same GPU, it is ok for amdgpu to block the 2nd process until amdgpu has fully completed the request from the 1st process. I think we are making the code more complex for a use-case that would be very rare.


+     pdd->dev->pcs_data.hosttrap_entry.base.active_count--;

+     if (!pdd->dev->pcs_data.hosttrap_entry.base.active_count) {

+             WRITE_ONCE(pdd->dev-

pcs_data.hosttrap_entry.base.stop_enable, true);

+             pc_sampling_stop = true;

+     }

+     mutex_unlock(&pdd->dev->pcs_data.mutex);



+     if (pc_sampling_stop) {

+             kfd_process_set_trap_pc_sampling_flag(&pdd->qpd,

+                     pdd->dev-

pcs_data.hosttrap_entry.base.pc_sample_info.method,

+false);

+

+             mutex_lock(&pdd->dev->pcs_data.mutex);

+             pdd->dev->pcs_data.hosttrap_entry.base.target_simd = 0;

+             pdd->dev->pcs_data.hosttrap_entry.base.target_wave_slot = 0;

+             WRITE_ONCE(pdd->dev-

pcs_data.hosttrap_entry.base.stop_enable, false);

+             mutex_unlock(&pdd->dev->pcs_data.mutex);

+     }

+

+     return 0;

 }



 static int kfd_pc_sample_create(struct kfd_process_device *pdd, @@ -251,7

+273,7 @@ int kfd_pc_sample(struct kfd_process_device *pdd,

              if (!pcs_entry->enabled)

                      return -EALREADY;

              else

-                     return kfd_pc_sample_stop(pdd);

+                     return kfd_pc_sample_stop(pdd, pcs_entry);

      }



      return -EINVAL;

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h

b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h

index 613910e0d440..badaa4d68cc4 100644

--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h

+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h

@@ -259,6 +259,8 @@ struct kfd_dev;

 struct kfd_dev_pc_sampling_data {

      uint32_t use_count;         /* Num of PC sampling sessions */

      uint32_t active_count;      /* Num of active sessions */

+     uint32_t target_simd;       /* target simd for trap */

+     uint32_t target_wave_slot;  /* target wave slot for trap */

      bool stop_enable;           /* pc sampling stop in process */

      struct idr pc_sampling_idr;

      struct kfd_pc_sample_info pc_sample_info;

--

2.25.1


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20231113/4c188dd5/attachment-0001.htm>