[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20200327035005.GU23230@ZenIV.linux.org.uk>
Date: Fri, 27 Mar 2020 03:50:05 +0000
From: Al Viro <viro@...iv.linux.org.uk>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Thomas Gleixner <tglx@...utronix.de>,
the arch/x86 maintainers <x86@...nel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [RFC][PATCH v2 6/8] x86: don't reload after cmpxchg in
unsafe_atomic_op2() loop
On Thu, Mar 26, 2020 at 08:23:59PM -0700, Linus Torvalds wrote:
> On Thu, Mar 26, 2020 at 7:28 PM Al Viro <viro@...iv.linux.org.uk> wrote:
> >
> > From: Al Viro <viro@...iv.linux.org.uk>
> >
> > lock cmpxchg leaves the current value in eax; no need to reload it.
>
> I think this one is buggy.
>
> Patch edited to remove the "-" lines, so that you see the end result:
>
> > int oldval = 0, ret, tem; \
> > asm volatile("1:\tmovl %2, %0\n" \
> > + "2:\tmovl\t%0, %3\n" \
> > "\t" insn "\n" \
> > + "3:\t" LOCK_PREFIX "cmpxchgl %3, %2\n" \
> > + "\tjnz\t2b\n" \
> > + "4:\n" \
> > "\t.section .fixup,\"ax\"\n" \
> > + "5:\tmov\t%5, %1\n" \
> > "\tjmp\t3b\n" \
> > "\t.previous\n" \
> > + _ASM_EXTABLE_UA(1b, 5b) \
> > + _ASM_EXTABLE_UA(3b, 5b) \
> > : "=&a" (oldval), "=&r" (ret), \
> > "+m" (*uaddr), "=&r" (tem) \
> > : "r" (oparg), "i" (-EFAULT), "1" (0)); \
>
> I think that
>
> "\tjmp\t3b\n"
>
> line in the fixup section should be
>
> "\tjmp\t4b\n"
>
> because you don't want to jump to the cmpxchg instruction.
>
> Maybe I'm misreading it.
You are not - I'd missed that one while renumbering the labels. Fixed and
pushed.
Powered by blists - more mailing lists