[PATCH] x86: Extend test_and_set_bit() test_and_clean_bit() to 64 bits in X86_64

[PATCH] x86: Extend test_and_set_bit() test_and_clean_bit() to 64 bits in X86_64

[Sheng Yang]

This fix 44/45 bit width memory can't boot up issue. The reason is
free_bootmem_node()->mark_bootmem_node()->__free() use test_and_clean_bit() to
clean node_bootmem_map, but for 44bits width address, the idx set bit 31 (43 -
12), which consider as a nagetive value for bts.

This patch applied to tip/mm.

Signed-off-by: Sheng Yang <sheng@linux.intel.com>
---
 arch/x86/include/asm/bitops.h |   24 +++++++++++++++---------
 1 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/arch/x86/include/asm/bitops.h b/arch/x86/include/asm/bitops.h
index 02b47a6..400dd28 100644
--- a/arch/x86/include/asm/bitops.h
+++ b/arch/x86/include/asm/bitops.h
@@ -41,6 +41,12 @@
 #define CONST_MASK_ADDR(nr, addr)	BITOP_ADDR((void *)(addr) + ((nr)>>3))
 #define CONST_MASK(nr)			(1 << ((nr) & 7))
 
+#ifdef CONFIG_X86_64
+#define REX_X86 "rex "
+#else
+#define REX_X86
+#endif
+
 /**
  * set_bit - Atomically set a bit in memory
  * @nr: the bit to set
@@ -192,11 +198,11 @@ static inline void change_bit(int nr, volatile unsigned long *addr)
  * This operation is atomic and cannot be reordered.
  * It also implies a memory barrier.
  */
-static inline int test_and_set_bit(int nr, volatile unsigned long *addr)
+static inline int test_and_set_bit(long int nr, volatile unsigned long *addr)
 {
 	int oldbit;
 
-	asm volatile(LOCK_PREFIX "bts %2,%1\n\t"
+	asm volatile(LOCK_PREFIX REX_X86 "bts %2,%1\n\t"
 		     "sbb %0,%0" : "=r" (oldbit), ADDR : "Ir" (nr) : "memory");
 
 	return oldbit;
@@ -224,11 +230,11 @@ test_and_set_bit_lock(int nr, volatile unsigned long *addr)
  * If two examples of this operation race, one can appear to succeed
  * but actually fail.  You must protect multiple accesses with a lock.
  */
-static inline int __test_and_set_bit(int nr, volatile unsigned long *addr)
+static inline int __test_and_set_bit(long int nr, volatile unsigned long *addr)
 {
 	int oldbit;
 
-	asm("bts %2,%1\n\t"
+	asm(REX_X86 "bts %2,%1\n\t"
 	    "sbb %0,%0"
 	    : "=r" (oldbit), ADDR
 	    : "Ir" (nr));
@@ -243,14 +249,13 @@ static inline int __test_and_set_bit(int nr, volatile unsigned long *addr)
  * This operation is atomic and cannot be reordered.
  * It also implies a memory barrier.
  */
-static inline int test_and_clear_bit(int nr, volatile unsigned long *addr)
+static inline int test_and_clear_bit(long int nr, volatile unsigned long *addr)
 {
 	int oldbit;
 
-	asm volatile(LOCK_PREFIX "btr %2,%1\n\t"
+	asm volatile(LOCK_PREFIX REX_X86 "btr %2,%1\n\t"
 		     "sbb %0,%0"
 		     : "=r" (oldbit), ADDR : "Ir" (nr) : "memory");
-
 	return oldbit;
 }
 
@@ -263,11 +268,12 @@ static inline int test_and_clear_bit(int nr, volatile unsigned long *addr)
  * If two examples of this operation race, one can appear to succeed
  * but actually fail.  You must protect multiple accesses with a lock.
  */
-static inline int __test_and_clear_bit(int nr, volatile unsigned long *addr)
+static inline int __test_and_clear_bit(long int nr,
+				       volatile unsigned long *addr)
 {
 	int oldbit;
 
-	asm volatile("btr %2,%1\n\t"
+	asm volatile(REX_X86 "btr %2,%1\n\t"
 		     "sbb %0,%0"
 		     : "=r" (oldbit), ADDR
 		     : "Ir" (nr));
-- 
1.5.4.5

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] x86: Extend test_and_set_bit() test_and_clean_bit() to 64 bits in X86_64

[Andi Kleen]

Sheng Yang <sheng@linux.intel.com> writes:

> -static inline int test_and_set_bit(int nr, volatile unsigned long *addr)
> +static inline int test_and_set_bit(long int nr, volatile unsigned long *addr)
>  {
>  	int oldbit;
>  
> -	asm volatile(LOCK_PREFIX "bts %2,%1\n\t"
> +	asm volatile(LOCK_PREFIX REX_X86 "bts %2,%1\n\t"

Use btsq on 64bit, then you don't need the explicit rex prefix.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] x86: Extend test_and_set_bit() test_and_clean_bit() to 64 bits in X86_64

[Sheng Yang]

On Wednesday 13 May 2009 16:38:29 Andi Kleen wrote:
> Sheng Yang <sheng@linux.intel.com> writes:
> > -static inline int test_and_set_bit(int nr, volatile unsigned long *addr)
> > +static inline int test_and_set_bit(long int nr, volatile unsigned long
> > *addr) {
> >  	int oldbit;
> >
> > -	asm volatile(LOCK_PREFIX "bts %2,%1\n\t"
> > +	asm volatile(LOCK_PREFIX REX_X86 "bts %2,%1\n\t"
>
> Use btsq on 64bit, then you don't need the explicit rex prefix.

Hi Andi

Well, I just think lots of "#ifdef/#else" is a little annoying here, then use 
REX...

-- 
regards
Yang, Sheng

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] x86: Extend test_and_set_bit() test_and_clean_bit() to 64 bits in X86_64

[Andi Kleen]

> Well, I just think lots of "#ifdef/#else" is a little annoying here, then use 
> REX...

Better add a 'q' string concatination then. The problem with rex is that most 
people can't read it even if they know assembler --  they don't know
all the details of instruction encoding.

-Andi
-- 
ak@linux.intel.com -- Speaking for myself only.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] x86: Extend test_and_set_bit() test_and_clean_bit() to 64 bits in X86_64

[H. Peter Anvin]

Andi Kleen wrote:
>> Well, I just think lots of "#ifdef/#else" is a little annoying here, then use 
>> REX...
> 
> Better add a 'q' string concatination then. The problem with rex is that most 
> people can't read it even if they know assembler --  they don't know
> all the details of instruction encoding.
> 

The right way to do it is to pass the proper type of register.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] x86: Extend test_and_set_bit() test_and_clean_bit() to 64 bits in X86_64

[Andi Kleen]

On Thu, May 14, 2009 at 07:09:47AM -0700, H. Peter Anvin wrote:
> Andi Kleen wrote:
> >> Well, I just think lots of "#ifdef/#else" is a little annoying here, then use 
> >> REX...
> > 
> > Better add a 'q' string concatination then. The problem with rex is that most 
> > people can't read it even if they know assembler --  they don't know
> > all the details of instruction encoding.
> > 
> 
> The right way to do it is to pass the proper type of register.

For the input index register you don't actually need 64bit and for the
value it's typically memory anyways.


-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] x86: Extend test_and_set_bit() test_and_clean_bit() to 64 bits in X86_64

[H. Peter Anvin]

Andi Kleen wrote:
>>>
>> The right way to do it is to pass the proper type of register.
> 
> For the input index register you don't actually need 64bit and for the
> value it's typically memory anyways.
> 

If you have a 64-bit operation you have a 64-bit index register.  And
you need a 64-bit index for it to handle over 2^31 (since it is signed.)

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] x86: Extend test_and_set_bit() test_and_clean_bit() to 64 bits in X86_64

[Andi Kleen]

On Thu, May 14, 2009 at 07:16:10AM -0700, H. Peter Anvin wrote:
> Andi Kleen wrote:
> >>>
> >> The right way to do it is to pass the proper type of register.
> > 
> > For the input index register you don't actually need 64bit and for the
> > value it's typically memory anyways.
> > 
> 
> If you have a 64-bit operation you have a 64-bit index register.  And
> you need a 64-bit index for it to handle over 2^31 (since it is signed.)

Pretty much all the bit ops and a few other operations currently have
2/4GB limits on x86-64. I don't think that's going to change.

In the kernel nothing is ever that big continuously anyways.

-Andi
-- 
ak@linux.intel.com -- Speaking for myself only.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] x86: Extend test_and_set_bit() test_and_clean_bit() to 64 bits in X86_64

[H. Peter Anvin]

Andi Kleen wrote:
> 
> Pretty much all the bit ops and a few other operations currently have
> 2/4GB limits on x86-64. I don't think that's going to change.
> 
> In the kernel nothing is ever that big continuously anyways.
> 

Uhm, that *is* the problem at hand... specifically the bootmem map on
multi-terabyte systems.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] x86: Extend test_and_set_bit() test_and_clean_bit() to 64 bits in X86_64

[Andi Kleen]

On Thu, May 14, 2009 at 07:25:22AM -0700, H. Peter Anvin wrote:
> Andi Kleen wrote:
> > 
> > Pretty much all the bit ops and a few other operations currently have
> > 2/4GB limits on x86-64. I don't think that's going to change.
> > 
> > In the kernel nothing is ever that big continuously anyways.
> > 
> 
> Uhm, that *is* the problem at hand... specifically the bootmem map on
> multi-terabyte systems.

Well they have to fix a lot of more stuff then, when I did 
all the inline assembler >2GB objects were a explicit non goal. 
It also wouldn't surprise me if that wasn't true on other architectures too.

It would be better to just use open coded C for that case and avoid inline 
assembler.

-Andi
-- 
ak@linux.intel.com -- Speaking for myself only.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] x86: Extend test_and_set_bit() test_and_clean_bit() to 64 bits in X86_64

[H. Peter Anvin]

Andi Kleen wrote:
> 
> Well they have to fix a lot of more stuff then, when I did 
> all the inline assembler >2GB objects were a explicit non goal. 
> It also wouldn't surprise me if that wasn't true on other architectures too.

512 MB, fwiw...

> It would be better to just use open coded C for that case and avoid inline 
> assembler.

It's not like the extra REX prefix is going to matter significantly for
any application, and given how trivial it is it doesn't seem like a big
deal at all.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] x86: Extend test_and_set_bit() test_and_clean_bit() to 64 bits in X86_64

[H. Peter Anvin]

[-- Attachment #1: Type: text/plain, Size: 504 bytes --]

Sheng Yang wrote:
> This fix 44/45 bit width memory can't boot up issue. The reason is
> free_bootmem_node()->mark_bootmem_node()->__free() use test_and_clean_bit() to
> clean node_bootmem_map, but for 44bits width address, the idx set bit 31 (43 -
> 12), which consider as a nagetive value for bts.
> 
> This patch applied to tip/mm.

Hi Sheng,

Could you try the attached patch instead?

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.


[-- Attachment #2: diff --]
[-- Type: text/plain, Size: 3886 bytes --]

diff --git a/Documentation/x86/boot.txt b/Documentation/x86/boot.txt
index e020366..ccc1bd4 100644
--- a/Documentation/x86/boot.txt
+++ b/Documentation/x86/boot.txt
@@ -50,6 +50,11 @@ Protocol 2.08:	(Kernel 2.6.26) Added crc32 checksum and ELF format
 Protocol 2.09:	(Kernel 2.6.26) Added a field of 64-bit physical
 		pointer to single linked list of struct	setup_data.
 
+Protocol 2.10:	(Kernel 2.6.31?) A protocol for relaxed alignment
+	 	beyond the kernel_alignment added, new init_size and
+	 	pref_address fields.
+	 	
+
 **** MEMORY LAYOUT
 
 The traditional memory map for the kernel loader, used for Image or
@@ -173,7 +178,7 @@ Offset	Proto	Name		Meaning
 022C/4	2.03+	ramdisk_max	Highest legal initrd address
 0230/4	2.05+	kernel_alignment Physical addr alignment required for kernel
 0234/1	2.05+	relocatable_kernel Whether kernel is relocatable or not
-0235/1	N/A	pad2		Unused
+0235/1	2.10+	min_alignment	Minimum alignment, as a power of 2
 0236/2	N/A	pad3		Unused
 0238/4	2.06+	cmdline_size	Maximum size of the kernel command line
 023C/4	2.07+	hardware_subarch Hardware subarchitecture
@@ -182,6 +187,8 @@ Offset	Proto	Name		Meaning
 024C/4	2.08+	payload_length	Length of kernel payload
 0250/8	2.09+	setup_data	64-bit physical pointer to linked list
 				of struct setup_data
+0258/8	2.10+	pref_address	Preferred loading address
+0260/4	2.10+	init_size	Linear memory required during initialization
 
 (1) For backwards compatibility, if the setup_sects field contains 0, the
     real value is 4.
@@ -482,11 +489,15 @@ Protocol:	2.03+
   0x37FFFFFF, you can start your ramdisk at 0x37FE0000.)
 
 Field name:	kernel_alignment
-Type:		read (reloc)
+Type:		read/modify (reloc)
 Offset/size:	0x230/4
-Protocol:	2.05+
+Protocol:	2.05+ (read), 2.10+ (modify)
 
-  Alignment unit required by the kernel (if relocatable_kernel is true.)
+  Alignment unit required by the kernel (if relocatable_kernel is
+  true.)  Starting with protocol version 2.10, this reflects the
+  kernel alignment preferred for optimal performance and can be
+  modified by the loader; see the min_alignment and pref_address field
+  below.
 
 Field name:	relocatable_kernel
 Type:		read (reloc)
@@ -498,6 +509,22 @@ Protocol:	2.05+
   After loading, the boot loader must set the code32_start field to
   point to the loaded code, or to a boot loader hook.
 
+Field name:	min_alignment
+Type:		read (reloc)
+Offset/size:	0x235/1
+Protocol:	2.10+
+
+  This field, if nonzero, indicates as a power of 2 the minimum
+  alignment required, as opposed to preferred, by the kernel to boot.
+  If a boot loader makes use of this field, it should update the
+  kernel_alignment field with the alignment unit desired; typically:
+
+	kernel_alignment = 1 << min_alignment
+
+  There may be a considerable performance cost with an excessively
+  misaligned kernel.  Therefore, a loader should typically try each
+  power-of-two alignment from kernel_alignment down to this alignment.
+
 Field name:	cmdline_size
 Type:		read
 Offset/size:	0x238/4
@@ -582,6 +609,27 @@ Protocol:	2.09+
   sure to consider the case where the linked list already contains
   entries.
 
+Field name:	pref_address
+Type:		read (reloc)
+Offset/size:	0x258/8
+Protocol:	2.10+
+
+  This field, if nonzero, represents a preferred load address for the
+  kernel.  A relocating bootloader should attempt to load at this
+  address if possible.
+
+
+Field name:	init_size
+Type:		read
+Offset/size:	0x25c/4
+
+  This field indicates the amount of linear contiguous memory starting
+  at the kernel load address (rounded up to kernel_alignment) that the
+  kernel needs before it is capable of examining its memory map.  This
+  is not the same thing as the total amount of memory the kernel needs
+  to boot, but it can be used by a relocating boot loader to help
+  select a safe load address for the kernel.
+
 
 **** THE IMAGE CHECKSUM
 

Re: [PATCH] x86: Extend test_and_set_bit() test_and_clean_bit() to 64 bits in X86_64

[H. Peter Anvin]

H. Peter Anvin wrote:
> Sheng Yang wrote:
>> This fix 44/45 bit width memory can't boot up issue. The reason is
>> free_bootmem_node()->mark_bootmem_node()->__free() use test_and_clean_bit() to
>> clean node_bootmem_map, but for 44bits width address, the idx set bit 31 (43 -
>> 12), which consider as a nagetive value for bts.
>>
>> This patch applied to tip/mm.
> 
> Hi Sheng,
> 
> Could you try the attached patch instead?
> 
> 	-hpa
> 

Sorry, wrong patch entirely... here is the right one.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] x86: Extend test_and_set_bit() test_and_clean_bit() to 64 bits in X86_64

[H. Peter Anvin]

[-- Attachment #1: Type: text/plain, Size: 702 bytes --]

H. Peter Anvin wrote:
> H. Peter Anvin wrote:
>> Sheng Yang wrote:
>>> This fix 44/45 bit width memory can't boot up issue. The reason is
>>> free_bootmem_node()->mark_bootmem_node()->__free() use test_and_clean_bit() to
>>> clean node_bootmem_map, but for 44bits width address, the idx set bit 31 (43 -
>>> 12), which consider as a nagetive value for bts.
>>>
>>> This patch applied to tip/mm.
>> Hi Sheng,
>>
>> Could you try the attached patch instead?
>>
> 
> Sorry, wrong patch entirely... here is the right one.
> 

This time, for real?  Sheesh.  I'm having a morning, apparently.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.


[-- Attachment #2: diff --]
[-- Type: text/plain, Size: 9028 bytes --]

diff --git a/arch/x86/include/asm/bitops.h b/arch/x86/include/asm/bitops.h
index 02b47a6..56fd9cc 100644
--- a/arch/x86/include/asm/bitops.h
+++ b/arch/x86/include/asm/bitops.h
@@ -41,6 +41,16 @@
 #define CONST_MASK_ADDR(nr, addr)	BITOP_ADDR((void *)(addr) + ((nr)>>3))
 #define CONST_MASK(nr)			(1 << ((nr) & 7))
 
+/*
+ * How to treat the bitops index for bitops instructions.  Casting this
+ * to unsigned long correctly generates 64-bit operations on 64 bits.
+ */
+#ifdef CONFIG_X86_64
+#define IDX(nr) "Jr" (nr)
+#else
+#define IDX(nr) "Ir" (nr)
+#endif
+
 /**
  * set_bit - Atomically set a bit in memory
  * @nr: the bit to set
@@ -57,7 +67,7 @@
  * restricted to acting on a single-word quantity.
  */
 static __always_inline void
-set_bit(unsigned int nr, volatile unsigned long *addr)
+set_bit(unsigned long nr, volatile unsigned long *addr)
 {
 	if (IS_IMMEDIATE(nr)) {
 		asm volatile(LOCK_PREFIX "orb %1,%0"
@@ -66,7 +76,7 @@ set_bit(unsigned int nr, volatile unsigned long *addr)
 			: "memory");
 	} else {
 		asm volatile(LOCK_PREFIX "bts %1,%0"
-			: BITOP_ADDR(addr) : "Ir" (nr) : "memory");
+			: BITOP_ADDR(addr) : IDX(nr) : "memory");
 	}
 }
 
@@ -79,9 +89,9 @@ set_bit(unsigned int nr, volatile unsigned long *addr)
  * If it's called on the same region of memory simultaneously, the effect
  * may be that only one operation succeeds.
  */
-static inline void __set_bit(int nr, volatile unsigned long *addr)
+static inline void __set_bit(unsigned long nr, volatile unsigned long *addr)
 {
-	asm volatile("bts %1,%0" : ADDR : "Ir" (nr) : "memory");
+	asm volatile("bts %1,%0" : ADDR : IDX(nr) : "memory");
 }
 
 /**
@@ -95,7 +105,7 @@ static inline void __set_bit(int nr, volatile unsigned long *addr)
  * in order to ensure changes are visible on other processors.
  */
 static __always_inline void
-clear_bit(int nr, volatile unsigned long *addr)
+clear_bit(unsigned long nr, volatile unsigned long *addr)
 {
 	if (IS_IMMEDIATE(nr)) {
 		asm volatile(LOCK_PREFIX "andb %1,%0"
@@ -104,7 +114,7 @@ clear_bit(int nr, volatile unsigned long *addr)
 	} else {
 		asm volatile(LOCK_PREFIX "btr %1,%0"
 			: BITOP_ADDR(addr)
-			: "Ir" (nr));
+			: IDX(nr));
 	}
 }
 
@@ -116,15 +126,15 @@ clear_bit(int nr, volatile unsigned long *addr)
  * clear_bit() is atomic and implies release semantics before the memory
  * operation. It can be used for an unlock.
  */
-static inline void clear_bit_unlock(unsigned nr, volatile unsigned long *addr)
+static inline void clear_bit_unlock(unsigned long nr, volatile unsigned long *addr)
 {
 	barrier();
 	clear_bit(nr, addr);
 }
 
-static inline void __clear_bit(int nr, volatile unsigned long *addr)
+static inline void __clear_bit(unsigned long nr, volatile unsigned long *addr)
 {
-	asm volatile("btr %1,%0" : ADDR : "Ir" (nr));
+	asm volatile("btr %1,%0" : ADDR : IDX(nr));
 }
 
 /*
@@ -139,7 +149,7 @@ static inline void __clear_bit(int nr, volatile unsigned long *addr)
  * No memory barrier is required here, because x86 cannot reorder stores past
  * older loads. Same principle as spin_unlock.
  */
-static inline void __clear_bit_unlock(unsigned nr, volatile unsigned long *addr)
+static inline void __clear_bit_unlock(unsigned long nr, volatile unsigned long *addr)
 {
 	barrier();
 	__clear_bit(nr, addr);
@@ -157,9 +167,9 @@ static inline void __clear_bit_unlock(unsigned nr, volatile unsigned long *addr)
  * If it's called on the same region of memory simultaneously, the effect
  * may be that only one operation succeeds.
  */
-static inline void __change_bit(int nr, volatile unsigned long *addr)
+static inline void __change_bit(unsigned long nr, volatile unsigned long *addr)
 {
-	asm volatile("btc %1,%0" : ADDR : "Ir" (nr));
+	asm volatile("btc %1,%0" : ADDR : IDX(nr));
 }
 
 /**
@@ -171,7 +181,7 @@ static inline void __change_bit(int nr, volatile unsigned long *addr)
  * Note that @nr may be almost arbitrarily large; this function is not
  * restricted to acting on a single-word quantity.
  */
-static inline void change_bit(int nr, volatile unsigned long *addr)
+static inline void change_bit(unsigned long nr, volatile unsigned long *addr)
 {
 	if (IS_IMMEDIATE(nr)) {
 		asm volatile(LOCK_PREFIX "xorb %1,%0"
@@ -180,7 +190,7 @@ static inline void change_bit(int nr, volatile unsigned long *addr)
 	} else {
 		asm volatile(LOCK_PREFIX "btc %1,%0"
 			: BITOP_ADDR(addr)
-			: "Ir" (nr));
+			: IDX(nr));
 	}
 }
 
@@ -192,12 +202,12 @@ static inline void change_bit(int nr, volatile unsigned long *addr)
  * This operation is atomic and cannot be reordered.
  * It also implies a memory barrier.
  */
-static inline int test_and_set_bit(int nr, volatile unsigned long *addr)
+static inline int test_and_set_bit(unsigned long nr, volatile unsigned long *addr)
 {
 	int oldbit;
 
 	asm volatile(LOCK_PREFIX "bts %2,%1\n\t"
-		     "sbb %0,%0" : "=r" (oldbit), ADDR : "Ir" (nr) : "memory");
+		     "sbb %0,%0" : "=r" (oldbit), ADDR : IDX(nr) : "memory");
 
 	return oldbit;
 }
@@ -210,7 +220,7 @@ static inline int test_and_set_bit(int nr, volatile unsigned long *addr)
  * This is the same as test_and_set_bit on x86.
  */
 static __always_inline int
-test_and_set_bit_lock(int nr, volatile unsigned long *addr)
+test_and_set_bit_lock(unsigned long nr, volatile unsigned long *addr)
 {
 	return test_and_set_bit(nr, addr);
 }
@@ -224,14 +234,14 @@ test_and_set_bit_lock(int nr, volatile unsigned long *addr)
  * If two examples of this operation race, one can appear to succeed
  * but actually fail.  You must protect multiple accesses with a lock.
  */
-static inline int __test_and_set_bit(int nr, volatile unsigned long *addr)
+static inline int __test_and_set_bit(unsigned long nr, volatile unsigned long *addr)
 {
 	int oldbit;
 
 	asm("bts %2,%1\n\t"
 	    "sbb %0,%0"
 	    : "=r" (oldbit), ADDR
-	    : "Ir" (nr));
+	    : IDX(nr));
 	return oldbit;
 }
 
@@ -243,13 +253,13 @@ static inline int __test_and_set_bit(int nr, volatile unsigned long *addr)
  * This operation is atomic and cannot be reordered.
  * It also implies a memory barrier.
  */
-static inline int test_and_clear_bit(int nr, volatile unsigned long *addr)
+static inline int test_and_clear_bit(unsigned long nr, volatile unsigned long *addr)
 {
 	int oldbit;
 
 	asm volatile(LOCK_PREFIX "btr %2,%1\n\t"
 		     "sbb %0,%0"
-		     : "=r" (oldbit), ADDR : "Ir" (nr) : "memory");
+		     : "=r" (oldbit), ADDR : IDX(nr) : "memory");
 
 	return oldbit;
 }
@@ -263,26 +273,26 @@ static inline int test_and_clear_bit(int nr, volatile unsigned long *addr)
  * If two examples of this operation race, one can appear to succeed
  * but actually fail.  You must protect multiple accesses with a lock.
  */
-static inline int __test_and_clear_bit(int nr, volatile unsigned long *addr)
+static inline int __test_and_clear_bit(unsigned long nr, volatile unsigned long *addr)
 {
 	int oldbit;
 
 	asm volatile("btr %2,%1\n\t"
 		     "sbb %0,%0"
 		     : "=r" (oldbit), ADDR
-		     : "Ir" (nr));
+		     : IDX(nr));
 	return oldbit;
 }
 
 /* WARNING: non atomic and it can be reordered! */
-static inline int __test_and_change_bit(int nr, volatile unsigned long *addr)
+static inline int __test_and_change_bit(unsigned long nr, volatile unsigned long *addr)
 {
 	int oldbit;
 
 	asm volatile("btc %2,%1\n\t"
 		     "sbb %0,%0"
 		     : "=r" (oldbit), ADDR
-		     : "Ir" (nr) : "memory");
+		     : IDX(nr) : "memory");
 
 	return oldbit;
 }
@@ -295,31 +305,31 @@ static inline int __test_and_change_bit(int nr, volatile unsigned long *addr)
  * This operation is atomic and cannot be reordered.
  * It also implies a memory barrier.
  */
-static inline int test_and_change_bit(int nr, volatile unsigned long *addr)
+static inline int test_and_change_bit(unsigned long nr, volatile unsigned long *addr)
 {
 	int oldbit;
 
 	asm volatile(LOCK_PREFIX "btc %2,%1\n\t"
 		     "sbb %0,%0"
-		     : "=r" (oldbit), ADDR : "Ir" (nr) : "memory");
+		     : "=r" (oldbit), ADDR : IDX(nr) : "memory");
 
 	return oldbit;
 }
 
-static __always_inline int constant_test_bit(unsigned int nr, const volatile unsigned long *addr)
+static __always_inline int constant_test_bit(unsigned long nr, const volatile unsigned long *addr)
 {
 	return ((1UL << (nr % BITS_PER_LONG)) &
 		(((unsigned long *)addr)[nr / BITS_PER_LONG])) != 0;
 }
 
-static inline int variable_test_bit(int nr, volatile const unsigned long *addr)
+static inline int variable_test_bit(unsigned long nr, volatile const unsigned long *addr)
 {
 	int oldbit;
 
 	asm volatile("bt %2,%1\n\t"
 		     "sbb %0,%0"
 		     : "=r" (oldbit)
-		     : "m" (*(unsigned long *)addr), "Ir" (nr));
+		     : "m" (*(unsigned long *)addr), IDX(nr));
 
 	return oldbit;
 }
@@ -330,7 +340,7 @@ static inline int variable_test_bit(int nr, volatile const unsigned long *addr)
  * @nr: bit number to test
  * @addr: Address to start counting from
  */
-static int test_bit(int nr, const volatile unsigned long *addr);
+static int test_bit(unsigned long nr, const volatile unsigned long *addr);
 #endif
 
 #define test_bit(nr, addr)			\

Re: [PATCH] x86: Extend test_and_set_bit() test_and_clean_bit() to 64 bits in X86_64

[Sheng Yang]

On Thursday 14 May 2009 01:29:15 H. Peter Anvin wrote:
> H. Peter Anvin wrote:
> > H. Peter Anvin wrote:
> >> Sheng Yang wrote:
> >>> This fix 44/45 bit width memory can't boot up issue. The reason is
> >>> free_bootmem_node()->mark_bootmem_node()->__free() use
> >>> test_and_clean_bit() to clean node_bootmem_map, but for 44bits width
> >>> address, the idx set bit 31 (43 - 12), which consider as a nagetive
> >>> value for bts.
> >>>
> >>> This patch applied to tip/mm.
> >>
> >> Hi Sheng,
> >>
> >> Could you try the attached patch instead?
> >
> > Sorry, wrong patch entirely... here is the right one.
>
> This time, for real?  Sheesh.  I'm having a morning, apparently.
>
> 	-hpa

Yeah, this one also works well(lightly tested). :)

But one thing should be noticed that, bit ops recognized the input as signed. 
According to SDM 2A 3.1.1.7 Operation Section, Bit(BitBase, BitOffset) can 
accept BitOffset as negative value, then search backward... Well, I indeed 
don't know when we need this, but I think keep signed here should be better...

-- 
regards
Yang, Sheng 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] x86: Extend test_and_set_bit() test_and_clean_bit() to 64 bits in X86_64

[H. Peter Anvin]

Sheng Yang wrote:
> 
> Yeah, this one also works well(lightly tested). :)
> 
> But one thing should be noticed that, bit ops recognized the input as signed. 
> According to SDM 2A 3.1.1.7 Operation Section, Bit(BitBase, BitOffset) can 
> accept BitOffset as negative value, then search backward... Well, I indeed 
> don't know when we need this, but I think keep signed here should be better...
> 

Urk, you're right.  How daft.  I had preferred to switch it to unsigned
long to match MIPS and SPARC, but that probably is a good reason to
leave it signed.  Pain.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] x86: Extend test_and_set_bit() test_and_clean_bit() to 64 bits in X86_64

[Christoph Hellwig]

On Wed, May 13, 2009 at 04:17:27PM +0800, Sheng Yang wrote:
> This fix 44/45 bit width memory can't boot up issue. The reason is
> free_bootmem_node()->mark_bootmem_node()->__free() use test_and_clean_bit() to
> clean node_bootmem_map, but for 44bits width address, the idx set bit 31 (43 -
> 12), which consider as a nagetive value for bts.

Should we really have different prototypes for these helpers on
different architectures?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] x86: Extend test_and_set_bit() test_and_clean_bit() to 64 bits in X86_64

[H. Peter Anvin]

Christoph Hellwig wrote:
> On Wed, May 13, 2009 at 04:17:27PM +0800, Sheng Yang wrote:
>> This fix 44/45 bit width memory can't boot up issue. The reason is
>> free_bootmem_node()->mark_bootmem_node()->__free() use test_and_clean_bit() to
>> clean node_bootmem_map, but for 44bits width address, the idx set bit 31 (43 -
>> 12), which consider as a nagetive value for bts.
> 
> Should we really have different prototypes for these helpers on
> different architectures?
> 

We already do: SPARC and MIPS have unsigned long, and apparently have
been unsigned long for a long time.  Given how the x86 bitops work, they
would have to be signed, which would mean to introduce a third
prototype, which really is the suck, but it's not like it would be the
only oddball.  Either that or we have to redesign the bootmem system for
very large amounts of memory.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>