On 04/12/2025 09:03, Lazar, Lijo wrote:
>
>
> On 12/4/2025 1:06 PM, Krzysztof Kozlowski wrote:
>> On 03/12/2025 13:54, Lijo Lazar wrote:
>>> + BUILD_BUG_ON(sizeof(cwsr_trap_gfx11_hex) > PAGE_SIZE);
>>> + cwsr_info->isa_buf = cwsr_trap_gfx11_hex;
>>> + cwsr_info->isa_sz = sizeof(cwsr_trap_gfx11_hex);
>>> + } else {
>>> + BUILD_BUG_ON(sizeof(cwsr_trap_gfx12_hex) >
>>> + AMDGPU_CWSR_TBA_MAX_SIZE);
>>> + cwsr_info->isa_buf = cwsr_trap_gfx12_hex;
>>> + cwsr_info->isa_sz = sizeof(cwsr_trap_gfx12_hex);
>>> + }
>>> +}
>>> +
>>> +int amdgpu_cwsr_init(struct amdgpu_device *adev)
>>> +{
>>> + struct amdgpu_cwsr_info *cwsr_info __free(kfree) = NULL;
>>
>>
>> This is an undesired syntax explicitly documented as one to avoid. You
>> need here proper assignment, not NULL. Please don't use cleanup.h if you
>> do not intend to follow it because it does not make the code simpler.
>>
>
> Could you explain more about the hazard here? There are no multiple
> cleanup variables declared in this case.
>
I am not saying there is a hazard. I am saying that you do not follow
coding style and very explicit, documented rule. There are exceptions of
course, but they need reason and such is missing here. You made the code
worse here, more confusing with the fake assignment, fake constructor.
If you do not want to follow cleanup.h coding style, then simply do not
use cleanup.h. Cleanup.h is to make code simpler but worse.
Best regards,
Krzysztof
