* [PATCH] drm/amdkfd: Cast atomic64_read return value
@ 2021-09-13 14:19 Michel Dänzer
2021-09-13 15:19 ` Felix Kuehling
0 siblings, 1 reply; 6+ messages in thread
From: Michel Dänzer @ 2021-09-13 14:19 UTC (permalink / raw)
To: Felix Kuehling, Alex Deucher, Christian König, Pan, Xinhui
Cc: Lyude Paul, amd-gfx
From: Michel Dänzer <mdaenzer@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
Signed-off-by: Michel Dänzer <mdaenzer@redhat.com>
---
drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c b/drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c
index ed4bc5f844ce..46e1c0cda94c 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c
@@ -223,7 +223,7 @@ void kfd_smi_event_update_thermal_throttling(struct kfd_dev *dev,
len = snprintf(fifo_in, sizeof(fifo_in), "%x %llx:%llx\n",
KFD_SMI_EVENT_THERMAL_THROTTLE, throttle_bitmask,
- atomic64_read(&adev->smu.throttle_int_counter));
+ (u64)atomic64_read(&adev->smu.throttle_int_counter));
add_event_to_kfifo(dev, KFD_SMI_EVENT_THERMAL_THROTTLE, fifo_in, len);
}
--
2.33.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] drm/amdkfd: Cast atomic64_read return value
2021-09-13 14:19 [PATCH] drm/amdkfd: Cast atomic64_read return value Michel Dänzer
@ 2021-09-13 15:19 ` Felix Kuehling
2021-09-13 16:18 ` Michel Dänzer
0 siblings, 1 reply; 6+ messages in thread
From: Felix Kuehling @ 2021-09-13 15:19 UTC (permalink / raw)
To: Michel Dänzer, Alex Deucher, Christian König, Pan, Xinhui
Cc: Lyude Paul, amd-gfx
Am 2021-09-13 um 10:19 a.m. schrieb Michel Dänzer:
> From: Michel Dänzer <mdaenzer@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? For the record, these are the definition for x86 and x86_64 on Linux
5.13:
./arch/x86/include/asm/atomic64_32.h:static inline s64
arch_atomic64_read(const atomic64_t *v)
./arch/x86/include/asm/atomic64_64.h:static inline s64
arch_atomic64_read(const atomic64_t *v)
Looks like x86 uses int-ll64.h (64-bit types are long-long). Some other
architectures use int-l64.h (64-bit types are long). On architectures
that use int-l64.h, this patch just casts s64 (long) to u64 (unsigned
long), which doesn't fix the problem.
Regards,
Felix
>
> Signed-off-by: Michel Dänzer <mdaenzer@redhat.com>
> ---
> drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c b/drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c
> index ed4bc5f844ce..46e1c0cda94c 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c
> @@ -223,7 +223,7 @@ void kfd_smi_event_update_thermal_throttling(struct kfd_dev *dev,
>
> len = snprintf(fifo_in, sizeof(fifo_in), "%x %llx:%llx\n",
> KFD_SMI_EVENT_THERMAL_THROTTLE, throttle_bitmask,
> - atomic64_read(&adev->smu.throttle_int_counter));
> + (u64)atomic64_read(&adev->smu.throttle_int_counter));
>
> add_event_to_kfifo(dev, KFD_SMI_EVENT_THERMAL_THROTTLE, fifo_in, len);
> }
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdkfd: Cast atomic64_read return value
2021-09-13 15:19 ` Felix Kuehling
@ 2021-09-13 16:18 ` Michel Dänzer
2021-09-13 16:28 ` Felix Kuehling
0 siblings, 1 reply; 6+ messages in thread
From: Michel Dänzer @ 2021-09-13 16:18 UTC (permalink / raw)
To: Felix Kuehling, Alex Deucher, Christian König, Pan, Xinhui
Cc: Lyude Paul, amd-gfx
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@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.
--
Earthling Michel Dänzer | https://redhat.com
Libre software enthusiast | Mesa and X developer
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdkfd: Cast atomic64_read return value
2021-09-13 16:18 ` Michel Dänzer
@ 2021-09-13 16:28 ` Felix Kuehling
2021-09-13 16:42 ` Michel Dänzer
0 siblings, 1 reply; 6+ messages in thread
From: Felix Kuehling @ 2021-09-13 16:28 UTC (permalink / raw)
To: Michel Dänzer, Alex Deucher, Christian König, Pan, Xinhui
Cc: Lyude Paul, amd-gfx
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@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.
Regards,
Felix
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdkfd: Cast atomic64_read return value
2021-09-13 16:28 ` Felix Kuehling
@ 2021-09-13 16:42 ` Michel Dänzer
0 siblings, 0 replies; 6+ messages in thread
From: Michel Dänzer @ 2021-09-13 16:42 UTC (permalink / raw)
To: Felix Kuehling, Alex Deucher, Christian König, Pan, Xinhui
Cc: Lyude Paul, amd-gfx
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@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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] drm/amdkfd: Cast atomic64_read return value
@ 2021-09-13 14:15 Michel Dänzer
0 siblings, 0 replies; 6+ messages in thread
From: Michel Dänzer @ 2021-09-13 14:15 UTC (permalink / raw)
To: Felix Kuehling, Alex Deucher, Christian König, Pan, Xinhui
Cc: Lyude Paul, amd-gfx
From: Michel Dänzer <mdaenzer@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
Signed-off-by: Michel Dänzer <mdaenzer@redhat.com>
---
drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c b/drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c
index ed4bc5f844ce..46e1c0cda94c 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_smi_events.c
@@ -223,7 +223,7 @@ void kfd_smi_event_update_thermal_throttling(struct kfd_dev *dev,
len = snprintf(fifo_in, sizeof(fifo_in), "%x %llx:%llx\n",
KFD_SMI_EVENT_THERMAL_THROTTLE, throttle_bitmask,
- atomic64_read(&adev->smu.throttle_int_counter));
+ (u64)atomic64_read(&adev->smu.throttle_int_counter));
add_event_to_kfifo(dev, KFD_SMI_EVENT_THERMAL_THROTTLE, fifo_in, len);
}
--
2.33.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-09-13 16:42 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-13 14:19 [PATCH] drm/amdkfd: Cast atomic64_read return value Michel Dänzer
2021-09-13 15:19 ` Felix Kuehling
2021-09-13 16:18 ` Michel Dänzer
2021-09-13 16:28 ` Felix Kuehling
2021-09-13 16:42 ` Michel Dänzer
-- strict thread matches above, loose matches on Subject: below --
2021-09-13 14:15 Michel Dänzer
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.