2.6.9-rc1-mm4 sparc reiser4 build broken - undefined atomic_sub_and_test

2.6.9-rc1-mm4 sparc reiser4 build broken - undefined atomic_sub_and_test

[Paul Jackson]

The default config for sparc on 2.6.9-rc1-mm4 doesn't build, using the
crosstools from http://developer.osdl.org/dev/plm/cross_compile.

Hans,

  Andrew counts on us to build for various arch's, especially when
  submitting something non-trivial.  The above crosstools work
  pretty good - give them a try.

The final link fails with:

  fs/built-in.o(.text+0x58618): In function `end_io_handler':
  : undefined reference to `atomic_sub_and_test'
  make[1]: *** [arch/sparc/boot/image] Error 1

The macro 'atomic_sub_and_test' is defined for more or less every other
arch, in various include/asm-*/atomic.h files, but not defined for
sparc.

This macro is used in:

	fs/reiser4/flush_queue.c:
               if (atomic_sub_and_test(bio->bi_vcnt, &fq->nr_submitted))

If I disable the config items:

  CONFIG_REISER4_FS=y
  CONFIG_REISER4_LARGE_KEY=y

then it builds ok (with the bogus #else removed from cachefs.h, as
already reported on lkml).

-- 
                          I won't rest till it's the best ...
                          Programmer, Linux Scalability
                          Paul Jackson <pj@sgi.com> 1.650.933.1373

Re: 2.6.9-rc1-mm4 sparc reiser4 build broken - undefined atomic_sub_and_test

[William Lee Irwin III]

On Sun, Sep 12, 2004 at 03:12:35AM -0700, Paul Jackson wrote:
> The final link fails with:
>   fs/built-in.o(.text+0x58618): In function `end_io_handler':
>   : undefined reference to `atomic_sub_and_test'
>   make[1]: *** [arch/sparc/boot/image] Error 1
> The macro 'atomic_sub_and_test' is defined for more or less every other
> arch, in various include/asm-*/atomic.h files, but not defined for
> sparc.
> This macro is used in:
> 	fs/reiser4/flush_queue.c:
>                if (atomic_sub_and_test(bio->bi_vcnt, &fq->nr_submitted))
> If I disable the config items:
>   CONFIG_REISER4_FS=y
>   CONFIG_REISER4_LARGE_KEY=y
> then it builds ok (with the bogus #else removed from cachefs.h, as
> already reported on lkml).

I'm loath to add this for the sake of reiser4; I rather favor the use of
portable constructs.


-- wli

Re: 2.6.9-rc1-mm4 sparc reiser4 build broken - undefined atomic_sub_and_test

[Alex Zarochentsev]

On Sun, Sep 12, 2004 at 03:12:35AM -0700, Paul Jackson wrote:
> The default config for sparc on 2.6.9-rc1-mm4 doesn't build, using the
> crosstools from http://developer.osdl.org/dev/plm/cross_compile.
> 
> Hans,
> 
>   Andrew counts on us to build for various arch's, especially when
>   submitting something non-trivial.  The above crosstools work
>   pretty good - give them a try.
> 
> The final link fails with:
> 
>   fs/built-in.o(.text+0x58618): In function `end_io_handler':
>   : undefined reference to `atomic_sub_and_test'
>   make[1]: *** [arch/sparc/boot/image] Error 1
> 
> The macro 'atomic_sub_and_test' is defined for more or less every other
> arch, in various include/asm-*/atomic.h files, but not defined for
> sparc.

It is defined in both asm-sparc/atomic.h and asm-sparc64/atomic.h, 
but <asm/atomic.h> was not included into fs/reiser4/flush_queue.c 

please try this patch:
=============================
--- reiser4-linux-2.6/fs/reiser4/flush_queue.c.orig	2004-09-12 20:28:10.000000000 +0400
+++ reiser4-linux-2.6/fs/reiser4/flush_queue.c	2004-09-12 20:28:26.000000000 +0400
@@ -11,6 +11,7 @@
 #include "vfs_ops.h"
 #include "writeout.h"
 
+#include <asm/atomic.h>
 #include <linux/bio.h>
 #include <linux/mm.h>
 #include <linux/pagemap.h>
===============================


> This macro is used in:
> 
> 	fs/reiser4/flush_queue.c:
>                if (atomic_sub_and_test(bio->bi_vcnt, &fq->nr_submitted))
> 
> If I disable the config items:
> 
>   CONFIG_REISER4_FS=y
>   CONFIG_REISER4_LARGE_KEY=y
> 
> then it builds ok (with the bogus #else removed from cachefs.h, as
> already reported on lkml).
> 
> -- 
>                           I won't rest till it's the best ...
>                           Programmer, Linux Scalability
>                           Paul Jackson <pj@sgi.com> 1.650.933.1373
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

-- 
Alex.

Re: 2.6.9-rc1-mm4 sparc reiser4 build broken - undefined atomic_sub_and_test

[Paul Jackson]

Alex writes:
> please try this patch:
> ...
> +++ reiser4-linux-2.6/fs/reiser4/flush_queue.c
> ...
> +#include <asm/atomic.h>

This patch doesn't help.

Both with and without this patch, I observe the following:

 1) include/asm/atomic.h is listed in fs/reiser4/.flush_queue.o.cmd
    (apparently it is included indirectly, even if not explicitly so)
 2) The command:
	make fs/reiser4/flush_queue.o
    produces the output:
	fs/reiser4/flush_queue.c: In function `end_io_handler':
	fs/reiser4/flush_queue.c:451: warning: implicit declaration of function `atomic_sub_and_test'
 3) include/asm/atomic.h (which is include/asm-sparc/atomic.h via the asm symlink)
    does _not_ mention atomic_sub_and_test
 4) include/asm-sparc64/atomic.h _does_ mention atomic_sub_and_test (and build ok)
 5) the final kernel link fails with:
	fs/built-in.o(.text+0x58618): In function `end_io_handler':
	: undefined reference to `atomic_sub_and_test'
	make[1]: *** [arch/sparc/boot/image] Error 1

I also saw an email from Bill Irwin go by, explaining that he did not
choose to add atomic_sub_and_test to include/asm-sparc/atomic.h, which
email is consistent with my observation that sparc atomic.h does not
define atomic_sub_and_test.

It would seem that your asm-sparc/atomic.h is not the same as mine.

I believe that mine is the one in 2.6.9-rc1-mm4, which is the same as
the latest one in Linus' bk tree, since its most recent change of:

  1.10 04/05/14 19:00:05 akpm@osdl.org[torvalds] +10 -0
  Implement atomic_inc_and_test() on various architectures

and continuing through now.  This atomic.h is 4550 bytes long, with an
md5sum of:

	90eb38997e21e579fc1cd1617180d630  include/asm-sparc/atomic.h

And, to repeat myself, it has no mention of atomic_sub_and_test.

-- 
                          I won't rest till it's the best ...
                          Programmer, Linux Scalability
                          Paul Jackson <pj@sgi.com> 1.650.933.1373

Re: 2.6.9-rc1-mm4 sparc reiser4 build broken - undefined atomic_sub_and_test

[Alex Zarochentsev]

On Sun, Sep 12, 2004 at 11:49:48AM -0700, Paul Jackson wrote:
> Alex writes:
> > please try this patch:
> > ...
> > +++ reiser4-linux-2.6/fs/reiser4/flush_queue.c
> > ...
> > +#include <asm/atomic.h>
> 
> This patch doesn't help.
> 
> Both with and without this patch, I observe the following:
> 
>  1) include/asm/atomic.h is listed in fs/reiser4/.flush_queue.o.cmd
>     (apparently it is included indirectly, even if not explicitly so)
>  2) The command:
> 	make fs/reiser4/flush_queue.o
>     produces the output:
> 	fs/reiser4/flush_queue.c: In function `end_io_handler':
> 	fs/reiser4/flush_queue.c:451: warning: implicit declaration of function `atomic_sub_and_test'
>  3) include/asm/atomic.h (which is include/asm-sparc/atomic.h via the asm symlink)
>     does _not_ mention atomic_sub_and_test
>  4) include/asm-sparc64/atomic.h _does_ mention atomic_sub_and_test (and build ok)
>  5) the final kernel link fails with:
> 	fs/built-in.o(.text+0x58618): In function `end_io_handler':
> 	: undefined reference to `atomic_sub_and_test'
> 	make[1]: *** [arch/sparc/boot/image] Error 1
> 
> I also saw an email from Bill Irwin go by, explaining that he did not
> choose to add atomic_sub_and_test to include/asm-sparc/atomic.h, which
> email is consistent with my observation that sparc atomic.h does not
> define atomic_sub_and_test.
> 
> It would seem that your asm-sparc/atomic.h is not the same as mine.
> 
> I believe that mine is the one in 2.6.9-rc1-mm4, which is the same as
> the latest one in Linus' bk tree, since its most recent change of:
> 
>   1.10 04/05/14 19:00:05 akpm@osdl.org[torvalds] +10 -0
>   Implement atomic_inc_and_test() on various architectures
> 
> and continuing through now.  This atomic.h is 4550 bytes long, with an
> md5sum of:
> 
> 	90eb38997e21e579fc1cd1617180d630  include/asm-sparc/atomic.h
> 
> And, to repeat myself, it has no mention of atomic_sub_and_test.

ah, that was atomic24_sub_and_test(), marked as
/* This is the old 24-bit implementation. ... */

I think adding the wrappers for atomic_sub_and_test() wouldn't be wrong:

===== include/asm-sparc/atomic.h 1.10 vs edited =====
--- 1.10/include/asm-sparc/atomic.h	Sat May 15 06:00:05 2004
+++ edited/include/asm-sparc/atomic.h	Sun Sep 12 23:37:05 2004
@@ -44,8 +44,9 @@
  * other cases.
  */
 #define atomic_inc_and_test(v) (atomic_inc_return(v) == 0)
-
 #define atomic_dec_and_test(v) (atomic_dec_return(v) == 0)
+#define atomic_add_and_test(i, v) (atomic_add_return(i, v) == 0)
+#define atomic_sub_and_test(i, v) (atomic_sub_return(i, v) == 0)
 
 /* This is the old 24-bit implementation.  It's still used internally
  * by some sparc-specific code, notably the semaphore implementation.
=============================

Interesting that asm-sparc64/atomic.h defines atomic_add/sub_and_test().

>                           I won't rest till it's the best ...
>                           Programmer, Linux Scalability
>                           Paul Jackson <pj@sgi.com> 1.650.933.1373

-- 
Alex.

Re: 2.6.9-rc1-mm4 sparc reiser4 build broken - undefined atomic_sub_and_test

[Hugh Dickins]

On Sun, 12 Sep 2004, Alex Zarochentsev wrote:
> On Sun, Sep 12, 2004 at 11:49:48AM -0700, Paul Jackson wrote:
> > 
> > I also saw an email from Bill Irwin go by, explaining that he did not
> > choose to add atomic_sub_and_test to include/asm-sparc/atomic.h, which
> > email is consistent with my observation that sparc atomic.h does not
> > define atomic_sub_and_test.
> 
> I think adding the wrappers for atomic_sub_and_test() wouldn't be wrong:

But Bill already said he doesn't want it, and there are several other
architectures than sparc which don't have it - it's unclear why any do.

Several people were working to get atomic_add_return and its friends into
the remaining architectures (such as i386), Andrew's taken those patches
into -mm5, so it would make sense for reiser4 to use atomic_sub_return now.

Oh, except that particular variant is missing from s390: better add it in.

Signed-off-by: Hugh Dickins <hugh@veritas.com>

--- 2.6.9-rc1-mm5/fs/reiser4/flush_queue.c	2004-09-13 12:22:11.472865432 +0100
+++ linux/fs/reiser4/flush_queue.c	2004-09-13 13:15:50.505877800 +0100
@@ -447,7 +447,7 @@ end_io_handler(struct bio *bio, unsigned
 
 		/* If all write requests registered in this "fq" are done we up
 		 * the semaphore. */
-		if (atomic_sub_and_test(bio->bi_vcnt, &fq->nr_submitted))
+		if (atomic_sub_return(bio->bi_vcnt, &fq->nr_submitted) == 0)
 			up(&fq->io_sem);
 	}
 
--- 2.6.9-rc1-mm5/include/asm-s390/atomic.h	2004-06-16 06:20:36.000000000 +0100
+++ linux/include/asm-s390/atomic.h	2004-09-13 13:18:33.456105600 +0100
@@ -61,6 +61,10 @@ static __inline__ void atomic_sub(int i,
 {
 	       __CS_LOOP(v, i, "sr");
 }
+static __inline__ int atomic_sub_return(int i, atomic_t * v)
+{
+	return __CS_LOOP(v, i, "sr");
+}
 static __inline__ void atomic_inc(volatile atomic_t * v)
 {
 	       __CS_LOOP(v, 1, "ar");

Re: 2.6.9-rc1-mm4 sparc reiser4 build broken - undefined atomic_sub_and_test

[Roman Zippel]

Hi,

On Mon, 13 Sep 2004, Hugh Dickins wrote:

> But Bill already said he doesn't want it, [...]
> 
> -		if (atomic_sub_and_test(bio->bi_vcnt, &fq->nr_submitted))
> +		if (atomic_sub_return(bio->bi_vcnt, &fq->nr_submitted) == 0)

And that is more portable how? atomic_sub_and_test() allows some archs to 
generate better code and the rest can still fall back to 
atomic_sub_return. (Maybe it's time for <linux/atomic.h>/ 
<asm-generic/atomic.h>?)

bye, Roman

Re: 2.6.9-rc1-mm4 sparc reiser4 build broken - undefined atomic_sub_and_test

[Hugh Dickins]

On Mon, 13 Sep 2004, Roman Zippel wrote:
> On Mon, 13 Sep 2004, Hugh Dickins wrote:
> 
> > But Bill already said he doesn't want it, [...]
> > 
> > -		if (atomic_sub_and_test(bio->bi_vcnt, &fq->nr_submitted))
> > +		if (atomic_sub_return(bio->bi_vcnt, &fq->nr_submitted) == 0)
> 
> And that is more portable how?

It's more portable in that all but s390 already provide it
(and I expect Martin will be happy to add it).

Hugh

Re: 2.6.9-rc1-mm4 sparc reiser4 build broken - undefined atomic_sub_and_test

[Roman Zippel]

Hi,

On Mon, 13 Sep 2004, Hugh Dickins wrote:

> On Mon, 13 Sep 2004, Roman Zippel wrote:
> > On Mon, 13 Sep 2004, Hugh Dickins wrote:
> > 
> > > But Bill already said he doesn't want it, [...]
> > > 
> > > -		if (atomic_sub_and_test(bio->bi_vcnt, &fq->nr_submitted))
> > > +		if (atomic_sub_return(bio->bi_vcnt, &fq->nr_submitted) == 0)
> > 
> > And that is more portable how?
> 
> It's more portable in that all but s390 already provide it
> (and I expect Martin will be happy to add it).

So why not add the missing atomic_sub_and_test() (using simply 
atomic_sub_return)?

bye, Roman

Re: 2.6.9-rc1-mm4 sparc reiser4 build broken - undefined atomic_sub_and_test

[Hugh Dickins]

On Mon, 13 Sep 2004, Roman Zippel wrote:
> 
> So why not add the missing atomic_sub_and_test() (using simply 
> atomic_sub_return)?

sparc and s390 are not the only arches lacking atomic_sub_and_test.

Go ahead and send the patches changing all the arches that have it to
define __ARCH_HAS_ATOMIC_SUB_AND_TEST, and add asm-generic/atomic.h
for those that don't etc; but to me that seems like a waste of time -
unless Zam convinces us that Reiser4 will need every last ounce of
cpu on this particular line of code.

Hugh

Re: 2.6.9-rc1-mm4 sparc reiser4 build broken - undefined atomic_sub_and_test

[Roman Zippel]

Hi,

On Mon, 13 Sep 2004, Hugh Dickins wrote:

> Go ahead and send the patches changing all the arches that have it to
> define __ARCH_HAS_ATOMIC_SUB_AND_TEST, and add asm-generic/atomic.h
> for those that don't etc; but to me that seems like a waste of time -

Well, just doing it for atomic_sub_and_test would indeed be not really 
useful, but adding the missing macros isn't that difficult.
I'm still curious what's up with this portability argument...

bye, Roman

diff -ur linux-2.6.9-rc1-mm5.org/include/asm-arm/atomic.h linux-2.6.9-rc1-mm5/include/asm-arm/atomic.h
--- linux-2.6.9-rc1-mm5.org/include/asm-arm/atomic.h	2004-09-13 17:28:55.000000000 +0200
+++ linux-2.6.9-rc1-mm5/include/asm-arm/atomic.h	2004-09-13 17:40:18.000000000 +0200
@@ -147,6 +147,8 @@
 #define atomic_sub(i, v)	(void) atomic_sub_return(i, v)
 #define atomic_dec(v)		(void) atomic_sub_return(1, v)
 
+#define atomic_add_and_test(i,v) (atomic_add_return((i), (v)) == 0)
+#define atomic_sub_and_test(i,v) (atomic_sub_return((i), (v)) == 0)
 #define atomic_inc_and_test(v)	(atomic_add_return(1, v) == 0)
 #define atomic_dec_and_test(v)	(atomic_sub_return(1, v) == 0)
 #define atomic_inc_return(v)    (atomic_add_return(1, v))
diff -ur linux-2.6.9-rc1-mm5.org/include/asm-arm26/atomic.h linux-2.6.9-rc1-mm5/include/asm-arm26/atomic.h
--- linux-2.6.9-rc1-mm5.org/include/asm-arm26/atomic.h	2004-09-13 17:28:54.000000000 +0200
+++ linux-2.6.9-rc1-mm5/include/asm-arm26/atomic.h	2004-09-13 17:56:11.000000000 +0200
@@ -123,6 +123,9 @@
 	return atomic_add_return(-i, v);
 }
 
+#define atomic_add_and_test(i,v)	(atomic_add_return((i), (v)) == 0)
+#define atomic_sub_and_test(i,v)	(atomic_sub_return((i), (v)) == 0)
+#define atomic_inc_and_test(v)		(atomic_add_return(1, v) == 0)
 #define atomic_inc_return(v)  (atomic_add_return(1,v))
 #define atomic_dec_return(v)  (atomic_sub_return(1,v))
 
diff -ur linux-2.6.9-rc1-mm5.org/include/asm-h8300/atomic.h linux-2.6.9-rc1-mm5/include/asm-h8300/atomic.h
--- linux-2.6.9-rc1-mm5.org/include/asm-h8300/atomic.h	2004-06-17 10:40:26.000000000 +0200
+++ linux-2.6.9-rc1-mm5/include/asm-h8300/atomic.h	2004-09-13 17:45:43.000000000 +0200
@@ -26,6 +26,7 @@
 
 #define atomic_add(i, v) atomic_add_return(i, v)
 #define atomic_add_negative(a, v)	(atomic_add_return((a), (v)) < 0)
+#define atomic_add_and_test(i,v)	(atomic_add_return((i), (v)) == 0)
 
 static __inline__ int atomic_sub_return(int i, atomic_t *v)
 {
@@ -37,6 +38,7 @@
 }
 
 #define atomic_sub(i, v) atomic_sub_return(i, v)
+#define atomic_sub_and_test(i,v)	(atomic_sub_return((i), (v)) == 0)
 
 static __inline__ int atomic_inc_return(atomic_t *v)
 {
diff -ur linux-2.6.9-rc1-mm5.org/include/asm-parisc/atomic.h linux-2.6.9-rc1-mm5/include/asm-parisc/atomic.h
--- linux-2.6.9-rc1-mm5.org/include/asm-parisc/atomic.h	2004-06-17 10:41:22.000000000 +0200
+++ linux-2.6.9-rc1-mm5/include/asm-parisc/atomic.h	2004-09-13 17:46:19.000000000 +0200
@@ -196,8 +196,10 @@
  * other cases.
  */
 #define atomic_inc_and_test(v) (atomic_inc_return(v) == 0)
+#define atomic_add_and_test(i,v) (atomic_add_return((i), (v)) == 0)
 
 #define atomic_dec_and_test(v)	(atomic_dec_return(v) == 0)
+#define atomic_sub_and_test(i,v) (atomic_sub_return((i), (v)) == 0)
 
 #define ATOMIC_INIT(i)	{ (i) }
 
diff -ur linux-2.6.9-rc1-mm5.org/include/asm-s390/atomic.h linux-2.6.9-rc1-mm5/include/asm-s390/atomic.h
--- linux-2.6.9-rc1-mm5.org/include/asm-s390/atomic.h	2004-06-17 10:41:39.000000000 +0200
+++ linux-2.6.9-rc1-mm5/include/asm-s390/atomic.h	2004-09-13 17:51:17.000000000 +0200
@@ -53,6 +53,10 @@
 {
 	return __CS_LOOP(v, i, "ar");
 }
+static __inline__ int atomic_add_and_test(int i, atomic_t * v)
+{
+	return __CS_LOOP(v, i, "ar") == 0;
+}
 static __inline__ int atomic_add_negative(int i, atomic_t * v)
 {
 	return __CS_LOOP(v, i, "ar") < 0;
@@ -61,6 +65,14 @@
 {
 	       __CS_LOOP(v, i, "sr");
 }
+static __inline__ int atomic_sub_return(int i, atomic_t * v)
+{
+	return __CS_LOOP(v, i, "sr");
+}
+static __inline__ int atomic_sub_and_test(int i, atomic_t * v)
+{
+	return __CS_LOOP(v, i, "sr") == 0;
+}
 static __inline__ void atomic_inc(volatile atomic_t * v)
 {
 	       __CS_LOOP(v, 1, "ar");
diff -ur linux-2.6.9-rc1-mm5.org/include/asm-sparc/atomic.h linux-2.6.9-rc1-mm5/include/asm-sparc/atomic.h
--- linux-2.6.9-rc1-mm5.org/include/asm-sparc/atomic.h	2004-06-17 10:41:51.000000000 +0200
+++ linux-2.6.9-rc1-mm5/include/asm-sparc/atomic.h	2004-09-13 17:51:52.000000000 +0200
@@ -44,8 +44,10 @@
  * other cases.
  */
 #define atomic_inc_and_test(v) (atomic_inc_return(v) == 0)
+#define atomic_add_and_test(i,v) (atomic_add_return((i), (v)) == 0)
 
 #define atomic_dec_and_test(v) (atomic_dec_return(v) == 0)
+#define atomic_sub_and_test(i,v) (atomic_sub_return((i), (v)) == 0)
 
 /* This is the old 24-bit implementation.  It's still used internally
  * by some sparc-specific code, notably the semaphore implementation.

Re: 2.6.9-rc1-mm4 sparc reiser4 build broken - undefined atomic_sub_and_test

[Hugh Dickins]

On Mon, 13 Sep 2004, Roman Zippel wrote:
> I'm still curious what's up with this portability argument...

What portability argument?

Hugh

Re: 2.6.9-rc1-mm4 sparc reiser4 build broken - undefined atomic_sub_and_test

[Alex Zarochentsev]

On Mon, Sep 13, 2004 at 03:58:37PM +0100, Hugh Dickins wrote:
> On Mon, 13 Sep 2004, Roman Zippel wrote:
> > 
> > So why not add the missing atomic_sub_and_test() (using simply 
> > atomic_sub_return)?
> 
> sparc and s390 are not the only arches lacking atomic_sub_and_test.
> 
> Go ahead and send the patches changing all the arches that have it to
> define __ARCH_HAS_ATOMIC_SUB_AND_TEST, and add asm-generic/atomic.h
> for those that don't etc; but to me that seems like a waste of time -
> unless Zam convinces us that Reiser4 will need every last ounce of

I do not, Hans will ;-)

I just like to know what atomic.h common functions would be in 2.6.9+, because
nowdays the API is not consisent accross the arches. 

atomic_sub_return() is OK.

> cpu on this particular line of code.
> 
> Hugh

-- 
Alex.

Re: 2.6.9-rc1-mm4 sparc reiser4 build broken - undefined atomic_sub_and_test

[Tonnerre]

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

Salut,

On Mon, Sep 13, 2004 at 06:03:28PM +0200, Roman Zippel wrote:
> +#define atomic_add_and_test(i,v) (atomic_add_return((i), (v)) == 0)
> +#define atomic_sub_and_test(i,v) (atomic_sub_return((i), (v)) == 0)

This is no longer atomic, is it? I mean, there's no guarantee that the
atomic_add_return   and   the    comparison   are   executed   without
interruption, is there?

I wonder whether it's supposed to be..

>  #define atomic_sub(i, v) atomic_sub_return(i, v)

Maybe for some compilers we should cast away the result?

			Tonnerre

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: 2.6.9-rc1-mm4 sparc reiser4 build broken - undefined atomic_sub_and_test

[Roman Zippel]

Hi,

On Mon, 13 Sep 2004, Tonnerre wrote:

> On Mon, Sep 13, 2004 at 06:03:28PM +0200, Roman Zippel wrote:
> > +#define atomic_add_and_test(i,v) (atomic_add_return((i), (v)) == 0)
> > +#define atomic_sub_and_test(i,v) (atomic_sub_return((i), (v)) == 0)
> 
> This is no longer atomic, is it? I mean, there's no guarantee that the
> atomic_add_return   and   the    comparison   are   executed   without
> interruption, is there?

Only the read/write access needs to be atomic, when the comparison happens 
is irrelevant.

bye, Roman

Re: 2.6.9-rc1-mm4 sparc reiser4 build broken - undefined atomic_s ub_and_test

[Hugh Dickins]

On Mon, 13 Sep 2004, Tonnerre wrote:
> On Mon, Sep 13, 2004 at 06:03:28PM +0200, Roman Zippel wrote:
> > +#define atomic_add_and_test(i,v) (atomic_add_return((i), (v)) == 0)
> > +#define atomic_sub_and_test(i,v) (atomic_sub_return((i), (v)) == 0)
> 
> This is no longer atomic, is it? I mean, there's no guarantee that the
> atomic_add_return   and   the    comparison   are   executed   without
> interruption, is there?

It's true that the atomic_add_return and the comparison are not executed
atomically, but they don't need to be: a value is equal to 0, or not,
however long ago that value was computed, no matter what happened since.

The important thing is that the value is the atomic result of the atomic
operation: which may not be the case when you use atomic_add followed by
atomic_read, but is what's guaranteed by atomic_add_return.

Hugh

Re: 2.6.9-rc1-mm4 sparc reiser4 build broken - undefined atomic_sub_and_test

[William Lee Irwin III]

On Mon, Sep 13, 2004 at 03:58:37PM +0100, Hugh Dickins wrote:
>> sparc and s390 are not the only arches lacking atomic_sub_and_test.
>> Go ahead and send the patches changing all the arches that have it to
>> define __ARCH_HAS_ATOMIC_SUB_AND_TEST, and add asm-generic/atomic.h
>> for those that don't etc; but to me that seems like a waste of time -
>> unless Zam convinces us that Reiser4 will need every last ounce of

On Mon, Sep 13, 2004 at 09:19:36PM +0400, Alex Zarochentsev wrote:
> I do not, Hans will ;-)
> I just like to know what atomic.h common functions would be in 2.6.9+,
> because nowdays the API is not consisent accross the arches. 
> atomic_sub_return() is OK.

sparc32 is very legacy; in a quick IRC poll of sparc32 users there was
approximately zero interest in new filesystems and most users used nfs
and/or ext[23]. One should note that as these cpus are very slow and
the systems have very little RAM compared to modern ones, kernel memory
footprint and the cpu complexity of fs operations for small fs's is of
high importance. I really don't expect reiser4 to ever be runtime
tested on sparc32. UltraSPARC is another matter entirely.


-- wli

Re: 2.6.9-rc1-mm4 sparc reiser4 build broken - undefined atomic_sub_and_test

[Roman Zippel]

Hi,

On Mon, 13 Sep 2004, William Lee Irwin III wrote:

> > I just like to know what atomic.h common functions would be in 2.6.9+,
> > because nowdays the API is not consisent accross the arches. 
> > atomic_sub_return() is OK.
> 
> sparc32 is very legacy;

Sparc is not really relevant in this case, as it basically uses 
atomic_sub_return() anyway, but i386 or m68k can benefit from avoiding 
atomic_sub_return() if it's possible and that not only in reiserfs.

bye, Roman

Re: 2.6.9-rc1-mm4 sparc reiser4 build broken - undefined atomic_sub_and_test

[William Lee Irwin III]

On Mon, 13 Sep 2004, William Lee Irwin III wrote:
>> sparc32 is very legacy;

On Tue, Sep 14, 2004 at 11:00:50AM +0200, Roman Zippel wrote:
> Sparc is not really relevant in this case, as it basically uses 
> atomic_sub_return() anyway, but i386 or m68k can benefit from avoiding 
> atomic_sub_return() if it's possible and that not only in reiserfs.

The only point I had to make was that I'd rather avoid adding arch
hooks for code that will never be run on the arch. I suppose for the
sake of compiletesting...


-- wli

Add atomic_sub_and_test() to sparc32, implemented in terms of
atomic_sub_return(),  so reiser4 can be simultaneously microoptimized
and made to pass compilation testing on sparc32.

Index: mm5-2.6.9-rc1/include/asm-sparc/atomic.h
===================================================================
--- mm5-2.6.9-rc1.orig/include/asm-sparc/atomic.h	2004-08-13 22:37:25.000000000 -0700
+++ mm5-2.6.9-rc1/include/asm-sparc/atomic.h	2004-09-14 01:59:51.579542880 -0700
@@ -46,6 +46,7 @@
 #define atomic_inc_and_test(v) (atomic_inc_return(v) == 0)
 
 #define atomic_dec_and_test(v) (atomic_dec_return(v) == 0)
+#define atomic_sub_and_test(v) (atomic_sub_return(v) == 0)
 
 /* This is the old 24-bit implementation.  It's still used internally
  * by some sparc-specific code, notably the semaphore implementation.

[sparc32] add atomic_sub_and_test() to make reiser4 code microoptimized for x86 compile on sparc32

[William Lee Irwin III]

On Tue, Sep 14, 2004 at 02:10:45AM -0700, William Lee Irwin III wrote:
> The only point I had to make was that I'd rather avoid adding arch
> hooks for code that will never be run on the arch. I suppose for the
> sake of compiletesting...

Repost with appropriate Subject: line (I'm trying to cut down on these).


-- wli

Add atomic_sub_and_test() to sparc32, implemented in terms of
atomic_sub_return(),  so reiser4 can be simultaneously microoptimized
for x86 and made to pass compilation testing on sparc32.


Index: mm5-2.6.9-rc1/include/asm-sparc/atomic.h
===================================================================
--- mm5-2.6.9-rc1.orig/include/asm-sparc/atomic.h	2004-08-13 22:37:25.000000000 -0700
+++ mm5-2.6.9-rc1/include/asm-sparc/atomic.h	2004-09-14 01:59:51.579542880 -0700
@@ -46,6 +46,7 @@
 #define atomic_inc_and_test(v) (atomic_inc_return(v) == 0)
 
 #define atomic_dec_and_test(v) (atomic_dec_return(v) == 0)
+#define atomic_sub_and_test(v) (atomic_sub_return(v) == 0)
 
 /* This is the old 24-bit implementation.  It's still used internally
  * by some sparc-specific code, notably the semaphore implementation.

Re: [sparc32] add atomic_sub_and_test() to make reiser4 code microoptimized for x86 compile on sparc32

[William Lee Irwin III]

On Tue, Sep 14, 2004 at 02:15:05AM -0700, William Lee Irwin III wrote:
> Repost with appropriate Subject: line (I'm trying to cut down on these).

Repost with a correct patch.


-- wli

Add atomic_sub_and_test() to sparc32, implemented in terms of
atomic_sub_return(),  so reiser4 can be simultaneously microoptimized
for x86 and made to pass compilation testing on sparc32.

Index: mm5-2.6.9-rc1/include/asm-sparc/atomic.h
===================================================================
--- mm5-2.6.9-rc1.orig/include/asm-sparc/atomic.h	2004-09-14 08:14:08.215615280 -0700
+++ mm5-2.6.9-rc1/include/asm-sparc/atomic.h	2004-09-14 08:14:41.384572832 -0700
@@ -46,6 +46,7 @@
 #define atomic_inc_and_test(v) (atomic_inc_return(v) == 0)
 
 #define atomic_dec_and_test(v) (atomic_dec_return(v) == 0)
+#define atomic_sub_and_test(i, v) (atomic_sub_return(i, v) == 0)
 
 /* This is the old 24-bit implementation.  It's still used internally
  * by some sparc-specific code, notably the semaphore implementation.

Re: 2.6.9-rc1-mm4 sparc reiser4 build broken - undefined atomic_sub_and_test

[Roman Zippel]

Hi,

On Tue, 14 Sep 2004, Martin Schwidefsky wrote:

> > On some archs atomic_sub_return is more complex than atomic_sub_and_test
> > and there it does make a difference.
> 
> Well even if you can save lets say 10 cycles for the atomic_sub_return
> primitive you still won't notice any difference. The code in question
> is part of the end i/o function.

It's not just about this particular piece of code, if we introduce this as 
official API, people will start using it and in some cases it might 
matter. atomic_dec_and_test() can also be implemented using 
atomic_sub_return(), but I doubt you want to replace or deprecate it.

bye, Roman

Re: 2.6.9-rc1-mm4 sparc reiser4 build broken - undefined atomic_sub_and_test

[Roman Zippel]

Hi,

On Mon, 13 Sep 2004, Martin Schwidefsky wrote:

> Well, adding the missing s390 atomic operations is easy (see patch).
> But I really doubt that it will make a measurable difference in
> performance on ANY architecture.

On some archs atomic_sub_return is more complex than atomic_sub_and_test 
and there it does make a difference.

bye, Roman

Re: 2.6.9-rc1-mm4 sparc reiser4 build broken - undefined atomic_sub_and_test

[Martin Schwidefsky]

> > > But Bill already said he doesn't want it, [...]
> > > 
> > > -		 		 if (atomic_sub_and_test(bio->bi_vcnt, &fq->nr_submitted))
> > > +		 		 if (atomic_sub_return(bio->bi_vcnt, &fq->nr_submitted) == 0)
> > 
> > And that is more portable how?
> 
> It's more portable in that all but s390 already provide it
> (and I expect Martin will be happy to add it).

Well, adding the missing s390 atomic operations is easy (see patch).
But I really doubt that it will make a measurable difference in
performance on ANY architecture. You could do this with a
(atomic_add_return(-x, &v) == 0) ...

blue skies,
  Martin.

---

From: Martin Schwidefsky <schwidefsky@de.ibm.com>

Add atomic[64]_add_and_test, atomic[64]_sub_return and
atomic[64]_sub_and_test primitives.

Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>

diffstat:
 include/asm-s390/atomic.h |   24 ++++++++++++++++++++++++
 1 files changed, 24 insertions(+)

diff -urN linux-2.6/include/asm-s390/atomic.h linux-2.6-s390/include/asm-s390/atomic.h
--- linux-2.6/include/asm-s390/atomic.h	2004-08-14 12:54:48.000000000 +0200
+++ linux-2.6-s390/include/asm-s390/atomic.h	2004-09-13 19:04:13.000000000 +0200
@@ -53,6 +53,10 @@
 {
 	return __CS_LOOP(v, i, "ar");
 }
+static __inline__ int atomic_add_and_test(int i, atomic_t * v)
+{
+	return __CS_LOOP(v, i, "ar") == 0;
+}
 static __inline__ int atomic_add_negative(int i, atomic_t * v)
 {
 	return __CS_LOOP(v, i, "ar") < 0;
@@ -61,6 +65,14 @@
 {
 	       __CS_LOOP(v, i, "sr");
 }
+static __inline__ int atomic_sub_return(int i, atomic_t * v)
+{
+	return __CS_LOOP(v, i, "sr");
+}
+static __inline__ int atomic_sub_and_test(int i, atomic_t * v)
+{
+	return __CS_LOOP(v, i, "sr") == 0;
+}
 static __inline__ void atomic_inc(volatile atomic_t * v)
 {
 	       __CS_LOOP(v, 1, "ar");
@@ -127,6 +139,10 @@
 {
 	return __CSG_LOOP(v, i, "agr");
 }
+static __inline__ long long atomic64_add_and_test(int i, atomic64_t * v)
+{
+	return __CSG_LOOP(v, i, "agr") == 0;
+}
 static __inline__ long long atomic64_add_negative(int i, atomic64_t * v)
 {
 	return __CSG_LOOP(v, i, "agr") < 0;
@@ -135,6 +151,14 @@
 {
 	       __CSG_LOOP(v, i, "sgr");
 }
+static __inline__ long long atomic64_sub_return(int i, atomic64_t * v)
+{
+	return __CSG_LOOP(v, i, "sgr");
+}
+static __inline__ long long atomic64_sub_and_test(int i, atomic64_t * v)
+{
+	return __CSG_LOOP(v, i, "agr") == 0;
+}
 static __inline__ void atomic64_inc(volatile atomic64_t * v)
 {
 	       __CSG_LOOP(v, 1, "agr");