On Tue, Sep 10, 2013 at 09:34:52AM -0700, Linus Torvalds wrote:
> On Tue, Sep 10, 2013 at 6:56 AM, Ingo Molnar <mingo@xxxxxxxxxx> wrote:
> >
> > +static __always_inline bool __preempt_count_dec_and_test(void)
> > +{
> > + unsigned char c;
> > +
> > + asm ("decl " __percpu_arg(0) "; sete %1"
> > + : "+m" (__preempt_count), "=qm" (c));
> > +
> > + return c != 0;
> > +}
> >
> > And that's where the sete and test originates from.
>
> We could make this use "asm goto" instead.
>
> An "asm goto" cannot have outputs, but this particular one doesn't
> _need_ outputs. You could mark the preempt_count memory as an input,
> and then have a memory clobber. I think you need the memory clobber
> anyway for that preempt-count thing.
>
> So I _think_ something like
>
> static __always_inline bool __preempt_count_dec_and_test(void)
> {
> asm goto("decl " __percpu_arg(0) "\n\t"
> "je %l[became_zero]"
> : :"m" (__preempt_count):"memory":became_zero);
> return 0;
> became_zero:
> return 1;
> }
The usage site:
#define preempt_enable() \
do { \
barrier(); \
if (unlikely(preempt_count_dec_and_test())) \
__preempt_schedule(); \
} while (0)
Already includes the barrier explicitly, so do we still need the memory
clobber in that asm goto thing?
That said, your change results in:
ffffffff8106f420 <kick_process>:
ffffffff8106f420: 55 push %rbp
ffffffff8106f421: 65 ff 04 25 e0 b7 00 incl %gs:0xb7e0
ffffffff8106f428: 00
ffffffff8106f429: 48 89 e5 mov %rsp,%rbp
ffffffff8106f42c: 48 8b 47 08 mov 0x8(%rdi),%rax
ffffffff8106f430: 8b 50 18 mov 0x18(%rax),%edx
ffffffff8106f433: 65 8b 04 25 1c b0 00 mov %gs:0xb01c,%eax
ffffffff8106f43a: 00
ffffffff8106f43b: 39 c2 cmp %eax,%edx
ffffffff8106f43d: 74 1b je ffffffff8106f45a <kick_process+0x3a>
ffffffff8106f43f: 89 d1 mov %edx,%ecx
ffffffff8106f441: 48 c7 c0 00 2c 01 00 mov $0x12c00,%rax
ffffffff8106f448: 48 8b 0c cd a0 bc cb mov -0x7e344360(,%rcx,8),%rcx
ffffffff8106f44f: 81
ffffffff8106f450: 48 3b bc 08 00 08 00 cmp 0x800(%rax,%rcx,1),%rdi
ffffffff8106f457: 00
ffffffff8106f458: 74 26 je ffffffff8106f480 <kick_process+0x60>
* ffffffff8106f45a: 65 ff 0c 25 e0 b7 00 decl %gs:0xb7e0
ffffffff8106f461: 00
* ffffffff8106f462: 74 0c je ffffffff8106f470 <kick_process+0x50>
ffffffff8106f464: 5d pop %rbp
ffffffff8106f465: c3 retq
ffffffff8106f466: 66 2e 0f 1f 84 00 00 nopw %cs:0x0(%rax,%rax,1)
ffffffff8106f46d: 00 00 00
* ffffffff8106f470: e8 9b b6 f9 ff callq ffffffff8100ab10 <___preempt_schedule>
ffffffff8106f475: 5d pop %rbp
ffffffff8106f476: c3 retq
ffffffff8106f477: 66 0f 1f 84 00 00 00 nopw 0x0(%rax,%rax,1)
ffffffff8106f47e: 00 00
ffffffff8106f480: 89 d7 mov %edx,%edi
ffffffff8106f482: ff 15 b8 e0 ba 00 callq *0xbae0b8(%rip) # ffffffff81c1d540 <smp_ops+0x20>
ffffffff8106f488: eb d0 jmp ffffffff8106f45a <kick_process+0x3a>
ffffffff8106f48a: 66 0f 1f 44 00 00 nopw 0x0(%rax,%rax,1)
Which is indeed perfect. So should I go 'fix' the other _and_test()
functions we have to do this same thing?
--
To unsubscribe from this list: send the line "unsubscribe linux-arch" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
