Re: [PATCH 5/6] drm/amdgpu: Don't hardcode thread name length

["Christian König" <ckoenig.leichtzumerken@gmail.com>]

Am 25.11.20 um 18:01 schrieb Luben Tuikov:
> On 2020-11-25 04:55, Christian König wrote:
>> Am 25.11.20 um 04:17 schrieb Luben Tuikov:
>>> Introduce a macro DRM_THREAD_NAME_LEN
>>> and use that to define ring name size,
>>> instead of hardcoding it to 16.
>>>
>>> Signed-off-by: Luben Tuikov <luben.tuikov@amd.com>
>>> ---
>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 2 +-
>>>    include/drm/gpu_scheduler.h              | 2 ++
>>>    2 files changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>>> index 7112137689db..bbd46c6dec65 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
>>> @@ -230,7 +230,7 @@ struct amdgpu_ring {
>>>    	unsigned		wptr_offs;
>>>    	unsigned		fence_offs;
>>>    	uint64_t		current_ctx;
>>> -	char			name[16];
>>> +	char			name[DRM_THREAD_NAME_LEN];
>>>    	u32                     trail_seq;
>>>    	unsigned		trail_fence_offs;
>>>    	u64			trail_fence_gpu_addr;
>>> diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
>>> index 61f7121e1c19..3a5686c3b5e9 100644
>>> --- a/include/drm/gpu_scheduler.h
>>> +++ b/include/drm/gpu_scheduler.h
>>> @@ -30,6 +30,8 @@
>>>    
>>>    #define MAX_WAIT_SCHED_ENTITY_Q_EMPTY msecs_to_jiffies(1000)
>>>    
>>> +#define DRM_THREAD_NAME_LEN     TASK_COMM_LEN
>>> +
>> The thread name is an amdgpu specific thing. I don't think we should
>> have that in the scheduler.
> I need it in DRM when creating the done thread from the name
> of the main scheduler thread. Since DRM creates threads,
> the main scheduler thread and the done thread, it would
> be good to have a preliminary limit to the name string.
>
>> And why do you use TASK_COMM_LEN here? That is completely unrelated stuff.
> If you trace down into the kernel, TASK_COMM_LEN seems to be used in
> snprintf() when naming a kernel thread, and its value is 16--same
> as the one used in amdgpu.

Oh, that's new to me. In my memory this name was used as a filename in 
debugfs only.

>
> So the size of the name string transitions from amdgpu to DRM to kernel
> proper, where amdgpu and kernel proper set it to max 16, but DRM doesn't
> give it a limit.
>
> Sure, I can remove it from DRM, and just use a local limit
> when snprintf() the name when creating a tread, possibly
> using TASK_COMM_LEN. (That's in the next patch.)

Yeah, just use TASK_COMM_LEN directly where appropriate.

Regards,
Christian.

>
> Would that be better? I can do that in v2 of this patchset.
>
> Thanks,
> Luben
>
>> Regards,
>> Christian.
>>
>>>    struct drm_gpu_scheduler;
>>>    struct drm_sched_rq;
>>>    
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx