On Fri, Jan 10, 2025 at 12:10 PM Mario Limonciello <mario.limonciello@xxxxxxx> wrote: > > On 1/9/2025 21:19, Gerry Liu wrote: > > > > > >> 2025年1月9日 01:19，Mario Limonciello <mario.limonciello@xxxxxxx > >> <mailto:mario.limonciello@xxxxxxx>> 写道： > >> > >> 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@xxxxxxxxxxxxxxxxx > >>> <mailto:gerry@xxxxxxxxxxxxxxxxx>> > >>> --- > >>> 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. Why explicitly assign numbers to these? if we just use them as an enum we can add enums in the future wherever we want in the sequence. Alex > > 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); > >>> } > >>> } > > >
