RE: [PATCH 1/12] generic *_bit()

RE: [PATCH 1/12] generic *_bit()

[Luck, Tony]

> > Intel doesn't care about big endian (cfr. your lkml back issues of January
> > 2006).
> 
> Incorrect.  Intel does actually produce big endian CPUs - most of the
> Intel IXP (ARM based) stuff is big endian.  It just depends which part
> of Intel you're referring to.

Set PSR.be (and DCR.be) to 1 and ia64 becomes a big-endian cpu (which,
IIRC, how HP-UX uses it).

-Tony

Re: [PATCH 1/12] generic *_bit()

[Russell King]

On Fri, Feb 03, 2006 at 11:24:30AM +0100, Geert Uytterhoeven wrote:
> On Wed, 1 Feb 2006, Russell King wrote:
> > Invalid assumption, from the point of view of endianness across different
> > architectures.  Consider where bit 0 is for a LE and BE unsigned long *
> > vs a LE and BE unsigned char *.
> 
> Intel doesn't care about big endian (cfr. your lkml back issues of January
> 2006).

Incorrect.  Intel does actually produce big endian CPUs - most of the
Intel IXP (ARM based) stuff is big endian.  It just depends which part
of Intel you're referring to.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

Re: [PATCH 1/12] generic *_bit()

[Geert Uytterhoeven]

On Wed, 1 Feb 2006, Russell King wrote:
> On Wed, Feb 01, 2006 at 10:07:28AM -0800, Chen, Kenneth W wrote:
> > Christoph Hellwig wrote on Wednesday, February 01, 2006 10:03 AM
> > > > Akinobu Mita wrote on Wednesday, January 25, 2006 7:29 PM
> > > > > This patch introduces the C-language equivalents of the functions below:
> > > > > 
> > > > > - atomic operation:
> > > > > void set_bit(int nr, volatile unsigned long *addr);
> > > > > void clear_bit(int nr, volatile unsigned long *addr);
> > > > > void change_bit(int nr, volatile unsigned long *addr);
> > > > > int test_and_set_bit(int nr, volatile unsigned long *addr);
> > > > > int test_and_clear_bit(int nr, volatile unsigned long *addr);
> > > > > int test_and_change_bit(int nr, volatile unsigned long *addr);
> > > > 
> > > > I wonder why you did not make these functions take volatile
> > > > unsigned int * address argument?
> > > 
> > > Because they are defined to operate on arrays of unsigned long
> > 
> > I think these should be defined to operate on arrays of unsigned int.
> > Bit is a bit, no matter how many byte you load (8/16/32/64), you can
> > only operate on just one bit.
> 
> Invalid assumption, from the point of view of endianness across different
> architectures.  Consider where bit 0 is for a LE and BE unsigned long *
> vs a LE and BE unsigned char *.

Intel doesn't care about big endian (cfr. your lkml back issues of January
2006).

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

RE: [PATCH 1/12] generic *_bit()

[Paul Mackerras]

Chen, Kenneth W writes:

> Christoph Hellwig wrote on Wednesday, February 01, 2006 10:03 AM
> > Because they are defined to operate on arrays of unsigned long
> 
> I think these should be defined to operate on arrays of unsigned int.
> Bit is a bit, no matter how many byte you load (8/16/32/64), you can
> only operate on just one bit.

Christoph is right.  Changing to unsigned int would change the layout
on big-endian 64-bit platforms.

Paul.

Re: [PATCH 1/12] generic *_bit()

[Andreas Schwab]

Anton Altaparmakov <aia21@cam.ac.uk> writes:

> The name seems a bit silly as I imagine most fs drivers would be able to 
> use them and there already are ext2 and minix versions.  Probably ought 
> be renamed to a more generic name like le_test_bit() or something...

Minix is even more complicated, since the on-disk format is different
between architectures (the m68k port of Minix did not handle that
correctly).

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH 1/12] generic *_bit()

[Anton Altaparmakov]

On Wed, 1 Feb 2006, Grant Grundler wrote:
> On Wed, Feb 01, 2006 at 10:49:08PM +0000, Anton Altaparmakov wrote:
> > Err, searching by anything other than bytes is useless for a file system 
> > driver.  Otherwise you get all sorts of disgustingly horrible allocation 
> > patterns depending on the endianness of the machine...
> 
> Well, tell that to ext2/3 maintainers since they introduced
> the ext2_test_bit() and friends. They do require LE handling
> of the bit array since that's an on-disk format. See how big endian
> machines (parisc/ppc/sparc/etc) deal with it in asm/bitops.h.

Oh, I hadn't noticed those before.  Thanks.

The name seems a bit silly as I imagine most fs drivers would be able to 
use them and there already are ext2 and minix versions.  Probably ought 
be renamed to a more generic name like le_test_bit() or something...

Best regards,

	Anton
-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/

Re: [PATCH 1/12] generic *_bit()

[Grant Grundler]

On Wed, Feb 01, 2006 at 10:49:08PM +0000, Anton Altaparmakov wrote:
> Err, searching by anything other than bytes is useless for a file system 
> driver.  Otherwise you get all sorts of disgustingly horrible allocation 
> patterns depending on the endianness of the machine...

Well, tell that to ext2/3 maintainers since they introduced
the ext2_test_bit() and friends. They do require LE handling
of the bit array since that's an on-disk format. See how big endian
machines (parisc/ppc/sparc/etc) deal with it in asm/bitops.h.

grant

Re: [PATCH 1/12] generic *_bit()

[Anton Altaparmakov]

On Wed, 1 Feb 2006, Grant Grundler wrote:
> On Wed, Feb 01, 2006 at 01:41:03PM -0800, Chen, Kenneth W wrote:
> > > Well, if it doesn't matter, why is unsigned int better?
> > 
> > I was coming from the angle of having bitop operate on unsigned
> > int *, so people don't have to type cast or change bit flag variable
> > to unsigned long for various structures.  With unsigned int type for
> > bit flag, some of them are not even close to fully utilized. for example:
> > 
> > thread_info->flags uses 18 bits
> > thread_struct->flags uses 7 bits
> > 
> > It's a waste of memory to define a variable that kernel will *never*
> > touch the 4 MSB in that field.
> 
> Agreed. Good point. But this can be mitigated if the code using "unsigned int"
> (or unsigned byte) first loads the value into a local unsigned long variable.
> That typically translates into a tmp register anyway. Compiler will help
> you find places where that needs to happen.
> 
> Counter point is bit arrays (e.g. bit maps) like cpumask_t are
> typically much larger than 32-bits (typically distro's ship with
> NR_CPUS set to 256 or so).  File system code also likes bit arrays
> for block allocation tables. Searching a bit array using unsigned
> long is 2x faster on 64-bit architectures. I don't want to give
> that up and I'm pretty sure Tony Luck, Paul Mckerras and a few
> others would object unless you can give a better reason.

Err, searching by anything other than bytes is useless for a file system 
driver.  Otherwise you get all sorts of disgustingly horrible allocation 
patterns depending on the endianness of the machine...

Best regards,

	Anton
-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/

Re: [PATCH 1/12] generic *_bit()

[Grant Grundler]

On Wed, Feb 01, 2006 at 01:41:03PM -0800, Chen, Kenneth W wrote:
> > Well, if it doesn't matter, why is unsigned int better?
> 
> I was coming from the angle of having bitop operate on unsigned
> int *, so people don't have to type cast or change bit flag variable
> to unsigned long for various structures.  With unsigned int type for
> bit flag, some of them are not even close to fully utilized. for example:
> 
> thread_info->flags uses 18 bits
> thread_struct->flags uses 7 bits
> 
> It's a waste of memory to define a variable that kernel will *never*
> touch the 4 MSB in that field.

Agreed. Good point. But this can be mitigated if the code using "unsigned int"
(or unsigned byte) first loads the value into a local unsigned long variable.
That typically translates into a tmp register anyway. Compiler will help
you find places where that needs to happen.

Counter point is bit arrays (e.g. bit maps) like cpumask_t are
typically much larger than 32-bits (typically distro's ship with
NR_CPUS set to 256 or so).  File system code also likes bit arrays
for block allocation tables. Searching a bit array using unsigned
long is 2x faster on 64-bit architectures. I don't want to give
that up and I'm pretty sure Tony Luck, Paul Mckerras and a few
others would object unless you can give a better reason.

Obviously neither memory footprint nor speed of walking memory is an
the issue for 32-bit arches (where unsigned long == unsigned int).


> > unsigned long is typically the native register size, right?
> > I'd expect that to be more efficient on most arches.
> 
> The only difference that I can think of on Itanium processor is the
> memory operation, you either load/store 4 or 8 bytes. Once the data
> is in the CPU register, it doesn't make any difference whether it is
> operating on 32bit or entire 64 bit. I don't know about others RISC
> arch though whether it is more efficient with native register size.

agreed. I was thinking mostly of the bit map search - not searching
within a single unsigned int.

grant

RE: [PATCH 1/12] generic *_bit()

[Chen, Kenneth W]

Grant Grundler wrote on Wednesday, February 01, 2006 11:40 AM
> On Wed, Feb 01, 2006 at 10:07:28AM -0800, Chen, Kenneth W wrote:
> > I think these should be defined to operate on arrays of unsigned int.
> > Bit is a bit, no matter how many byte you load (8/16/32/64), you can
> > only operate on just one bit.
> 
> Well, if it doesn't matter, why is unsigned int better?


I was coming from the angle of having bitop operate on unsigned
int *, so people don't have to type cast or change bit flag variable
to unsigned long for various structures.  With unsigned int type for
bit flag, some of them are not even close to fully utilized. for example:

thread_info->flags uses 18 bits
thread_struct->flags uses 7 bits

It's a waste of memory to define a variable that kernel will *never*
touch the 4 MSB in that field.


> unsigned long is typically the native register size, right?
> I'd expect that to be more efficient on most arches.


The only difference that I can think of on Itanium processor is the
memory operation, you either load/store 4 or 8 bytes. Once the data
is in the CPU register, it doesn't make any difference whether it is
operating on 32bit or entire 64 bit. I don't know about others RISC
arch though whether it is more efficient with native register size.

- Ken

Re: [PATCH 1/12] generic *_bit()

[Grant Grundler]

On Wed, Feb 01, 2006 at 10:07:28AM -0800, Chen, Kenneth W wrote:
> I think these should be defined to operate on arrays of unsigned int.
> Bit is a bit, no matter how many byte you load (8/16/32/64), you can
> only operate on just one bit.

Well, if it doesn't matter, why is unsigned int better?

unsigned long is typically the native register size, right?
I'd expect that to be more efficient on most arches.

grant

Re: [PATCH 1/12] generic *_bit()

[Russell King]

On Wed, Feb 01, 2006 at 11:25:25AM -0800, Chen, Kenneth W wrote:
> Russell King wrote on Wednesday, February 01, 2006 11:20 AM
> > > I think these should be defined to operate on arrays of unsigned int.
> > > Bit is a bit, no matter how many byte you load (8/16/32/64), you can
> > > only operate on just one bit.
> > 
> > Invalid assumption, from the point of view of endianness across different
> > architectures.  Consider where bit 0 is for a LE and BE unsigned long *
> > vs a LE and BE unsigned char *.
> 
> Where the bit end up in LE or BE is irrelevant. As long as one always
> use the same bit numbering and same address pointer type, you always
> get the same bit.  Or am I missing something?

>From a 32-bit long perspective, bit 0 of a long is always the bit which
represents odd numbers.  Where this falls depends on the endianness:

                       MSB                    LSB
big-endian long0:      byte0  byte1  byte2  byte3
little-endian long0:   byte3  byte2  byte1  byte0

Bit 0 of a BE long ends up at byte 3 bit 0.
Bit 0 of a LE long ends up at byte 0 bit 0.

However, bit 0 of a byte stream is always byte 0 bit 0.

Hence, converting the bitops to take a different sized pointer from
the one we presently pass changes the semantics of the function for
big endian machines - by the fact that you change the order of bits
in memory.

Whether this matters or not is up to how the bitops are used.  If
it's something which only bitops operate on, it probably doesn't
make that much difference.  If it's some external data or some
data which is accessed in other ways, it most certainly does matter.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

RE: [PATCH 1/12] generic *_bit()

[Chen, Kenneth W]

Russell King wrote on Wednesday, February 01, 2006 11:20 AM
> > I think these should be defined to operate on arrays of unsigned int.
> > Bit is a bit, no matter how many byte you load (8/16/32/64), you can
> > only operate on just one bit.
> 
> Invalid assumption, from the point of view of endianness across different
> architectures.  Consider where bit 0 is for a LE and BE unsigned long *
> vs a LE and BE unsigned char *.

Where the bit end up in LE or BE is irrelevant. As long as one always
use the same bit numbering and same address pointer type, you always
get the same bit.  Or am I missing something?

- Ken

Re: [PATCH 1/12] generic *_bit()

[Russell King]

On Wed, Feb 01, 2006 at 10:07:28AM -0800, Chen, Kenneth W wrote:
> Christoph Hellwig wrote on Wednesday, February 01, 2006 10:03 AM
> > > Akinobu Mita wrote on Wednesday, January 25, 2006 7:29 PM
> > > > This patch introduces the C-language equivalents of the functions below:
> > > > 
> > > > - atomic operation:
> > > > void set_bit(int nr, volatile unsigned long *addr);
> > > > void clear_bit(int nr, volatile unsigned long *addr);
> > > > void change_bit(int nr, volatile unsigned long *addr);
> > > > int test_and_set_bit(int nr, volatile unsigned long *addr);
> > > > int test_and_clear_bit(int nr, volatile unsigned long *addr);
> > > > int test_and_change_bit(int nr, volatile unsigned long *addr);
> > > 
> > > I wonder why you did not make these functions take volatile
> > > unsigned int * address argument?
> > 
> > Because they are defined to operate on arrays of unsigned long
> 
> I think these should be defined to operate on arrays of unsigned int.
> Bit is a bit, no matter how many byte you load (8/16/32/64), you can
> only operate on just one bit.

Invalid assumption, from the point of view of endianness across different
architectures.  Consider where bit 0 is for a LE and BE unsigned long *
vs a LE and BE unsigned char *.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

RE: [PATCH 1/12] generic *_bit()

[Chen, Kenneth W]

Christoph Hellwig wrote on Wednesday, February 01, 2006 10:03 AM
> > Akinobu Mita wrote on Wednesday, January 25, 2006 7:29 PM
> > > This patch introduces the C-language equivalents of the functions below:
> > > 
> > > - atomic operation:
> > > void set_bit(int nr, volatile unsigned long *addr);
> > > void clear_bit(int nr, volatile unsigned long *addr);
> > > void change_bit(int nr, volatile unsigned long *addr);
> > > int test_and_set_bit(int nr, volatile unsigned long *addr);
> > > int test_and_clear_bit(int nr, volatile unsigned long *addr);
> > > int test_and_change_bit(int nr, volatile unsigned long *addr);
> > 
> > I wonder why you did not make these functions take volatile
> > unsigned int * address argument?
> 
> Because they are defined to operate on arrays of unsigned long

I think these should be defined to operate on arrays of unsigned int.
Bit is a bit, no matter how many byte you load (8/16/32/64), you can
only operate on just one bit.

- Ken

Re: [PATCH 1/12] generic *_bit()

[Christoph Hellwig]

On Wed, Feb 01, 2006 at 07:11:34AM -0800, Chen, Kenneth W wrote:
> Akinobu Mita wrote on Wednesday, January 25, 2006 7:29 PM
> > This patch introduces the C-language equivalents of the functions below:
> > 
> > - atomic operation:
> > void set_bit(int nr, volatile unsigned long *addr);
> > void clear_bit(int nr, volatile unsigned long *addr);
> > void change_bit(int nr, volatile unsigned long *addr);
> > int test_and_set_bit(int nr, volatile unsigned long *addr);
> > int test_and_clear_bit(int nr, volatile unsigned long *addr);
> > int test_and_change_bit(int nr, volatile unsigned long *addr);
> 
> I wonder why you did not make these functions take volatile
> unsigned int * address argument?

Because they are defined to operate on arrays of unsigned long

RE: [PATCH 1/12] generic *_bit()

[Chen, Kenneth W]

Akinobu Mita wrote on Wednesday, January 25, 2006 7:29 PM
> This patch introduces the C-language equivalents of the functions below:
> 
> - atomic operation:
> void set_bit(int nr, volatile unsigned long *addr);
> void clear_bit(int nr, volatile unsigned long *addr);
> void change_bit(int nr, volatile unsigned long *addr);
> int test_and_set_bit(int nr, volatile unsigned long *addr);
> int test_and_clear_bit(int nr, volatile unsigned long *addr);
> int test_and_change_bit(int nr, volatile unsigned long *addr);

I wonder why you did not make these functions take volatile
unsigned int * address argument?

- Ken

[PATCH 1/12] generic *_bit()

[Akinobu Mita]

This patch introduces the C-language equivalents of the functions below:

- atomic operation:
void set_bit(int nr, volatile unsigned long *addr);
void clear_bit(int nr, volatile unsigned long *addr);
void change_bit(int nr, volatile unsigned long *addr);
int test_and_set_bit(int nr, volatile unsigned long *addr);
int test_and_clear_bit(int nr, volatile unsigned long *addr);
int test_and_change_bit(int nr, volatile unsigned long *addr);

- non-atomic operation:
void __set_bit(int nr, volatile unsigned long *addr);
void __clear_bit(int nr, volatile unsigned long *addr);
void __change_bit(int nr, volatile unsigned long *addr);
int __test_and_set_bit(int nr, volatile unsigned long *addr);
int __test_and_clear_bit(int nr, volatile unsigned long *addr);
int __test_and_change_bit(int nr, volatile unsigned long *addr);
int test_bit(int nr, const volatile unsigned long *addr);

HAVE_ARCH_ATOMIC_BITOPS is defined when the architecture has its own
{,test_and_}{set,clear,change}_bit()

HAVE_ARCH_NON_ATOMIC_BITOPS is defined when the architecture has its own
__{,test_and_}{set,clear,change}_bit() and test_bit()

This code largely copied from:
include/asm-powerpc/bitops.h
include/asm-parisc/bitops.h
include/asm-parisc/atomic.h


Index: 2.6-git/include/asm-generic/bitops.h
===================================================================
--- 2.6-git.orig/include/asm-generic/bitops.h	2006-01-25 19:07:14.000000000 +0900
+++ 2.6-git/include/asm-generic/bitops.h	2006-01-25 19:14:08.000000000 +0900
@@ -1,56 +1,198 @@
 #ifndef _ASM_GENERIC_BITOPS_H_
 #define _ASM_GENERIC_BITOPS_H_
 
+#include <asm/types.h>
+
+#define BITOP_MASK(nr)		(1UL << ((nr) % BITS_PER_LONG))
+#define BITOP_WORD(nr)		((nr) / BITS_PER_LONG)
+
+#ifndef HAVE_ARCH_ATOMIC_BITOPS
+
+#ifdef CONFIG_SMP
+#include <asm/spinlock.h>
+#include <asm/cache.h>		/* we use L1_CACHE_BYTES */
+
+/* Use an array of spinlocks for our atomic_ts.
+ * Hash function to index into a different SPINLOCK.
+ * Since "a" is usually an address, use one spinlock per cacheline.
+ */
+#  define ATOMIC_HASH_SIZE 4
+#  define ATOMIC_HASH(a) (&(__atomic_hash[ (((unsigned long) a)/L1_CACHE_BYTES) & (ATOMIC_HASH_SIZE-1) ]))
+
+extern raw_spinlock_t __atomic_hash[ATOMIC_HASH_SIZE] __lock_aligned;
+
+/* Can't use raw_spin_lock_irq because of #include problems, so
+ * this is the substitute */
+#define _atomic_spin_lock_irqsave(l,f) do {	\
+	raw_spinlock_t *s = ATOMIC_HASH(l);	\
+	local_irq_save(f);			\
+	__raw_spin_lock(s);			\
+} while(0)
+
+#define _atomic_spin_unlock_irqrestore(l,f) do {	\
+	raw_spinlock_t *s = ATOMIC_HASH(l);		\
+	__raw_spin_unlock(s);				\
+	local_irq_restore(f);				\
+} while(0)
+
+
+#else
+#  define _atomic_spin_lock_irqsave(l,f) do { local_irq_save(f); } while (0)
+#  define _atomic_spin_unlock_irqrestore(l,f) do { local_irq_restore(f); } while (0)
+#endif
+
 /*
  * For the benefit of those who are trying to port Linux to another
  * architecture, here are some C-language equivalents.  You should
  * recode these in the native assembly language, if at all possible.
- * To guarantee atomicity, these routines call cli() and sti() to
- * disable interrupts while they operate.  (You have to provide inline
- * routines to cli() and sti().)
  *
- * Also note, these routines assume that you have 32 bit longs.
- * You will have to change this if you are trying to port Linux to the
- * Alpha architecture or to a Cray.  :-)
- * 
  * C language equivalents written by Theodore Ts'o, 9/26/92
  */
 
-extern __inline__ int set_bit(int nr,long * addr)
+static __inline__ void set_bit(int nr, volatile unsigned long *addr)
+{
+	unsigned long mask = BITOP_MASK(nr);
+	unsigned long *p = ((unsigned long *)addr) + BITOP_WORD(nr);
+	unsigned long flags;
+
+	_atomic_spin_lock_irqsave(p, flags);
+	*p  |= mask;
+	_atomic_spin_unlock_irqrestore(p, flags);
+}
+
+static __inline__ void clear_bit(int nr, volatile unsigned long *addr)
+{
+	unsigned long mask = BITOP_MASK(nr);
+	unsigned long *p = ((unsigned long *)addr) + BITOP_WORD(nr);
+	unsigned long flags;
+
+	_atomic_spin_lock_irqsave(p, flags);
+	*p &= ~mask;
+	_atomic_spin_unlock_irqrestore(p, flags);
+}
+
+static __inline__ void change_bit(int nr, volatile unsigned long *addr)
+{
+	unsigned long mask = BITOP_MASK(nr);
+	unsigned long *p = ((unsigned long *)addr) + BITOP_WORD(nr);
+	unsigned long flags;
+
+	_atomic_spin_lock_irqsave(p, flags);
+	*p ^= mask;
+	_atomic_spin_unlock_irqrestore(p, flags);
+}
+
+static __inline__ int test_and_set_bit(int nr, volatile unsigned long *addr)
+{
+	unsigned long mask = BITOP_MASK(nr);
+	unsigned long *p = ((unsigned long *)addr) + BITOP_WORD(nr);
+	unsigned long old;
+	unsigned long flags;
+
+	_atomic_spin_lock_irqsave(p, flags);
+	old = *p;
+	*p = old | mask;
+	_atomic_spin_unlock_irqrestore(p, flags);
+
+	return (old & mask) != 0;
+}
+
+static __inline__ int test_and_clear_bit(int nr, volatile unsigned long *addr)
 {
-	int	mask, retval;
+	unsigned long mask = BITOP_MASK(nr);
+	unsigned long *p = ((unsigned long *)addr) + BITOP_WORD(nr);
+	unsigned long old;
+	unsigned long flags;
+
+	_atomic_spin_lock_irqsave(p, flags);
+	old = *p;
+	*p = old & ~mask;
+	_atomic_spin_unlock_irqrestore(p, flags);
 
-	addr += nr >> 5;
-	mask = 1 << (nr & 0x1f);
-	cli();
-	retval = (mask & *addr) != 0;
-	*addr |= mask;
-	sti();
-	return retval;
+	return (old & mask) != 0;
 }
 
-extern __inline__ int clear_bit(int nr, long * addr)
+static __inline__ int test_and_change_bit(int nr, volatile unsigned long *addr)
 {
-	int	mask, retval;
+	unsigned long mask = BITOP_MASK(nr);
+	unsigned long *p = ((unsigned long *)addr) + BITOP_WORD(nr);
+	unsigned long old;
+	unsigned long flags;
+
+	_atomic_spin_lock_irqsave(p, flags);
+	old = *p;
+	*p = old ^ mask;
+	_atomic_spin_unlock_irqrestore(p, flags);
 
-	addr += nr >> 5;
-	mask = 1 << (nr & 0x1f);
-	cli();
-	retval = (mask & *addr) != 0;
-	*addr &= ~mask;
-	sti();
-	return retval;
+	return (old & mask) != 0;
 }
 
-extern __inline__ int test_bit(int nr, const unsigned long * addr)
+#endif /* HAVE_ARCH_ATOMIC_BITOPS */
+
+#ifndef HAVE_ARCH_NON_ATOMIC_BITOPS
+
+static __inline__ void __set_bit(int nr, volatile unsigned long *addr)
 {
-	int	mask;
+	unsigned long mask = BITOP_MASK(nr);
+	unsigned long *p = ((unsigned long *)addr) + BITOP_WORD(nr);
 
-	addr += nr >> 5;
-	mask = 1 << (nr & 0x1f);
-	return ((mask & *addr) != 0);
+	*p  |= mask;
 }
 
+static __inline__ void __clear_bit(int nr, volatile unsigned long *addr)
+{
+	unsigned long mask = BITOP_MASK(nr);
+	unsigned long *p = ((unsigned long *)addr) + BITOP_WORD(nr);
+
+	*p &= ~mask;
+}
+
+static __inline__ void __change_bit(int nr, volatile unsigned long *addr)
+{
+	unsigned long mask = BITOP_MASK(nr);
+	unsigned long *p = ((unsigned long *)addr) + BITOP_WORD(nr);
+
+	*p ^= mask;
+}
+
+static __inline__ int __test_and_set_bit(int nr, volatile unsigned long *addr)
+{
+	unsigned long mask = BITOP_MASK(nr);
+	unsigned long *p = ((unsigned long *)addr) + BITOP_WORD(nr);
+	unsigned long old = *p;
+
+	*p = old | mask;
+	return (old & mask) != 0;
+}
+
+static __inline__ int __test_and_clear_bit(int nr, volatile unsigned long *addr)
+{
+	unsigned long mask = BITOP_MASK(nr);
+	unsigned long *p = ((unsigned long *)addr) + BITOP_WORD(nr);
+	unsigned long old = *p;
+
+	*p = old & ~mask;
+	return (old & mask) != 0;
+}
+
+static __inline__ int __test_and_change_bit(int nr,
+					    volatile unsigned long *addr)
+{
+	unsigned long mask = BITOP_MASK(nr);
+	unsigned long *p = ((unsigned long *)addr) + BITOP_WORD(nr);
+	unsigned long old = *p;
+
+	*p = old ^ mask;
+	return (old & mask) != 0;
+}
+
+static __inline__ int test_bit(int nr, __const__ volatile unsigned long *addr)
+{
+	return 1UL & (addr[BITOP_WORD(nr)] >> (nr & (BITS_PER_LONG-1)));
+}
+
+#endif /* HAVE_ARCH_NON_ATOMIC_BITOPS */
+
 /*
  * fls: find last bit set.
  */