On 04/12/2025 09:06, Krzysztof Kozlowski wrote:
> 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.
s/but worse/,not worse/
Best regards,
Krzysztof
