[PATCH] drm/amdkfd: Cast atomic64_read return value

[Michel Dänzer]

On 2021-09-13 18:28, Felix Kuehling wrote:
> Am 2021-09-13 um 12:18 p.m. schrieb Michel Dänzer:
>> On 2021-09-13 17:19, Felix Kuehling wrote:
>>> Am 2021-09-13 um 10:19 a.m. schrieb Michel Dänzer:
>>>> From: Michel Dänzer <mdaenzer at redhat.com>
>>>>
>>>> Avoids warning with -Wformat:
>>>>
>>>>   CC [M]  drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_smi_events.o
>>>> ../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_smi_events.c: In function ‘kfd_smi_event_update_thermal_throttling’:
>>>> ../drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_smi_events.c:224:60: warning: format ‘%llx’ expects argument of type
>>>>  ‘long long unsigned int’, but argument 6 has type ‘long int’ [-Wformat=]
>>>>   224 |         len = snprintf(fifo_in, sizeof(fifo_in), "%x %x:%llx\n",
>>>>       |                                                         ~~~^
>>>>       |                                                            |
>>>>       |                                                            long long unsigned int
>>>>       |                                                         %lx
>>>>   225 |                        KFD_SMI_EVENT_THERMAL_THROTTLE, throttle_bitmask,
>>>>   226 |                        atomic64_read(&adev->smu.throttle_int_counter));
>>>>       |                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>>       |                        |
>>>>       |                        long int
>>> That's weird. As far as I can see, atomic64_read is defined to return
>>> s64, which should be the same as long long. Which architecture are you
>>> on?
>> This was from a 64-bit powerpc build. atomic64_read returns long there.
>>
>>
> This should be defined as s64:
> 
> ./arch/powerpc/include/asm/atomic.h:static __inline__ s64 atomic64_read(const atomic64_t *v)
> 
> In arch/powerpc/include/uapi/asm/types.h I see this:
> 
> /*
>  * This is here because we used to use l64 for 64bit powerpc
>  * and we don't want to impact user mode with our change to ll64
>  * in the kernel.
>  *
>  * However, some user programs are fine with this.  They can
>  * flag __SANE_USERSPACE_TYPES__ to get int-ll64.h here.
>  */
> #if !defined(__SANE_USERSPACE_TYPES__) && defined(__powerpc64__) && !defined(__KERNEL__)
> # include <asm-generic/int-l64.h>
> #else
> # include <asm-generic/int-ll64.h>
> #endif
> 
> 
> So in kernel mode it should be using int-ll64.h, which defines s64 as
> long-long. The cast to u64 won't help either way. It's either
> unnecessary or it's still unsigned long.

Ah, I see now this is because the RHEL 8 kernel is based on 4.18, where this still returned long for powerpc.

I guess I'll have to deal with this downstream, sorry for the noise.


-- 
Earthling Michel Dänzer               |               https://redhat.com
Libre software enthusiast             |             Mesa and X developer