Re: [RFC PATCH 03/13] drm/amdgpu: add a flag to track ras debugfs creation status

[Mario Limonciello <mario.limonciello@amd.com>]

On 1/9/2025 21:19, Gerry Liu wrote:
> 
> 
>> 2025年1月9日 01:19，Mario Limonciello <mario.limonciello@amd.com 
>> <mailto:mario.limonciello@amd.com>> 写道：
>>
>> On 1/8/2025 07:59, Jiang Liu wrote:
>>> Add a flag to track ras debugfs creation status, to avoid possible
>>> incorrect reference count management for ras block object  in function
>>> amdgpu_ras_aca_is_supported().
>>
>> Rather than taking a marker position, why not just check for
>> obj->fs_data.debugfs_name to be non NULL in amdgpu_ras_fs_fini()?
> I plan to use marker as a common status track mechanism, so used marker 
> here:)
> 
>>
>>> Signed-off-by: Jiang Liu <gerry@linux.alibaba.com 
>>> <mailto:gerry@linux.alibaba.com>>
>>> ---
>>>  drivers/gpu/drm/amd/amdgpu/amdgpu.h     | 2 +-
>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 9 +++++++--
>>>  2 files changed, 8 insertions(+), 3 deletions(-)
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/ 
>>> amd/amdgpu/amdgpu.h
>>> index 32941f29507c..2ef7d3102be3 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> @@ -378,7 +378,7 @@ enum amdgpu_marker {
>>> AMDGPU_MARKER_IRQ6= 6,
>>> AMDGPU_MARKER_IRQ7= 7,
>>> AMDGPU_MARKER_IRQ_MAX= 47,
>>> -AMDGPU_MARKER_DEBUGFS= 63,
>>> +AMDGPU_MARKER_RAS_DEBUGFS= 63,
>>
>> Any particular reason you jumped to 63 in this patch and then counted 
>> down in the next one?  IE why not throw it at 48 (and then 49 for next 
>> one)?
> I’m not sure how much markers are needed for IRQ, so I split the space 
> into two parts: one for irq and one for others.

I think it's up to Alex and Christian here but as it's all internal to 
amdgpu we can always reassign the enum in the future.

So I think we should just take 48 and 49.  If another IRQ is needed we 
can slot it in at 48 and bump these by 1.

Alex, Christian, thoughts?

> 
>>
>>>  };
>>>    #define AMDGPU_MARKER_INDEX_IRQ(idx)(AMDGPU_MARKER_INDEX_IRQ0 + (idx))
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c b/drivers/gpu/ 
>>> drm/amd/amdgpu/amdgpu_ras.c
>>> index 6d52e22691f7..efd72b07a185 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
>>> @@ -1996,7 +1996,8 @@ static void amdgpu_ras_debugfs_create(struct 
>>> amdgpu_device *adev,
>>>  {
>>> struct ras_manager *obj = amdgpu_ras_find_obj(adev, &head->head);
>>>  -if (!obj || !dir)
>>> +if (!obj || !dir ||
>>> +   amdgpu_ras_test_marker(adev, &head->head, AMDGPU_MARKER_RAS_DEBUGFS))
>>> return;
>>> get_obj(obj);
>>> @@ -2007,6 +2008,8 @@ static void amdgpu_ras_debugfs_create(struct 
>>> amdgpu_device *adev,
>>> debugfs_create_file(obj->fs_data.debugfs_name, S_IWUGO | S_IRUGO, dir,
>>>    obj, &amdgpu_ras_debugfs_ops);
>>> +
>>> +amdgpu_ras_set_marker(adev, &head->head, AMDGPU_MARKER_RAS_DEBUGFS);
>>>  }
>>>    static bool amdgpu_ras_aca_is_supported(struct amdgpu_device *adev)
>>> @@ -2134,7 +2137,9 @@ static int amdgpu_ras_fs_fini(struct 
>>> amdgpu_device *adev)
>>> if (IS_ENABLED(CONFIG_DEBUG_FS)) {
>>> list_for_each_entry_safe(con_obj, tmp, &con->head, node) {
>>> ip_obj = amdgpu_ras_find_obj(adev, &con_obj->head);
>>> -if (ip_obj)
>>> +if (ip_obj &&
>>> +   amdgpu_ras_test_and_clear_marker(adev, &ip_obj->head,
>>> +    AMDGPU_MARKER_RAS_DEBUGFS))
>>> put_obj(ip_obj);
>>> }
>>> }
>