[PATCH v1] x86/io: Define readq()/writeq() to use 64-bit type

[PATCH v1] x86/io: Define readq()/writeq() to use 64-bit type

[Andy Shevchenko]

Since non atomic readq() and writeq() were added some of the drivers
would like to use it in a manner of:

 #include <io-64-nonatomic-lo-hi.h>
...
 pr_debug("Debug value of some register: %016llx\n", readq(addr));

However, lo_hi_readq() always returns __u64 data, while readq()
on x86_64 defines it as unsigned long. and thus compiler warns
about type mismatch, although they are both 64-bit on x86_64.

Convert readq() and writeq() on x86 to operate on deterministic
64-bit type. The most of architectures in the kernel already are using
either unsigned long long, or u64 type for readq() / writeq().
This change propagates consistency in that sense.

While this is not an issue per se, though if someone wants to address it,
the anchor could be the commit

  797a796a13df ("asm-generic: architecture independent readq/writeq for 32bit environment")

where non-atomic variants had been introduced.

Note, there are only few users of above pattern and they will not be
affected because they do cast returned value. The actual warning has
been issued on not-yet-upstreamed code.

Potentially we might get a new warnings if some 64-bit only code
assigns returned value to unsigned long type of variable. This is
assumed to be addressed on case-by-case basis.

Reported-by: lkp <lkp@intel.com>
Cc: Hitoshi Mitake <mitake@dcl.info.waseda.ac.jp>
Cc: "Mehta, Sohil" <sohil.mehta@intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 arch/x86/include/asm/io.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
index 95e948627fd0..365f5ba9222b 100644
--- a/arch/x86/include/asm/io.h
+++ b/arch/x86/include/asm/io.h
@@ -94,10 +94,10 @@ build_mmio_write(__writel, "l", unsigned int, "r", )
 
 #ifdef CONFIG_X86_64
 
-build_mmio_read(readq, "q", unsigned long, "=r", :"memory")
-build_mmio_read(__readq, "q", unsigned long, "=r", )
-build_mmio_write(writeq, "q", unsigned long, "r", :"memory")
-build_mmio_write(__writeq, "q", unsigned long, "r", )
+build_mmio_read(readq, "q", unsigned long long, "=r", :"memory")
+build_mmio_read(__readq, "q", unsigned long long, "=r", )
+build_mmio_write(writeq, "q", unsigned long long, "r", :"memory")
+build_mmio_write(__writeq, "q", unsigned long long, "r", )
 
 #define readq_relaxed(a)	__readq(a)
 #define writeq_relaxed(v, a)	__writeq(v, a)
-- 
2.15.1

Re: [PATCH v1] x86/io: Define readq()/writeq() to use 64-bit type

[Mehta, Sohil]

On Fri, 2018-01-19 at 16:33 +0200, Andy Shevchenko wrote:
> Since non atomic readq() and writeq() were added some of the drivers
> would like to use it in a manner of:
> 
>  #include <io-64-nonatomic-lo-hi.h>
> ...
>  pr_debug("Debug value of some register: %016llx\n", readq(addr));
> 
> However, lo_hi_readq() always returns __u64 data, while readq()
> on x86_64 defines it as unsigned long. and thus compiler warns
> about type mismatch, although they are both 64-bit on x86_64.
> 
> Convert readq() and writeq() on x86 to operate on deterministic
> 64-bit type. The most of architectures in the kernel already are
> using
> either unsigned long long, or u64 type for readq() / writeq().
> This change propagates consistency in that sense.
> 
> While this is not an issue per se, though if someone wants to address
> it,
> the anchor could be the commit
> 
>   797a796a13df ("asm-generic: architecture independent readq/writeq
> for 32bit environment")
> 
> where non-atomic variants had been introduced.
> 
> Note, there are only few users of above pattern and they will not be
> affected because they do cast returned value. The actual warning has
> been issued on not-yet-upstreamed code.
> 
> Potentially we might get a new warnings if some 64-bit only code
> assigns returned value to unsigned long type of variable. This is
> assumed to be addressed on case-by-case basis.
> 
> Reported-by: lkp <lkp@intel.com>
> Cc: Hitoshi Mitake <mitake@dcl.info.waseda.ac.jp>
> Cc: "Mehta, Sohil" <sohil.mehta@intel.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  arch/x86/include/asm/io.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
> index 95e948627fd0..365f5ba9222b 100644
> --- a/arch/x86/include/asm/io.h
> +++ b/arch/x86/include/asm/io.h
> @@ -94,10 +94,10 @@ build_mmio_write(__writel, "l", unsigned int,
> "r", )
>  
>  #ifdef CONFIG_X86_64
>  
> -build_mmio_read(readq, "q", unsigned long, "=r", :"memory")
> -build_mmio_read(__readq, "q", unsigned long, "=r", )
> -build_mmio_write(writeq, "q", unsigned long, "r", :"memory")
> -build_mmio_write(__writeq, "q", unsigned long, "r", )
> +build_mmio_read(readq, "q", unsigned long long, "=r", :"memory")
> +build_mmio_read(__readq, "q", unsigned long long, "=r", )
> +build_mmio_write(writeq, "q", unsigned long long, "r", :"memory")
> +build_mmio_write(__writeq, "q", unsigned long long, "r", )
>  
>  #define readq_relaxed(a)	__readq(a)
>  #define writeq_relaxed(v, a)	__writeq(v, a)

The patch works for me:

Tested-by: Sohil Mehta <sohil.mehta@intel.com>

Sohil

Re: [PATCH v1] x86/io: Define readq()/writeq() to use 64-bit type

[hpa]

On January 22, 2018 4:32:14 PM PST, "Mehta, Sohil" <sohil.mehta@intel.com> wrote:
>On Fri, 2018-01-19 at 16:33 +0200, Andy Shevchenko wrote:
>> Since non atomic readq() and writeq() were added some of the drivers
>> would like to use it in a manner of:
>> 
>>  #include <io-64-nonatomic-lo-hi.h>
>> ...
>>  pr_debug("Debug value of some register: %016llx\n", readq(addr));
>> 
>> However, lo_hi_readq() always returns __u64 data, while readq()
>> on x86_64 defines it as unsigned long. and thus compiler warns
>> about type mismatch, although they are both 64-bit on x86_64.
>> 
>> Convert readq() and writeq() on x86 to operate on deterministic
>> 64-bit type. The most of architectures in the kernel already are
>> using
>> either unsigned long long, or u64 type for readq() / writeq().
>> This change propagates consistency in that sense.
>> 
>> While this is not an issue per se, though if someone wants to address
>> it,
>> the anchor could be the commit
>> 
>>   797a796a13df ("asm-generic: architecture independent readq/writeq
>> for 32bit environment")
>> 
>> where non-atomic variants had been introduced.
>> 
>> Note, there are only few users of above pattern and they will not be
>> affected because they do cast returned value. The actual warning has
>> been issued on not-yet-upstreamed code.
>> 
>> Potentially we might get a new warnings if some 64-bit only code
>> assigns returned value to unsigned long type of variable. This is
>> assumed to be addressed on case-by-case basis.
>> 
>> Reported-by: lkp <lkp@intel.com>
>> Cc: Hitoshi Mitake <mitake@dcl.info.waseda.ac.jp>
>> Cc: "Mehta, Sohil" <sohil.mehta@intel.com>
>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> ---
>>  arch/x86/include/asm/io.h | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>> 
>> diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
>> index 95e948627fd0..365f5ba9222b 100644
>> --- a/arch/x86/include/asm/io.h
>> +++ b/arch/x86/include/asm/io.h
>> @@ -94,10 +94,10 @@ build_mmio_write(__writel, "l", unsigned int,
>> "r", )
>>  
>>  #ifdef CONFIG_X86_64
>>  
>> -build_mmio_read(readq, "q", unsigned long, "=r", :"memory")
>> -build_mmio_read(__readq, "q", unsigned long, "=r", )
>> -build_mmio_write(writeq, "q", unsigned long, "r", :"memory")
>> -build_mmio_write(__writeq, "q", unsigned long, "r", )
>> +build_mmio_read(readq, "q", unsigned long long, "=r", :"memory")
>> +build_mmio_read(__readq, "q", unsigned long long, "=r", )
>> +build_mmio_write(writeq, "q", unsigned long long, "r", :"memory")
>> +build_mmio_write(__writeq, "q", unsigned long long, "r", )
>>  
>>  #define readq_relaxed(a)	__readq(a)
>>  #define writeq_relaxed(v, a)	__writeq(v, a)
>
>The patch works for me:
>
>Tested-by: Sohil Mehta <sohil.mehta@intel.com>
>
>Sohil

Wouldn't simply u64 make more sense?
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: [PATCH v1] x86/io: Define readq()/writeq() to use 64-bit type

[Andy Shevchenko]

On Mon, 2018-01-22 at 16:46 -0800, hpa@zytor.com wrote:
> On January 22, 2018 4:32:14 PM PST, "Mehta, Sohil" <sohil.mehta@intel.
> com> wrote:
> > On Fri, 2018-01-19 at 16:33 +0200, Andy Shevchenko wrote:

> > > +build_mmio_read(readq, "q", unsigned long long, "=r", :"memory")
> > > +build_mmio_read(__readq, "q", unsigned long long, "=r", )
> > > +build_mmio_write(writeq, "q", unsigned long long, "r", :"memory")
> > > +build_mmio_write(__writeq, "q", unsigned long long, "r", )
> > >  
> > >  #define readq_relaxed(a)	__readq(a)
> > >  #define writeq_relaxed(v, a)	__writeq(v, a)
> > 
> > The patch works for me:
> > 
> > Tested-by: Sohil Mehta <sohil.mehta@intel.com>
> > 

> Wouldn't simply u64 make more sense?

It would break a common style used in this module for the rest of
accessors. 

So, I prefer to go with unsigned long long and change later, if needed,
from POD types to uNN ones in entire file.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH v1] x86/io: Define readq()/writeq() to use 64-bit type

[Andy Shevchenko]

On Tue, 2018-01-23 at 10:32 +0200, Andy Shevchenko wrote:
> On Mon, 2018-01-22 at 16:46 -0800, hpa@zytor.com wrote:
> > On January 22, 2018 4:32:14 PM PST, "Mehta, Sohil"
> > <sohil.mehta@intel.com> wrote:
> > > On Fri, 2018-01-19 at 16:33 +0200, Andy Shevchenko wrote:
> > > > +build_mmio_read(readq, "q", unsigned long long, "=r",
> > > > :"memory")
> > > > +build_mmio_read(__readq, "q", unsigned long long, "=r", )
> > > > +build_mmio_write(writeq, "q", unsigned long long, "r",
> > > > :"memory")
> > > > +build_mmio_write(__writeq, "q", unsigned long long, "r", )

> > > The patch works for me:
> > > Tested-by: Sohil Mehta <sohil.mehta@intel.com>

> > Wouldn't simply u64 make more sense?

> It would break a common style used in this module for the rest of
> accessors. 

> So, I prefer to go with unsigned long long and change later, if
> needed,
> from POD types to uNN ones in entire file.

So, Peter, Ingo, Thomas, can we move forward with this one?

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

[tip:x86/urgent] x86/io: Define readq()/writeq() to use 64-bit type

[tip-bot for Andy Shevchenko]

Commit-ID:  0fc8483b698620ea3d8cc6635b54eccc613c23a3
Gitweb:     https://git.kernel.org/tip/0fc8483b698620ea3d8cc6635b54eccc613c23a3
Author:     Andy Shevchenko <andriy.shevchenko@linux.intel.com>
AuthorDate: Fri, 19 Jan 2018 16:33:22 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 13 Feb 2018 17:14:41 +0100

x86/io: Define readq()/writeq() to use 64-bit type

Since non atomic readq() and writeq() were added some of the drivers
would like to use it in a manner of:

 #include <io-64-nonatomic-lo-hi.h>
...
 pr_debug("Debug value of some register: %016llx\n", readq(addr));

However, lo_hi_readq() always returns __u64 data, while readq()
on x86_64 defines it as unsigned long. and thus compiler warns
about type mismatch, although they are both 64-bit on x86_64.

Convert readq() and writeq() on x86 to operate on deterministic
64-bit type. The most of architectures in the kernel already are using
either unsigned long long, or u64 type for readq() / writeq().
This change propagates consistency in that sense.

While this is not an issue per se, though if someone wants to address it,
the anchor could be the commit

  797a796a13df ("asm-generic: architecture independent readq/writeq for 32bit environment")

where non-atomic variants had been introduced.

Note, there are only few users of above pattern and they will not be
affected because they do cast returned value. The actual warning has
been issued on not-yet-upstreamed code.

Potentially we might get a new warnings if some 64-bit only code
assigns returned value to unsigned long type of variable. This is
assumed to be addressed on case-by-case basis.

Reported-by: lkp <lkp@intel.com>
Tested-by: Sohil Mehta <sohil.mehta@intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Hitoshi Mitake <mitake@dcl.info.waseda.ac.jp>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mehta, Sohil <sohil.mehta@intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/20180119143322.16555-1-andriy.shevchenko@linux.intel.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/include/asm/io.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
index 95e9486..365f5ba 100644
--- a/arch/x86/include/asm/io.h
+++ b/arch/x86/include/asm/io.h
@@ -94,10 +94,10 @@ build_mmio_write(__writel, "l", unsigned int, "r", )
 
 #ifdef CONFIG_X86_64
 
-build_mmio_read(readq, "q", unsigned long, "=r", :"memory")
-build_mmio_read(__readq, "q", unsigned long, "=r", )
-build_mmio_write(writeq, "q", unsigned long, "r", :"memory")
-build_mmio_write(__writeq, "q", unsigned long, "r", )
+build_mmio_read(readq, "q", unsigned long long, "=r", :"memory")
+build_mmio_read(__readq, "q", unsigned long long, "=r", )
+build_mmio_write(writeq, "q", unsigned long long, "r", :"memory")
+build_mmio_write(__writeq, "q", unsigned long long, "r", )
 
 #define readq_relaxed(a)	__readq(a)
 #define writeq_relaxed(v, a)	__writeq(v, a)

Re: [tip:x86/urgent] x86/io: Define readq()/writeq() to use 64-bit type

[Andy Shevchenko]

On Tue, 2018-02-13 at 08:58 -0800, tip-bot for Andy Shevchenko wrote:
> Commit-ID:  0fc8483b698620ea3d8cc6635b54eccc613c23a3
> Gitweb:     https://git.kernel.org/tip/0fc8483b698620ea3d8cc6635b54ecc
> c613c23a3
> Author:     Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> AuthorDate: Fri, 19 Jan 2018 16:33:22 +0200
> Committer:  Ingo Molnar <mingo@kernel.org>
> CommitDate: Tue, 13 Feb 2018 17:14:41 +0100

As of today I don't see this in linux-next

% git tag --contains 0fc8483b69862
next-20180214
next-20180215
next-20180216

What happened to this change?

> 
> x86/io: Define readq()/writeq() to use 64-bit type
> 
> Since non atomic readq() and writeq() were added some of the drivers
> would like to use it in a manner of:
> 
>  #include <io-64-nonatomic-lo-hi.h>
> ...
>  pr_debug("Debug value of some register: %016llx\n", readq(addr));
> 
> However, lo_hi_readq() always returns __u64 data, while readq()
> on x86_64 defines it as unsigned long. and thus compiler warns
> about type mismatch, although they are both 64-bit on x86_64.
> 
> Convert readq() and writeq() on x86 to operate on deterministic
> 64-bit type. The most of architectures in the kernel already are using
> either unsigned long long, or u64 type for readq() / writeq().
> This change propagates consistency in that sense.
> 
> While this is not an issue per se, though if someone wants to address
> it,
> the anchor could be the commit
> 
>   797a796a13df ("asm-generic: architecture independent readq/writeq
> for 32bit environment")
> 
> where non-atomic variants had been introduced.
> 
> Note, there are only few users of above pattern and they will not be
> affected because they do cast returned value. The actual warning has
> been issued on not-yet-upstreamed code.
> 
> Potentially we might get a new warnings if some 64-bit only code
> assigns returned value to unsigned long type of variable. This is
> assumed to be addressed on case-by-case basis.
> 
> Reported-by: lkp <lkp@intel.com>
> Tested-by: Sohil Mehta <sohil.mehta@intel.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Hitoshi Mitake <mitake@dcl.info.waseda.ac.jp>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Mehta, Sohil <sohil.mehta@intel.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Link: http://lkml.kernel.org/r/20180119143322.16555-1-andriy.shevchenk
> o@linux.intel.com
> Signed-off-by: Ingo Molnar <mingo@kernel.org>
> ---
>  arch/x86/include/asm/io.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
> index 95e9486..365f5ba 100644
> --- a/arch/x86/include/asm/io.h
> +++ b/arch/x86/include/asm/io.h
> @@ -94,10 +94,10 @@ build_mmio_write(__writel, "l", unsigned int, "r",
> )
>  
>  #ifdef CONFIG_X86_64
>  
> -build_mmio_read(readq, "q", unsigned long, "=r", :"memory")
> -build_mmio_read(__readq, "q", unsigned long, "=r", )
> -build_mmio_write(writeq, "q", unsigned long, "r", :"memory")
> -build_mmio_write(__writeq, "q", unsigned long, "r", )
> +build_mmio_read(readq, "q", unsigned long long, "=r", :"memory")
> +build_mmio_read(__readq, "q", unsigned long long, "=r", )
> +build_mmio_write(writeq, "q", unsigned long long, "r", :"memory")
> +build_mmio_write(__writeq, "q", unsigned long long, "r", )
>  
>  #define readq_relaxed(a)	__readq(a)
>  #define writeq_relaxed(v, a)	__writeq(v, a)

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [tip:x86/urgent] x86/io: Define readq()/writeq() to use 64-bit type

[Ingo Molnar]

* Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Tue, 2018-02-13 at 08:58 -0800, tip-bot for Andy Shevchenko wrote:
> > Commit-ID:  0fc8483b698620ea3d8cc6635b54eccc613c23a3
> > Gitweb:     https://git.kernel.org/tip/0fc8483b698620ea3d8cc6635b54ecc
> > c613c23a3
> > Author:     Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > AuthorDate: Fri, 19 Jan 2018 16:33:22 +0200
> > Committer:  Ingo Molnar <mingo@kernel.org>
> > CommitDate: Tue, 13 Feb 2018 17:14:41 +0100
> 
> As of today I don't see this in linux-next
> 
> % git tag --contains 0fc8483b69862
> next-20180214
> next-20180215
> next-20180216
> 
> What happened to this change?

Hm, so it's a real mystery: I merged it, then removed it 1.5 days later without 
reporting anything. According to the Git log timestamp the removal happened late 
at night, so maybe it was a tired typo?

Anyway, it's a good thing you kept out an eye for this: I re-applied the patch for 
v4.17 and will let you know if there's any problem in testing.

Thanks,

	Ingo

Re: [tip:x86/urgent] x86/io: Define readq()/writeq() to use 64-bit type

[Ingo Molnar]

* Ingo Molnar <mingo@kernel.org> wrote:

> > As of today I don't see this in linux-next
> > 
> > % git tag --contains 0fc8483b69862
> > next-20180214
> > next-20180215
> > next-20180216
> > 
> > What happened to this change?
> 
> Hm, so it's a real mystery: I merged it, then removed it 1.5 days later without 
> reporting anything. According to the Git log timestamp the removal happened late 
> at night, so maybe it was a tired typo?

So just a few seconds after sending this I remembered why I zapped it: it was a 
Sparse failure reported by the kbuild-test robot.

Here's the report:

  [tip:x86/urgent 14/14] drivers/infiniband/hw/hns/hns_roce_hw_v1.c:1690:22: sparse: incorrect type in argument 1 (different base types)

Since you were on Cc: of that report I assumed you'd take care of it.

Thanks,

	Ingo

Re: [tip:x86/urgent] x86/io: Define readq()/writeq() to use 64-bit type

[Andy Shevchenko]

On Sat, Mar 31, 2018 at 1:22 PM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Ingo Molnar <mingo@kernel.org> wrote:
>
>> > As of today I don't see this in linux-next
>> >
>> > % git tag --contains 0fc8483b69862
>> > next-20180214
>> > next-20180215
>> > next-20180216
>> >
>> > What happened to this change?
>>
>> Hm, so it's a real mystery: I merged it, then removed it 1.5 days later without
>> reporting anything. According to the Git log timestamp the removal happened late
>> at night, so maybe it was a tired typo?
>
> So just a few seconds after sending this I remembered why I zapped it: it was a
> Sparse failure reported by the kbuild-test robot.
>
> Here's the report:
>
>   [tip:x86/urgent 14/14] drivers/infiniband/hw/hns/hns_roce_hw_v1.c:1690:22: sparse: incorrect type in argument 1 (different base types)
>
> Since you were on Cc: of that report I assumed you'd take care of it.

Ah, yes. This one I fixed.

-- 
With Best Regards,
Andy Shevchenko

Re: [tip:x86/urgent] x86/io: Define readq()/writeq() to use 64-bit type

[Andy Shevchenko]

On Sat, Mar 31, 2018 at 3:06 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Sat, Mar 31, 2018 at 1:22 PM, Ingo Molnar <mingo@kernel.org> wrote:
>> * Ingo Molnar <mingo@kernel.org> wrote:

>>   [tip:x86/urgent 14/14] drivers/infiniband/hw/hns/hns_roce_hw_v1.c:1690:22: sparse: incorrect type in argument 1 (different base types)
>>
>> Since you were on Cc: of that report I assumed you'd take care of it.
>
> Ah, yes. This one I fixed.

https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git/commit/?h=for-next&id=71591d1280e5ef02c2af2ffb9801d0c842973be9

-- 
With Best Regards,
Andy Shevchenko

Re: [tip:x86/urgent] x86/io: Define readq()/writeq() to use 64-bit type

[Ingo Molnar]

* Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Sat, Mar 31, 2018 at 3:06 PM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Sat, Mar 31, 2018 at 1:22 PM, Ingo Molnar <mingo@kernel.org> wrote:
> >> * Ingo Molnar <mingo@kernel.org> wrote:
> 
> >>   [tip:x86/urgent 14/14] drivers/infiniband/hw/hns/hns_roce_hw_v1.c:1690:22: sparse: incorrect type in argument 1 (different base types)
> >>
> >> Since you were on Cc: of that report I assumed you'd take care of it.
> >
> > Ah, yes. This one I fixed.
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git/commit/?h=for-next&id=71591d1280e5ef02c2af2ffb9801d0c842973be9

Yeah, so this fix in the RDMA tree needs to go upstream first, before we can apply 
the changes to the interface. Could you resend at around v4.17-rc1 or so?

Thanks,

	Ingo

Re: [tip:x86/urgent] x86/io: Define readq()/writeq() to use 64-bit type

[Andy Shevchenko]

On Sat, 2018-03-31 at 20:45 +0200, Ingo Molnar wrote:
> * Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> 
> > On Sat, Mar 31, 2018 at 3:06 PM, Andy Shevchenko
> > <andy.shevchenko@gmail.com> wrote:
> > > On Sat, Mar 31, 2018 at 1:22 PM, Ingo Molnar <mingo@kernel.org>
> > > wrote:
> > > > * Ingo Molnar <mingo@kernel.org> wrote:
> > > >   [tip:x86/urgent 14/14]
> > > > drivers/infiniband/hw/hns/hns_roce_hw_v1.c:1690:22: sparse:
> > > > incorrect type in argument 1 (different base types)
> > > > 
> > > > Since you were on Cc: of that report I assumed you'd take care
> > > > of it.
> > > 
> > > Ah, yes. This one I fixed.
> > 
> > https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git/commit
> > /?h=for-next&id=71591d1280e5ef02c2af2ffb9801d0c842973be9
> 
> Yeah, so this fix in the RDMA tree needs to go upstream first, before
> we can apply 
> the changes to the interface. Could you resend at around v4.17-rc1 or
> so?

Sure. Thanks!

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [tip:x86/urgent] x86/io: Define readq()/writeq() to use 64-bit type

[Andy Shevchenko]

On Sat, 2018-03-31 at 20:45 +0200, Ingo Molnar wrote:
> * Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> 
> > On Sat, Mar 31, 2018 at 3:06 PM, Andy Shevchenko
> > <andy.shevchenko@gmail.com> wrote:
> > > On Sat, Mar 31, 2018 at 1:22 PM, Ingo Molnar <mingo@kernel.org>
> > > wrote:
> > > > * Ingo Molnar <mingo@kernel.org> wrote:
> > > >   [tip:x86/urgent 14/14]
> > > > drivers/infiniband/hw/hns/hns_roce_hw_v1.c:1690:22: sparse:
> > > > incorrect type in argument 1 (different base types)
> > > > 
> > > > Since you were on Cc: of that report I assumed you'd take care
> > > > of it.
> > > 
> > > Ah, yes. This one I fixed.
> > 
> > https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git/commit
> > /?h=for-next&id=71591d1280e5ef02c2af2ffb9801d0c842973be9
> 
> Yeah, so this fix in the RDMA tree needs to go upstream first, before
> we can apply 
> the changes to the interface. Could you resend at around v4.17-rc1 or
> so?

Btw, should I recend or you can still pick it up from linux-next or
mailing list?

I can resend tomorrow if needed.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [tip:x86/urgent] x86/io: Define readq()/writeq() to use 64-bit type

[Andy Shevchenko]

On Sat, Mar 31, 2018 at 1:19 PM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
>
>> On Tue, 2018-02-13 at 08:58 -0800, tip-bot for Andy Shevchenko wrote:
>> > Commit-ID:  0fc8483b698620ea3d8cc6635b54eccc613c23a3
>> > Gitweb:     https://git.kernel.org/tip/0fc8483b698620ea3d8cc6635b54ecc
>> > c613c23a3
>> > Author:     Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> > AuthorDate: Fri, 19 Jan 2018 16:33:22 +0200
>> > Committer:  Ingo Molnar <mingo@kernel.org>
>> > CommitDate: Tue, 13 Feb 2018 17:14:41 +0100
>>
>> As of today I don't see this in linux-next
>>
>> % git tag --contains 0fc8483b69862
>> next-20180214
>> next-20180215
>> next-20180216
>>
>> What happened to this change?
>
> Hm, so it's a real mystery: I merged it, then removed it 1.5 days later without
> reporting anything. According to the Git log timestamp the removal happened late
> at night, so maybe it was a tired typo?

Who knows? )

> Anyway, it's a good thing you kept out an eye for this: I re-applied the patch for
> v4.17 and will let you know if there's any problem in testing.

Actually I got the same warning when tried to do some debug printings
with readq() as a parameter.
That's why I have noticed.

Thank you for re-applying!

-- 
With Best Regards,
Andy Shevchenko

[tip:x86/platform] x86/io: Define readq()/writeq() to use 64-bit type

[tip-bot for Andy Shevchenko]

Commit-ID:  3d94548927a96f35da57984e3410f8b53757435a
Gitweb:     https://git.kernel.org/tip/3d94548927a96f35da57984e3410f8b53757435a
Author:     Andy Shevchenko <andriy.shevchenko@linux.intel.com>
AuthorDate: Fri, 19 Jan 2018 16:33:22 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Sat, 31 Mar 2018 11:41:43 +0200

x86/io: Define readq()/writeq() to use 64-bit type

Since non atomic readq() and writeq() were added some of the drivers
would like to use it in a manner of:

 #include <io-64-nonatomic-lo-hi.h>
...
 pr_debug("Debug value of some register: %016llx\n", readq(addr));

However, lo_hi_readq() always returns __u64 data, while readq()
on x86_64 defines it as unsigned long. and thus compiler warns
about type mismatch, although they are both 64-bit on x86_64.

Convert readq() and writeq() on x86 to operate on deterministic
64-bit type. The most of architectures in the kernel already are using
either unsigned long long, or u64 type for readq() / writeq().
This change propagates consistency in that sense.

While this is not an issue per se, though if someone wants to address it,
the anchor could be the commit

  797a796a13df ("asm-generic: architecture independent readq/writeq for 32bit environment")

where non-atomic variants had been introduced.

Note, there are only few users of above pattern and they will not be
affected because they do cast returned value. The actual warning has
been issued on not-yet-upstreamed code.

Potentially we might get a new warnings if some 64-bit only code
assigns returned value to unsigned long type of variable. This is
assumed to be addressed on case-by-case basis.

Reported-by: lkp <lkp@intel.com>
Tested-by: Sohil Mehta <sohil.mehta@intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Hitoshi Mitake <mitake@dcl.info.waseda.ac.jp>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mehta, Sohil <sohil.mehta@intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/20180119143322.16555-1-andriy.shevchenko@linux.intel.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/include/asm/io.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
index 95e948627fd0..365f5ba9222b 100644
--- a/arch/x86/include/asm/io.h
+++ b/arch/x86/include/asm/io.h
@@ -94,10 +94,10 @@ build_mmio_write(__writel, "l", unsigned int, "r", )
 
 #ifdef CONFIG_X86_64
 
-build_mmio_read(readq, "q", unsigned long, "=r", :"memory")
-build_mmio_read(__readq, "q", unsigned long, "=r", )
-build_mmio_write(writeq, "q", unsigned long, "r", :"memory")
-build_mmio_write(__writeq, "q", unsigned long, "r", )
+build_mmio_read(readq, "q", unsigned long long, "=r", :"memory")
+build_mmio_read(__readq, "q", unsigned long long, "=r", )
+build_mmio_write(writeq, "q", unsigned long long, "r", :"memory")
+build_mmio_write(__writeq, "q", unsigned long long, "r", )
 
 #define readq_relaxed(a)	__readq(a)
 #define writeq_relaxed(v, a)	__writeq(v, a)