Re: sparc: rename atomic_add_unless

Re: sparc: rename atomic_add_unless

[Josip Rodin]

Hi,

This patch[0] seems to fix a build issue, but it doesn't fix the
functionality - the previous change[1] changed the return value semantics,
it's now supposed to be to be just the old value, rather than a comparison.

[0] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;hg8624e401b0b7747762b5223fb23f86dcdacc93
[1] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;hò4219b4e90cf70ec4a211b17fbabc725a0ddf3c

-- 
     2. That which causes joy or happiness.

Re: sparc: rename atomic_add_unless

[Stephen Rothwell]

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

Hi,

On Thu, 28 Jul 2011 12:40:17 +0200 Josip Rodin <joy@entuzijast.net> wrote:
>
> This patch[0] seems to fix a build issue, but it doesn't fix the
> functionality - the previous change[1] changed the return value semantics,
> it's now supposed to be to be just the old value, rather than a comparison.
> 
> [0] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=678624e401b0b7747762b5223fb23f86dcdacc93
> [1] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=f24219b4e90cf70ec4a211b17fbabc725a0ddf3c

Ooops, sorry about that. Thanks for pointing it out.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

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

Re: sparc: rename atomic_add_unless

[David Miller]

From: Josip Rodin <joy@entuzijast.net>
Date: Thu, 28 Jul 2011 12:40:17 +0200

> This patch[0] seems to fix a build issue, but it doesn't fix the
> functionality - the previous change[1] changed the return value semantics,
> it's now supposed to be to be just the old value, rather than a comparison.
> 
> [0] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;hg8624e401b0b7747762b5223fb23f86dcdacc93
> [1] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;hò4219b4e90cf70ec4a211b17fbabc725a0ddf3c

Stephen please post a fix for this before a 32-bit sparc person
loses their filesystem or worse.

The return value semantics have changed for this function, you
can't just put two underscores in front of the name, export
the symbol, and say "Ship it!" :-)

Re: sparc: rename atomic_add_unless

[Stephen Rothwell]

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

Hi Dave,

On Thu, 04 Aug 2011 01:21:46 -0700 (PDT) David Miller <davem@davemloft.net> wrote:
>
> From: Josip Rodin <joy@entuzijast.net>
> Date: Thu, 28 Jul 2011 12:40:17 +0200
> 
> > This patch[0] seems to fix a build issue, but it doesn't fix the
> > functionality - the previous change[1] changed the return value semantics,
> > it's now supposed to be to be just the old value, rather than a comparison.
> > 
> > [0] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=678624e401b0b7747762b5223fb23f86dcdacc93
> > [1] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=f24219b4e90cf70ec4a211b17fbabc725a0ddf3c
> 
> Stephen please post a fix for this before a 32-bit sparc person
> loses their filesystem or worse.

Um, I could try to figure out how to do that, but what I did was respond
in July 29 to the message above cc'ing the author of the orignal patch in
the hope that they would write a fix.  Clearly that hope was in vain :-(

> The return value semantics have changed for this function, you
> can't just put two underscores in front of the name, export
> the symbol, and say "Ship it!" :-)

Well, I never actaully said that about my original build fix patch. :-(

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

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

Re: sparc: rename atomic_add_unless

[Josip Rodin]

On Thu, Aug 04, 2011 at 07:39:16PM +1000, Stephen Rothwell wrote:
> Hi Dave,
> 
> On Thu, 04 Aug 2011 01:21:46 -0700 (PDT) David Miller <davem@davemloft.net> wrote:
> >
> > From: Josip Rodin <joy@entuzijast.net>
> > Date: Thu, 28 Jul 2011 12:40:17 +0200
> > 
> > > This patch[0] seems to fix a build issue, but it doesn't fix the
> > > functionality - the previous change[1] changed the return value semantics,
> > > it's now supposed to be to be just the old value, rather than a comparison.
> > > 
> > > [0] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;hg8624e401b0b7747762b5223fb23f86dcdacc93
> > > [1] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;hò4219b4e90cf70ec4a211b17fbabc725a0ddf3c
> > 
> > Stephen please post a fix for this before a 32-bit sparc person
> > loses their filesystem or worse.
> 
> Um, I could try to figure out how to do that, but what I did was respond
> in July 29 to the message above cc'ing the author of the orignal patch in
> the hope that they would write a fix.  Clearly that hope was in vain :-(

It should be fairly trivial:

diff --git a/arch/sparc/lib/atomic32.c b/arch/sparc/lib/atomic32.c
index 8600eb2..1d32b54 100644
--- a/arch/sparc/lib/atomic32.c
+++ b/arch/sparc/lib/atomic32.c
@@ -65,7 +65,7 @@ int __atomic_add_unless(atomic_t *v, int a, int u)
        if (ret != u)
                v->counter += a;
        spin_unlock_irqrestore(ATOMIC_HASH(v), flags);
-       return ret != u;
+       return ret;
 }
 EXPORT_SYMBOL(__atomic_add_unless);

-- 
     2. That which causes joy or happiness.

Re: sparc: rename atomic_add_unless

[David Miller]

From: Josip Rodin <joy@entuzijast.net>
Date: Thu, 4 Aug 2011 11:43:15 +0200

> It should be fairly trivial:
> 
> diff --git a/arch/sparc/lib/atomic32.c b/arch/sparc/lib/atomic32.c
> index 8600eb2..1d32b54 100644
> --- a/arch/sparc/lib/atomic32.c
> +++ b/arch/sparc/lib/atomic32.c
> @@ -65,7 +65,7 @@ int __atomic_add_unless(atomic_t *v, int a, int u)
>         if (ret != u)
>                 v->counter += a;
>         spin_unlock_irqrestore(ATOMIC_HASH(v), flags);
> -       return ret != u;
> +       return ret;
>  }
>  EXPORT_SYMBOL(__atomic_add_unless);

Applied, thanks Josip.

Re: sparc: rename atomic_add_unless

[Arun Sharma]

On 8/4/11 2:39 AM, Stephen Rothwell wrote:

>> Stephen please post a fix for this before a 32-bit sparc person
>> loses their filesystem or worse.
>
> Um, I could try to figure out how to do that, but what I did was respond
> in July 29 to the message above cc'ing the author of the orignal patch in
> the hope that they would write a fix.  Clearly that hope was in vain :-(

I was on vacation for a few weeks and got buried under email. The patch 
looks right to me. Thanks for fixing Josip!

  -Arun