linux-mips
[Top] [All Lists]

Re: Bug in atomic_sub_if_positive

To: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
Subject: Re: Bug in atomic_sub_if_positive
From: Thiemo Seufer <ths@networkno.de>
Date: Wed, 2 Jul 2008 11:51:55 +0100
Cc: mlarsen@broadcom.com, linux-mips@linux-mips.org
In-reply-to: <20080702.193133.211490377.nemoto@toshiba-tops.co.jp>
Original-recipient: rfc822;linux-mips@linux-mips.org
References: <ADD7831BD377A74E9A1621D1EAAED18F0450AC61@NT-SJCA-0750.brcm.ad.broadcom.com> <20080702095955.GA7007@networkno.de> <20080702.193133.211490377.nemoto@toshiba-tops.co.jp>
Sender: linux-mips-bounce@linux-mips.org
User-agent: Mutt/1.5.18 (2008-05-17)
Atsushi Nemoto wrote:
> On Wed, 2 Jul 2008 10:59:56 +0100, Thiemo Seufer <ths@networkno.de> wrote:
> > > --- a/include/asm-mips/atomic.h   2008-06-25 22:38:43.159739000 -0700
> > > +++ b/include/asm-mips/atomic.h   2008-06-25 22:39:07.552065000 -0700
> > > @@ -292,10 +292,10 @@ static __inline__ int atomic_sub_if_posi
> > >           "       beqz    %0, 2f                                  \n"
> > >           "        subu   %0, %1, %3                              \n"
> > >           "       .set    reorder                                 \n"
> > > -         "1:                                                     \n"
> > >           "       .subsection 2                                   \n"
> > >           "2:     b       1b                                      \n"
> > >           "       .previous                                       \n"
> > > +         "1:                                                     \n"
> > 
> > AFAICS this change should make no difference to the generated code. I
> > suspect you assembler handles .subsection incorrectly. Can you provide
> > a disassembled exapmle which gets altered by this patch? Also, please
> > tell us the exact version of the assembler you use.
> 
> Why no defference?  The '1b' in subsection refer backword '1' label on
> the source code (which is a label for LL insn in this case with the
> patch) ?

Oh. Now that you spelled it out I see it, too. :-)

> Anyway I can provide them.  I'm using vanilla 2.17 and 2.18.
> 
> Without the patch:
> 801233bc <try_acquire_console_sem>:
> 801233bc:     lui     v0,0x8038
> 801233c0:     ll      a0,31976(v0)
> 801233c4:     addiu   v1,a0,-1
> 801233c8:     bltz    v1,801233dc <try_acquire_console_sem+0x20>
> 801233cc:     nop
> 801233d0:     sc      v1,31976(v0)
> 801233d4:     beqz    v1,80124dac <sys_syslog+0x8>
> 801233d8:     addiu   v1,a0,-1
> 801233dc:     bltz    v1,801233fc <try_acquire_console_sem+0x40>
> 801233e0:     li      v0,-1
> ...
> 80124dac:     b       801233dc <try_acquire_console_sem+0x20>
> 80124db0:     nop
> 
> With the patch:
> 801233bc <try_acquire_console_sem>:
> 801233bc:     lui     v0,0x8038
> 801233c0:     ll      a0,31976(v0)
> 801233c4:     addiu   v1,a0,-1
> 801233c8:     bltz    v1,801233dc <try_acquire_console_sem+0x20>
> 801233cc:     nop
> 801233d0:     sc      v1,31976(v0)
> 801233d4:     beqz    v1,80124dac <sys_syslog+0x8>
> 801233d8:     addiu   v1,a0,-1
> 801233dc:     bltz    v1,801233fc <try_acquire_console_sem+0x40>
> 801233e0:     li      v0,-1
> ...
> 80124dac:     b       801233c0 <try_acquire_console_sem+0x4>
> 80124db0:     nop
> 
> 
> The patch looks correct.

Agreed.


Thiemo

<Prev in Thread] Current Thread [Next in Thread>