Messages in this thread | | | Date | Fri, 27 Mar 2020 03:50:05 +0000 | From | Al Viro <> | 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@zeniv.linux.org.uk> wrote: > > > > From: Al Viro <viro@zeniv.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.
| |