bitops: use the UL literal for constants

Submitted by Andrey Vagin on June 17, 2018, 3:44 a.m.

Details

Message ID 20180617034442.6030-1-avagin@virtuozzo.com
State Accepted
Series "bitops: use the UL literal for constants"
Headers show

Commit Message

Andrey Vagin June 17, 2018, 3:44 a.m.
We operate by long variables in out bit arithmetics, so our constants
should be marked as long too.

Cc: Adrian Reber <areber@redhat.com>
Reported-by: Adrian Reber <areber@redhat.com>
Signed-off-by: Andrei Vagin <avagin@virtuozzo.com>
---
 include/common/asm-generic/bitops.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Patch hide | download patch | download mbox

diff --git a/include/common/asm-generic/bitops.h b/include/common/asm-generic/bitops.h
index e1a097511..0d861bdcc 100644
--- a/include/common/asm-generic/bitops.h
+++ b/include/common/asm-generic/bitops.h
@@ -28,25 +28,25 @@ 
 
 static inline void set_bit(int nr, volatile unsigned long *addr) {
 	addr += nr / BITS_PER_LONG;
-	*addr |= (1 << (nr % BITS_PER_LONG));
+	*addr |= (1UL << (nr % BITS_PER_LONG));
 }
 
 static inline void change_bit(int nr, volatile unsigned long *addr)
 {
 	addr += nr / BITS_PER_LONG;
-	*addr ^= (1 << (nr % BITS_PER_LONG));
+	*addr ^= (1UL << (nr % BITS_PER_LONG));
 }
 
 static inline int test_bit(int nr, volatile const unsigned long *addr)
 {
 	addr += nr / BITS_PER_LONG;
-	return (*addr & (1 << (nr % BITS_PER_LONG))) ? -1 : 0;
+	return (*addr & (1UL << (nr % BITS_PER_LONG))) ? -1 : 0;
 }
 
 static inline void clear_bit(int nr, volatile unsigned long *addr)
 {
 	addr += nr / BITS_PER_LONG;
-	*addr &= ~(1 << (nr % BITS_PER_LONG));
+	*addr &= ~(1UL << (nr % BITS_PER_LONG));
 }
 
 /**

Comments

Adrian Reber June 18, 2018, 6:50 a.m.
Hello Andrei,

I just tested this again and it indeed fixes the cow01 test case. For
aarch64 I am running criu 3.9 with the aarch64 patches on top of it. For
initial testing of this patch I applied the patch on a clean 3.9
checkout without the aarch64 patches so it failed.

Now I first applied the aarch64 patches on 3.9 and then this patch and
now everything seems to work correctly on aarch64.

Thanks for the patch and sorry for the confusion.

		Adrian

Tested-by: Adrian Reber <areber@redhat.com>

On Sun, Jun 17, 2018 at 06:44:42AM +0300, Andrei Vagin wrote:
> We operate by long variables in out bit arithmetics, so our constants
> should be marked as long too.
> 
> Cc: Adrian Reber <areber@redhat.com>
> Reported-by: Adrian Reber <areber@redhat.com>
> Signed-off-by: Andrei Vagin <avagin@virtuozzo.com>
> ---
>  include/common/asm-generic/bitops.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/include/common/asm-generic/bitops.h b/include/common/asm-generic/bitops.h
> index e1a097511..0d861bdcc 100644
> --- a/include/common/asm-generic/bitops.h
> +++ b/include/common/asm-generic/bitops.h
> @@ -28,25 +28,25 @@
>  
>  static inline void set_bit(int nr, volatile unsigned long *addr) {
>  	addr += nr / BITS_PER_LONG;
> -	*addr |= (1 << (nr % BITS_PER_LONG));
> +	*addr |= (1UL << (nr % BITS_PER_LONG));
>  }
>  
>  static inline void change_bit(int nr, volatile unsigned long *addr)
>  {
>  	addr += nr / BITS_PER_LONG;
> -	*addr ^= (1 << (nr % BITS_PER_LONG));
> +	*addr ^= (1UL << (nr % BITS_PER_LONG));
>  }
>  
>  static inline int test_bit(int nr, volatile const unsigned long *addr)
>  {
>  	addr += nr / BITS_PER_LONG;
> -	return (*addr & (1 << (nr % BITS_PER_LONG))) ? -1 : 0;
> +	return (*addr & (1UL << (nr % BITS_PER_LONG))) ? -1 : 0;
>  }
>  
>  static inline void clear_bit(int nr, volatile unsigned long *addr)
>  {
>  	addr += nr / BITS_PER_LONG;
> -	*addr &= ~(1 << (nr % BITS_PER_LONG));
> +	*addr &= ~(1UL << (nr % BITS_PER_LONG));
>  }
>  
>  /**
> -- 
> 2.14.3
>
Dmitry Safonov June 18, 2018, 2:50 p.m.
2018-06-18 7:50 GMT+01:00 Adrian Reber <areber@redhat.com>:
> Hello Andrei,
>
> I just tested this again and it indeed fixes the cow01 test case. For
> aarch64 I am running criu 3.9 with the aarch64 patches on top of it. For
> initial testing of this patch I applied the patch on a clean 3.9
> checkout without the aarch64 patches so it failed.
>
> Now I first applied the aarch64 patches on 3.9 and then this patch and
> now everything seems to work correctly on aarch64.
>
> Thanks for the patch and sorry for the confusion.
>
>                 Adrian
>
> Tested-by: Adrian Reber <areber@redhat.com>
>
> On Sun, Jun 17, 2018 at 06:44:42AM +0300, Andrei Vagin wrote:
>> We operate by long variables in out bit arithmetics, so our constants
>> should be marked as long too.
>>
>> Cc: Adrian Reber <areber@redhat.com>
>> Reported-by: Adrian Reber <areber@redhat.com>
>> Signed-off-by: Andrei Vagin <avagin@virtuozzo.com>

LGTM,
Reviewed-by: Dmitry Safonov <0x7f454c46@gmail.com>

Thanks,
             Dmitry
Andrey Vagin June 18, 2018, 5:55 p.m.
On Mon, Jun 18, 2018 at 08:50:17AM +0200, Adrian Reber wrote:
> Hello Andrei,
> 
> I just tested this again and it indeed fixes the cow01 test case. For
> aarch64 I am running criu 3.9 with the aarch64 patches on top of it. For
> initial testing of this patch I applied the patch on a clean 3.9
> checkout without the aarch64 patches so it failed.
> 
> Now I first applied the aarch64 patches on 3.9 and then this patch and
> now everything seems to work correctly on aarch64.
> 
> Thanks for the patch and sorry for the confusion.

Adrian, thank you for driving this work of supporting aarch64.

Could you check that "./zdtm.py run -a --pre 2" passes without fails
too?
> 
> 		Adrian
> 
> Tested-by: Adrian Reber <areber@redhat.com>
> 
> On Sun, Jun 17, 2018 at 06:44:42AM +0300, Andrei Vagin wrote:
> > We operate by long variables in out bit arithmetics, so our constants
> > should be marked as long too.
> > 
> > Cc: Adrian Reber <areber@redhat.com>
> > Reported-by: Adrian Reber <areber@redhat.com>
> > Signed-off-by: Andrei Vagin <avagin@virtuozzo.com>
> > ---
> >  include/common/asm-generic/bitops.h | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/include/common/asm-generic/bitops.h b/include/common/asm-generic/bitops.h
> > index e1a097511..0d861bdcc 100644
> > --- a/include/common/asm-generic/bitops.h
> > +++ b/include/common/asm-generic/bitops.h
> > @@ -28,25 +28,25 @@
> >  
> >  static inline void set_bit(int nr, volatile unsigned long *addr) {
> >  	addr += nr / BITS_PER_LONG;
> > -	*addr |= (1 << (nr % BITS_PER_LONG));
> > +	*addr |= (1UL << (nr % BITS_PER_LONG));
> >  }
> >  
> >  static inline void change_bit(int nr, volatile unsigned long *addr)
> >  {
> >  	addr += nr / BITS_PER_LONG;
> > -	*addr ^= (1 << (nr % BITS_PER_LONG));
> > +	*addr ^= (1UL << (nr % BITS_PER_LONG));
> >  }
> >  
> >  static inline int test_bit(int nr, volatile const unsigned long *addr)
> >  {
> >  	addr += nr / BITS_PER_LONG;
> > -	return (*addr & (1 << (nr % BITS_PER_LONG))) ? -1 : 0;
> > +	return (*addr & (1UL << (nr % BITS_PER_LONG))) ? -1 : 0;
> >  }
> >  
> >  static inline void clear_bit(int nr, volatile unsigned long *addr)
> >  {
> >  	addr += nr / BITS_PER_LONG;
> > -	*addr &= ~(1 << (nr % BITS_PER_LONG));
> > +	*addr &= ~(1UL << (nr % BITS_PER_LONG));
> >  }
> >  
> >  /**
> > -- 
> > 2.14.3
> >
Andrey Vagin June 18, 2018, 6 p.m.
Applied

On Sun, Jun 17, 2018 at 06:44:42AM +0300, Andrei Vagin wrote:
> We operate by long variables in out bit arithmetics, so our constants
> should be marked as long too.
> 
> Cc: Adrian Reber <areber@redhat.com>
> Reported-by: Adrian Reber <areber@redhat.com>
> Signed-off-by: Andrei Vagin <avagin@virtuozzo.com>
> ---
>  include/common/asm-generic/bitops.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/include/common/asm-generic/bitops.h b/include/common/asm-generic/bitops.h
> index e1a097511..0d861bdcc 100644
> --- a/include/common/asm-generic/bitops.h
> +++ b/include/common/asm-generic/bitops.h
> @@ -28,25 +28,25 @@
>  
>  static inline void set_bit(int nr, volatile unsigned long *addr) {
>  	addr += nr / BITS_PER_LONG;
> -	*addr |= (1 << (nr % BITS_PER_LONG));
> +	*addr |= (1UL << (nr % BITS_PER_LONG));
>  }
>  
>  static inline void change_bit(int nr, volatile unsigned long *addr)
>  {
>  	addr += nr / BITS_PER_LONG;
> -	*addr ^= (1 << (nr % BITS_PER_LONG));
> +	*addr ^= (1UL << (nr % BITS_PER_LONG));
>  }
>  
>  static inline int test_bit(int nr, volatile const unsigned long *addr)
>  {
>  	addr += nr / BITS_PER_LONG;
> -	return (*addr & (1 << (nr % BITS_PER_LONG))) ? -1 : 0;
> +	return (*addr & (1UL << (nr % BITS_PER_LONG))) ? -1 : 0;
>  }
>  
>  static inline void clear_bit(int nr, volatile unsigned long *addr)
>  {
>  	addr += nr / BITS_PER_LONG;
> -	*addr &= ~(1 << (nr % BITS_PER_LONG));
> +	*addr &= ~(1UL << (nr % BITS_PER_LONG));
>  }
>  
>  /**
> -- 
> 2.14.3
>
Adrian Reber June 18, 2018, 6:10 p.m.
On Mon, Jun 18, 2018 at 10:55:03AM -0700, Andrei Vagin wrote:
> > I just tested this again and it indeed fixes the cow01 test case. For
> > aarch64 I am running criu 3.9 with the aarch64 patches on top of it. For
> > initial testing of this patch I applied the patch on a clean 3.9
> > checkout without the aarch64 patches so it failed.
> > 
> > Now I first applied the aarch64 patches on 3.9 and then this patch and
> > now everything seems to work correctly on aarch64.
> > 
> > Thanks for the patch and sorry for the confusion.
> 
> Adrian, thank you for driving this work of supporting aarch64.
> 
> Could you check that "./zdtm.py run -a --pre 2" passes without fails
> too?

aarch64 does not support dirty memory tracking in the kernel, so I get:

# ./zdtm.py run -a --pre 2
Tracking memory is not available

# git grep "select HAVE_ARCH_SOFT_DIRTY"
arch/powerpc/Kconfig:   select HAVE_ARCH_SOFT_DIRTY if PPC_BOOK3S_64
arch/powerpc/Kconfig:   select HAVE_ARCH_SOFT_DIRTY if PPC_BOOK3S_64
arch/s390/Kconfig:      select HAVE_ARCH_SOFT_DIRTY
arch/x86/Kconfig:       select HAVE_ARCH_SOFT_DIRTY

Only on powerpc, s390 and x86 is it implemented in the kernel.

		Adrian
Andrey Vagin June 18, 2018, 6:11 p.m.
On Mon, Jun 18, 2018 at 08:10:07PM +0200, Adrian Reber wrote:
> On Mon, Jun 18, 2018 at 10:55:03AM -0700, Andrei Vagin wrote:
> > > I just tested this again and it indeed fixes the cow01 test case. For
> > > aarch64 I am running criu 3.9 with the aarch64 patches on top of it. For
> > > initial testing of this patch I applied the patch on a clean 3.9
> > > checkout without the aarch64 patches so it failed.
> > > 
> > > Now I first applied the aarch64 patches on 3.9 and then this patch and
> > > now everything seems to work correctly on aarch64.
> > > 
> > > Thanks for the patch and sorry for the confusion.
> > 
> > Adrian, thank you for driving this work of supporting aarch64.
> > 
> > Could you check that "./zdtm.py run -a --pre 2" passes without fails
> > too?
> 
> aarch64 does not support dirty memory tracking in the kernel, so I get:
> 
> # ./zdtm.py run -a --pre 2
> Tracking memory is not available
> 
> # git grep "select HAVE_ARCH_SOFT_DIRTY"
> arch/powerpc/Kconfig:   select HAVE_ARCH_SOFT_DIRTY if PPC_BOOK3S_64
> arch/powerpc/Kconfig:   select HAVE_ARCH_SOFT_DIRTY if PPC_BOOK3S_64
> arch/s390/Kconfig:      select HAVE_ARCH_SOFT_DIRTY
> arch/x86/Kconfig:       select HAVE_ARCH_SOFT_DIRTY
> 
> Only on powerpc, s390 and x86 is it implemented in the kernel.

Ok, I see. Thanks!

> 
> 		Adrian