2004-09-12 10:17:06

by Paul Jackson

[permalink] [raw]
Subject: 2.6.9-rc1-mm4 sparc reiser4 build broken - undefined atomic_sub_and_test

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 <[email protected]> 1.650.933.1373


2004-09-12 10:54:54

by William Lee Irwin III

[permalink] [raw]
Subject: Re: 2.6.9-rc1-mm4 sparc reiser4 build broken - undefined atomic_sub_and_test

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

2004-09-12 16:41:54

by Alexander Zarochentsev

[permalink] [raw]
Subject: Re: 2.6.9-rc1-mm4 sparc reiser4 build broken - undefined atomic_sub_and_test

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 <[email protected]> 1.650.933.1373
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

--
Alex.

2004-09-12 18:50:20

by Paul Jackson

[permalink] [raw]
Subject: Re: 2.6.9-rc1-mm4 sparc reiser4 build broken - undefined atomic_sub_and_test

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 [email protected][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 <[email protected]> 1.650.933.1373

2004-09-12 19:57:08

by Alexander Zarochentsev

[permalink] [raw]
Subject: Re: 2.6.9-rc1-mm4 sparc reiser4 build broken - undefined atomic_sub_and_test

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 [email protected][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 <[email protected]> 1.650.933.1373

--
Alex.

2004-09-13 12:43:04

by Hugh Dickins

[permalink] [raw]
Subject: Re: 2.6.9-rc1-mm4 sparc reiser4 build broken - undefined atomic_sub_and_test

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 <[email protected]>

--- 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");

2004-09-13 13:25:56

by Roman Zippel

[permalink] [raw]
Subject: Re: 2.6.9-rc1-mm4 sparc reiser4 build broken - undefined atomic_sub_and_test

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

2004-09-13 13:51:54

by Hugh Dickins

[permalink] [raw]
Subject: Re: 2.6.9-rc1-mm4 sparc reiser4 build broken - undefined atomic_sub_and_test

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

2004-09-13 14:10:36

by Roman Zippel

[permalink] [raw]
Subject: Re: 2.6.9-rc1-mm4 sparc reiser4 build broken - undefined atomic_sub_and_test

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

2004-09-13 15:12:29

by Hugh Dickins

[permalink] [raw]
Subject: Re: 2.6.9-rc1-mm4 sparc reiser4 build broken - undefined atomic_sub_and_test

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

2004-09-13 16:10:57

by Roman Zippel

[permalink] [raw]
Subject: Re: 2.6.9-rc1-mm4 sparc reiser4 build broken - undefined atomic_sub_and_test

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.

2004-09-13 16:24:14

by Hugh Dickins

[permalink] [raw]
Subject: Re: 2.6.9-rc1-mm4 sparc reiser4 build broken - undefined atomic_sub_and_test

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

What portability argument?

Hugh

2004-09-13 17:28:00

by Alexander Zarochentsev

[permalink] [raw]
Subject: Re: 2.6.9-rc1-mm4 sparc reiser4 build broken - undefined atomic_sub_and_test

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.

2004-09-13 20:07:55

by Tonnerre

[permalink] [raw]
Subject: Re: 2.6.9-rc1-mm4 sparc reiser4 build broken - undefined atomic_sub_and_test

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


Attachments:
(No filename) (543.00 B)
signature.asc (189.00 B)
Digital signature
Download all attachments

2004-09-13 20:19:02

by Roman Zippel

[permalink] [raw]
Subject: Re: 2.6.9-rc1-mm4 sparc reiser4 build broken - undefined atomic_sub_and_test

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

2004-09-13 20:22:12

by Hugh Dickins

[permalink] [raw]
Subject: Re: 2.6.9-rc1-mm4 sparc reiser4 build broken - undefined atomic_s ub_and_test

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

2004-09-14 02:19:43

by William Lee Irwin III

[permalink] [raw]
Subject: Re: 2.6.9-rc1-mm4 sparc reiser4 build broken - undefined atomic_sub_and_test

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

2004-09-14 09:03:24

by Roman Zippel

[permalink] [raw]
Subject: Re: 2.6.9-rc1-mm4 sparc reiser4 build broken - undefined atomic_sub_and_test

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

2004-09-14 09:12:11

by William Lee Irwin III

[permalink] [raw]
Subject: Re: 2.6.9-rc1-mm4 sparc reiser4 build broken - undefined atomic_sub_and_test

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.

2004-09-14 09:17:23

by William Lee Irwin III

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

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.

2004-09-14 15:39:31

by William Lee Irwin III

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

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.