On 10/30/2025 11:05 PM, Kuehling, Felix wrote:

> On 2025-10-29 23:45, Zhu Lingshan wrote:
>> The pasid is a per-process-per-device attribute,
>> therefore this commit implements per
>> struct kfd_process_device->pasid in sysfs
>
> Does anyone in user mode actually need this PASID? When we changed the
> PASID allocation to be per-process-device, we changed a bunch of our
> dmesg logging (and I think debugfs files, too) to report PIDs instead
> of PASIDs. So there should be no good reason to know PASIDs in user mode. 

Hello Felix,

This patch is to fix current buggy pasid in sysfs which is hard-coded 0,
if we don't need to expose pasid to user space, I think we should remove it 
from sysfs.

Thanks
Lingshan

>
> Regards,
>   Felix
>
>
>>
>> Signed-off-by: Zhu Lingshan <[email protected]>
>> ---
>>   drivers/gpu/drm/amd/amdkfd/kfd_priv.h    |  9 ++-------
>>   drivers/gpu/drm/amd/amdkfd/kfd_process.c | 18 +++++++++++-------
>>   2 files changed, 13 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>> b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>> index 70ef051511bb..6a3cfeccacd8 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>> @@ -864,6 +864,8 @@ struct kfd_process_device {
>>       bool has_reset_queue;
>>         u32 pasid;
>> +    char pasid_filename[MAX_SYSFS_FILENAME_LEN];
>> +    struct attribute attr_pasid;
>>   };
>>     #define qpd_to_pdd(x) container_of(x, struct kfd_process_device,
>> qpd)
>> @@ -983,7 +985,6 @@ struct kfd_process {
>>       /* Kobj for our procfs */
>>       struct kobject *kobj;
>>       struct kobject *kobj_queues;
>> -    struct attribute attr_pasid;
>>         /* Keep track cwsr init */
>>       bool has_cwsr;
>> @@ -1100,12 +1101,6 @@ void
>> kfd_process_device_remove_obj_handle(struct kfd_process_device *pdd,
>>                       int handle);
>>   struct kfd_process *kfd_lookup_process_by_pid(struct pid *pid);
>>   -/* PASIDs */
>> -int kfd_pasid_init(void);
>> -void kfd_pasid_exit(void);
>> -u32 kfd_pasid_alloc(void);
>> -void kfd_pasid_free(u32 pasid);
>> -
>>   /* Doorbells */
>>   size_t kfd_doorbell_process_slice(struct kfd_dev *kfd);
>>   int kfd_doorbell_init(struct kfd_dev *kfd);
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
>> b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
>> index ddfe30c13e9d..24cf3b250b37 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
>> @@ -328,9 +328,11 @@ static int kfd_get_cu_occupancy(struct attribute
>> *attr, char *buffer)
>>   static ssize_t kfd_procfs_show(struct kobject *kobj, struct
>> attribute *attr,
>>                      char *buffer)
>>   {
>> -    if (strcmp(attr->name, "pasid") == 0)
>> -        return snprintf(buffer, PAGE_SIZE, "%d\n", 0);
>> -    else if (strncmp(attr->name, "vram_", 5) == 0) {
>> +    if (strncmp(attr->name, "pasid_", 6) == 0) {
>> +        struct kfd_process_device *pdd = container_of(attr, struct
>> kfd_process_device,
>> +                                  attr_pasid);
>> +        return snprintf(buffer, PAGE_SIZE, "%u\n", pdd->pasid);
>> +    } else if (strncmp(attr->name, "vram_", 5) == 0) {
>>           struct kfd_process_device *pdd = container_of(attr, struct
>> kfd_process_device,
>>                                     attr_vram);
>>           return snprintf(buffer, PAGE_SIZE, "%llu\n",
>> atomic64_read(&pdd->vram_usage));
>> @@ -662,6 +664,7 @@ static void kfd_procfs_add_sysfs_files(struct
>> kfd_process *p)
>>        * Create sysfs files for each GPU:
>>        * - proc/<pid>/vram_<gpuid>
>>        * - proc/<pid>/sdma_<gpuid>
>> +     * - proc/<pid>/pasid_<gpuid>
>>        */
>>       for (i = 0; i < p->n_pdds; i++) {
>>           struct kfd_process_device *pdd = p->pdds[i];
>> @@ -675,6 +678,10 @@ static void kfd_procfs_add_sysfs_files(struct
>> kfd_process *p)
>>                pdd->dev->id);
>>           kfd_sysfs_create_file(p->kobj, &pdd->attr_sdma,
>>                           pdd->sdma_filename);
>> +
>> +        snprintf(pdd->pasid_filename, MAX_SYSFS_FILENAME_LEN,
>> "pasid_%u",
>> +             pdd->dev->id);
>> +        kfd_sysfs_create_file(p->kobj, &pdd->attr_pasid,
>> pdd->pasid_filename);
>>       }
>>   }
>>   @@ -888,9 +895,6 @@ struct kfd_process *kfd_create_process(struct
>> task_struct *thread)
>>               goto out;
>>           }
>>   -        kfd_sysfs_create_file(process->kobj, &process->attr_pasid,
>> -                      "pasid");
>> -
>>           process->kobj_queues = kobject_create_and_add("queues",
>>                               process->kobj);
>>           if (!process->kobj_queues)
>> @@ -1104,7 +1108,6 @@ static void kfd_process_remove_sysfs(struct
>> kfd_process *p)
>>       if (!p->kobj)
>>           return;
>>   -    sysfs_remove_file(p->kobj, &p->attr_pasid);
>>       kobject_del(p->kobj_queues);
>>       kobject_put(p->kobj_queues);
>>       p->kobj_queues = NULL;
>> @@ -1114,6 +1117,7 @@ static void kfd_process_remove_sysfs(struct
>> kfd_process *p)
>>             sysfs_remove_file(p->kobj, &pdd->attr_vram);
>>           sysfs_remove_file(p->kobj, &pdd->attr_sdma);
>> +        sysfs_remove_file(p->kobj, &pdd->attr_pasid);
>>             sysfs_remove_file(pdd->kobj_stats, &pdd->attr_evict);
>>           if (pdd->dev->kfd2kgd->get_cu_occupancy)

Reply via email to