drm/virtio: use %llu format string form atomic64_t

The virtgpu driver prints the last_seq variable using the %ld or %lu format string, which does not work correctly on all architectures and causes this compiler warning on ARM:

drivers/gpu/drm/virtio/virtgpu_fence.c: In function 'virtio_timeline_value_str':
drivers/gpu/drm/virtio/virtgpu_fence.c:64:22: warning: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'long long int' [-Wformat=]

  snprintf(str, size, "%lu", atomic64_read(&fence->drv->last_seq));
                      ^
drivers/gpu/drm/virtio/virtgpu_debugfs.c: In function 'virtio_gpu_debugfs_irq_info':
drivers/gpu/drm/virtio/virtgpu_debugfs.c:37:16: warning: format '%ld' expects argument of type 'long int', but argument 3 has type 'long long int' [-Wformat=]
  seq_printf(m, "fence %ld %lld\n",
                ^

In order to avoid the warnings, this changes the format strings to %llu and adds a cast to u64, which makes it work the same way everywhere.

Quoted 13 lines · Arnd Bergmann

The virtgpu driver prints the last_seq variable using the %ld or %lu format string, which does not work correctly on all architectures and causes this compiler warning on ARM:

drivers/gpu/drm/virtio/virtgpu_fence.c: In function 'virtio_timeline_value_str':
drivers/gpu/drm/virtio/virtgpu_fence.c:64:22: warning: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'long long int' [-Wformat=]

  snprintf(str, size, "%lu", atomic64_read(&fence->drv->last_seq));
                      ^
drivers/gpu/drm/virtio/virtgpu_debugfs.c: In function 'virtio_gpu_debugfs_irq_info':
drivers/gpu/drm/virtio/virtgpu_debugfs.c:37:16: warning: format '%ld' expects argument of type 'long int', but argument 3 has type 'long long int' [-Wformat=]
  seq_printf(m, "fence %ld %lld\n",
                ^

Quoted 2 lines

In order to avoid the warnings, this changes the format strings to %llu and adds a cast to u64, which makes it work the same way everywhere.

You have to wonder why atomic64_* functions do not use u64 types. If they're not reliant on manipulating 64-bit quantities, then what's the point of calling them atomic _64_.

Quoted 17 lines · On Wednesday 07 October 2015 11:45:02 Russell King - ARM Linux wrote:

Quoted 15 lines · Arnd Bergmann

The virtgpu driver prints the last_seq variable using the %ld or %lu format string, which does not work correctly on all architectures and causes this compiler warning on ARM:

drivers/gpu/drm/virtio/virtgpu_fence.c: In function 'virtio_timeline_value_str':
drivers/gpu/drm/virtio/virtgpu_fence.c:64:22: warning: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'long long int' [-Wformat=]

  snprintf(str, size, "%lu", atomic64_read(&fence->drv->last_seq));
                      ^
drivers/gpu/drm/virtio/virtgpu_debugfs.c: In function 'virtio_gpu_debugfs_irq_info':
drivers/gpu/drm/virtio/virtgpu_debugfs.c:37:16: warning: format '%ld' expects argument of type 'long int', but argument 3 has type 'long long int' [-Wformat=]
  seq_printf(m, "fence %ld %lld\n",
                ^

In order to avoid the warnings, this changes the format strings to %llu and adds a cast to u64, which makes it work the same way everywhere.

Quoted 3 lines

You have to wonder why atomic64_* functions do not use u64 types. If they're not reliant on manipulating 64-bit quantities, then what's the point of calling them atomic _64_.

I haven't checked all architectures, but I assume what happens is that 64-bit ones just #define atomic64_t atomic_long_t, so they don't have to provide three sets of functions.

Quoted 18 lines · On Wednesday 07 October 2015 13:04:06 Arnd Bergmann wrote:

Quoted 17 lines · On Wednesday 07 October 2015 11:45:02 Russell King - ARM Linux wrote:

Quoted 15 lines · Arnd Bergmann

The virtgpu driver prints the last_seq variable using the %ld or %lu format string, which does not work correctly on all architectures and causes this compiler warning on ARM:

drivers/gpu/drm/virtio/virtgpu_fence.c: In function 'virtio_timeline_value_str':
drivers/gpu/drm/virtio/virtgpu_fence.c:64:22: warning: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'long long int' [-Wformat=]

  snprintf(str, size, "%lu", atomic64_read(&fence->drv->last_seq));
                      ^
drivers/gpu/drm/virtio/virtgpu_debugfs.c: In function 'virtio_gpu_debugfs_irq_info':
drivers/gpu/drm/virtio/virtgpu_debugfs.c:37:16: warning: format '%ld' expects argument of type 'long int', but argument 3 has type 'long long int' [-Wformat=]
  seq_printf(m, "fence %ld %lld\n",
                ^

In order to avoid the warnings, this changes the format strings to %llu and adds a cast to u64, which makes it work the same way everywhere.

Quoted 7 lines

Quoted 3 lines

You have to wonder why atomic64_* functions do not use u64 types. If they're not reliant on manipulating 64-bit quantities, then what's the point of calling them atomic _64_.

I haven't checked all architectures, but I assume what happens is that 64-bit ones just #define atomic64_t atomic_long_t, so they don't have to provide three sets of functions.

scratch that, I just looked at all the architectures and found that it's just completely arbitrary, even within one architecture you get a mix of 'long' and 'long long', plus this gem from MIPS:

static __inline__ int atomic64_add_unless(atomic64_t *v, long a, long u)

which truncates the result to 32 bit.

Quoted 3 lines · Arnd Bergmann

static __inline__ int atomic64_add_unless(atomic64_t *v, long a, long u)

which truncates the result to 32 bit.

Woops.

See also my unanswered question in "atomic64 on 32-bit vs 64-bit (was:
Re: Add virtio gpu driver.)", which is still valid:
https://lkml.org/lkml/2015/6/28/18

Instead of sprinkling casts, is there any good reason why atomic64_read() and atomic64_t aren't "long long" everywhere, cfr. u64?

Quoted 11 lines · On Monday 19 October 2015 09:34:15 Geert Uytterhoeven wrote:

Quoted 3 lines · Arnd Bergmann

static __inline__ int atomic64_add_unless(atomic64_t *v, long a, long u)

which truncates the result to 32 bit.

Woops.

See also my unanswered question in "atomic64 on 32-bit vs 64-bit (was:
Re: Add virtio gpu driver.)", which is still valid:
https://lkml.org/lkml/2015/6/28/18

Regarding your question of

Quoted 2 lines

Instead of sprinkling casts, is there any good reason why atomic64_read() and atomic64_t aren't "long long" everywhere, cfr. u64?

I assume the answer is that some (all?) 64-bit architectures intentionally return 'long' here, in order for atomic_long_read() to return 'long' on all architectures, given the definitions from include/asm-generic/atomic-long.h

We would have to either change those, or we have to pick between atomic_long_* or atomic64_* to have a consistent return type.

Quoted 11 lines · On Monday 19 October 2015 09:34:15 Geert Uytterhoeven wrote:

Quoted 3 lines · Arnd Bergmann

static __inline__ int atomic64_add_unless(atomic64_t *v, long a, long u)

which truncates the result to 32 bit.

Woops.

See also my unanswered question in "atomic64 on 32-bit vs 64-bit (was:
Re: Add virtio gpu driver.)", which is still valid:
https://lkml.org/lkml/2015/6/28/18

Regarding your question of

Quoted 2 lines

Instead of sprinkling casts, is there any good reason why atomic64_read() and atomic64_t aren't "long long" everywhere, cfr. u64?

I assume the answer is that some (all?) 64-bit architectures intentionally return 'long' here, in order for atomic_long_read() to return 'long' on all architectures, given the definitions from include/asm-generic/atomic-long.h

We would have to either change those, or we have to pick between atomic_long_* or atomic64_* to have a consistent return type.

Quoted 18 lines · On Wednesday 07 October 2015 13:04:06 Arnd Bergmann wrote:

Quoted 17 lines · On Wednesday 07 October 2015 11:45:02 Russell King - ARM Linux wrote:

Quoted 15 lines · Arnd Bergmann

The virtgpu driver prints the last_seq variable using the %ld or %lu format string, which does not work correctly on all architectures and causes this compiler warning on ARM:

drivers/gpu/drm/virtio/virtgpu_fence.c: In function 'virtio_timeline_value_str':
drivers/gpu/drm/virtio/virtgpu_fence.c:64:22: warning: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'long long int' [-Wformat=]

  snprintf(str, size, "%lu", atomic64_read(&fence->drv->last_seq));
                      ^
drivers/gpu/drm/virtio/virtgpu_debugfs.c: In function 'virtio_gpu_debugfs_irq_info':
drivers/gpu/drm/virtio/virtgpu_debugfs.c:37:16: warning: format '%ld' expects argument of type 'long int', but argument 3 has type 'long long int' [-Wformat=]
  seq_printf(m, "fence %ld %lld\n",
                ^

In order to avoid the warnings, this changes the format strings to %llu and adds a cast to u64, which makes it work the same way everywhere.

Quoted 7 lines

Quoted 3 lines

You have to wonder why atomic64_* functions do not use u64 types. If they're not reliant on manipulating 64-bit quantities, then what's the point of calling them atomic _64_.

I haven't checked all architectures, but I assume what happens is that 64-bit ones just #define atomic64_t atomic_long_t, so they don't have to provide three sets of functions.

scratch that, I just looked at all the architectures and found that it's just completely arbitrary, even within one architecture you get a mix of 'long' and 'long long', plus this gem from MIPS:

static __inline__ int atomic64_add_unless(atomic64_t *v, long a, long u)

which truncates the result to 32 bit.

Quoted 3 lines

I haven't checked all architectures, but I assume what happens is that 64-bit ones just #define atomic64_t atomic_long_t, so they don't have to provide three sets of functions.

scratch that, I just looked at all the architectures and found that it's just completely arbitrary, even within one architecture you get a mix of 'long' and 'long long', plus this gem from MIPS:

static __inline__ int atomic64_add_unless(atomic64_t *v, long a, long u)

which truncates the result to 32 bit.

Quoted 8 lines · Arnd Bergmann

Quoted 3 lines

I haven't checked all architectures, but I assume what happens is that 64-bit ones just #define atomic64_t atomic_long_t, so they don't have to provide three sets of functions.

scratch that, I just looked at all the architectures and found that it's just completely arbitrary, even within one architecture you get a mix of 'long' and 'long long', plus this gem from MIPS:

Quoted 3 lines

static __inline__ int atomic64_add_unless(atomic64_t *v, long a, long u)

which truncates the result to 32 bit.

Eh... The result is 0/1 so nothing is truncated. Alpha, MIPS, PARISC and PowerPC are using the same prototype and x86 only differs in the use of inline instead __inline__. And anyway, that function on MIPS is only built for CONFIG_64BIT.

What's wrong on MIPS is the comment describing the function's return value which was changed by f24219b4e90cf70ec4a211b17fbabc725a0ddf3c (atomic: move atomic_add_unless to generic code) and I've queued up a patch to fix that since a few days. I guess that was a cut and paste error from __atomic_add_unless which indeed does return the old value.

Quoted 8 lines · Arnd Bergmann

Quoted 3 lines

I haven't checked all architectures, but I assume what happens is that 64-bit ones just #define atomic64_t atomic_long_t, so they don't have to provide three sets of functions.

scratch that, I just looked at all the architectures and found that it's just completely arbitrary, even within one architecture you get a mix of 'long' and 'long long', plus this gem from MIPS:

Quoted 3 lines

static __inline__ int atomic64_add_unless(atomic64_t *v, long a, long u)

which truncates the result to 32 bit.

Eh... The result is 0/1 so nothing is truncated. Alpha, MIPS, PARISC and PowerPC are using the same prototype and x86 only differs in the use of inline instead __inline__. And anyway, that function on MIPS is only built for CONFIG_64BIT.

What's wrong on MIPS is the comment describing the function's return value which was changed by f24219b4e90cf70ec4a211b17fbabc725a0ddf3c (atomic: move atomic_add_unless to generic code) and I've queued up a patch to fix that since a few days. I guess that was a cut and paste error from __atomic_add_unless which indeed does return the old value.

Quoted 3 lines

I haven't checked all architectures, but I assume what happens is that 64-bit ones just #define atomic64_t atomic_long_t, so they don't have to provide three sets of functions.

scratch that, I just looked at all the architectures and found that it's just completely arbitrary, even within one architecture you get a mix of 'long' and 'long long', plus this gem from MIPS:

static __inline__ int atomic64_add_unless(atomic64_t *v, long a, long u)

which truncates the result to 32 bit.

Quoted 17 lines · On Wednesday 07 October 2015 11:45:02 Russell King - ARM Linux wrote:

Quoted 15 lines · Arnd Bergmann

The virtgpu driver prints the last_seq variable using the %ld or %lu format string, which does not work correctly on all architectures and causes this compiler warning on ARM:

drivers/gpu/drm/virtio/virtgpu_fence.c: In function 'virtio_timeline_value_str':
drivers/gpu/drm/virtio/virtgpu_fence.c:64:22: warning: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'long long int' [-Wformat=]

  snprintf(str, size, "%lu", atomic64_read(&fence->drv->last_seq));
                      ^
drivers/gpu/drm/virtio/virtgpu_debugfs.c: In function 'virtio_gpu_debugfs_irq_info':
drivers/gpu/drm/virtio/virtgpu_debugfs.c:37:16: warning: format '%ld' expects argument of type 'long int', but argument 3 has type 'long long int' [-Wformat=]
  seq_printf(m, "fence %ld %lld\n",
                ^

In order to avoid the warnings, this changes the format strings to %llu and adds a cast to u64, which makes it work the same way everywhere.

Quoted 3 lines

You have to wonder why atomic64_* functions do not use u64 types. If they're not reliant on manipulating 64-bit quantities, then what's the point of calling them atomic _64_.

I haven't checked all architectures, but I assume what happens is that 64-bit ones just #define atomic64_t atomic_long_t, so they don't have to provide three sets of functions.

Quoted 13 lines · Arnd Bergmann

The virtgpu driver prints the last_seq variable using the %ld or %lu format string, which does not work correctly on all architectures and causes this compiler warning on ARM:

drivers/gpu/drm/virtio/virtgpu_fence.c: In function 'virtio_timeline_value_str':
drivers/gpu/drm/virtio/virtgpu_fence.c:64:22: warning: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'long long int' [-Wformat=]

  snprintf(str, size, "%lu", atomic64_read(&fence->drv->last_seq));
                      ^
drivers/gpu/drm/virtio/virtgpu_debugfs.c: In function 'virtio_gpu_debugfs_irq_info':
drivers/gpu/drm/virtio/virtgpu_debugfs.c:37:16: warning: format '%ld' expects argument of type 'long int', but argument 3 has type 'long long int' [-Wformat=]
  seq_printf(m, "fence %ld %lld\n",
                ^

Quoted 2 lines

In order to avoid the warnings, this changes the format strings to %llu and adds a cast to u64, which makes it work the same way everywhere.

You have to wonder why atomic64_* functions do not use u64 types. If they're not reliant on manipulating 64-bit quantities, then what's the point of calling them atomic _64_.

The virtgpu driver prints the last_seq variable using the %ld or %lu format string, which does not work correctly on all architectures and causes this compiler warning on ARM:

drivers/gpu/drm/virtio/virtgpu_fence.c: In function 'virtio_timeline_value_str':
drivers/gpu/drm/virtio/virtgpu_fence.c:64:22: warning: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'long long int' [-Wformat=]

  snprintf(str, size, "%lu", atomic64_read(&fence->drv->last_seq));
                      ^
drivers/gpu/drm/virtio/virtgpu_debugfs.c: In function 'virtio_gpu_debugfs_irq_info':
drivers/gpu/drm/virtio/virtgpu_debugfs.c:37:16: warning: format '%ld' expects argument of type 'long int', but argument 3 has type 'long long int' [-Wformat=]
  seq_printf(m, "fence %ld %lld\n",
                ^

In order to avoid the warnings, this changes the format strings to %llu and adds a cast to u64, which makes it work the same way everywhere.