linux-arch.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>, Andi Kleen <ak@linux.intel.com>,
	Peter Anvin <hpa@zytor.com>, Mike Galbraith <bitbucket@online.de>,
	Thomas Gleixner <tglx@linutronix.de>,
	Arjan van de Ven <arjan@linux.intel.com>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	"linux-arch@vger.kernel.org" <linux-arch@vger.kernel.org>
Subject: Re: [PATCH 0/7] preempt_count rework -v2
Date: Wed, 11 Sep 2013 19:43:42 -0700	[thread overview]
Message-ID: <CA+55aFwdS5-U8UtjZ8qsKyn7ap05MPXMFeU-GXgU2PE5KrCrHg@mail.gmail.com> (raw)
In-Reply-To: <20130912022040.GT31370@twins.programming.kicks-ass.net>

On Wed, Sep 11, 2013 at 7:20 PM, Peter Zijlstra <peterz@infradead.org> wrote:
>
> I split the thing up into two macros GEN_UNARY_RMWcc and
> GEN_BINARY_RMWcc which ends up being more readable as well as smaller
> code overall.

Yes, that looks like the right abstraction level. Powerful without
being complicated.

> I also attempted to convert asm/bitops.h, although I'm not sure it'll
> compile right with older GCCs due to the comment near BITOP_ADDR()

Actually, since you now have that memory clobber, it no longer
matters, and "m" is the right thing.

That said, the very same memory clobber may be what makes this whole
approach a loss, if it causes gcc to do tons of reloads or other
random things.

That memory clobber wasn't an issue for the whole
__preempt_count_dec_and_test() thing, because there we needed to keep
anything before the decrement contained anyway.

For other cases? Who knows.. A lot of the "change and test atomically"
things have the same containment need, so it might not be a problem.

> I might have to add the clobber to the macro arguments so we can do
> version without "memory" clobber, although bitops is inconsistent with
> that as well, __test_and_clear_bit() doesn't have a memory clobber but
> __test_and_change_bit() does.

You do need the memory clobber. The "asm goto" version can not have
outputs, so "=m" and "+m" are both no-go, and so the only thing left
is that memory clobber, as far as I can see..

The bitops didn't use to need it, because they had that "+/=m" that
already tells gcc that the target is changed.

           Linus

  reply	other threads:[~2013-09-12  2:43 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-10 13:08 [PATCH 0/7] preempt_count rework -v2 Peter Zijlstra
2013-09-10 13:08 ` Peter Zijlstra
2013-09-10 13:08 ` [PATCH 1/7] sched: Introduce preempt_count accessor functions Peter Zijlstra
2013-09-10 13:08   ` Peter Zijlstra
2013-09-10 13:08 ` [PATCH 2/7] sched: Add NEED_RESCHED to the preempt_count Peter Zijlstra
2013-09-11  1:59   ` Andy Lutomirski
2013-09-11  1:59     ` Andy Lutomirski
2013-09-11  8:25     ` Peter Zijlstra
2013-09-11 11:06       ` Peter Zijlstra
2013-09-11 13:34         ` Mike Galbraith
2013-09-12  6:01           ` Mike Galbraith
2013-09-11 16:35         ` Andy Lutomirski
2013-09-11 16:35           ` Andy Lutomirski
2013-09-11 18:05           ` Peter Zijlstra
2013-09-11 18:07             ` Andy Lutomirski
2013-09-11 11:14   ` Peter Zijlstra
2013-09-11 11:14     ` Peter Zijlstra
2013-09-10 13:08 ` [PATCH 3/7] sched, arch: Create asm/preempt.h Peter Zijlstra
2013-09-10 13:08   ` Peter Zijlstra
2013-09-10 13:08 ` [PATCH 4/7] sched: Create more preempt_count accessors Peter Zijlstra
2013-09-10 13:08   ` Peter Zijlstra
2013-09-10 13:08 ` [PATCH 5/7] sched: Extract the basic add/sub preempt_count modifiers Peter Zijlstra
2013-09-10 13:08   ` Peter Zijlstra
2013-09-10 13:08 ` [PATCH 6/7] sched, x86: Provide a per-cpu preempt_count implementation Peter Zijlstra
2013-09-10 13:08   ` Peter Zijlstra
2013-09-10 13:27   ` Peter Zijlstra
2013-09-10 14:02   ` Eric Dumazet
2013-09-10 15:25     ` Peter Zijlstra
2013-09-10 16:48   ` Peter Zijlstra
2013-09-10 13:08 ` [PATCH 7/7] sched, x86: Optimize the preempt_schedule() call Peter Zijlstra
2013-09-10 13:42   ` Ingo Molnar
2013-09-10 13:42     ` Ingo Molnar
2013-09-10 13:55     ` Jan Beulich
2013-09-10 14:25       ` Ingo Molnar
2013-09-10 13:51 ` [PATCH 0/7] preempt_count rework -v2 Ingo Molnar
2013-09-10 13:51   ` Ingo Molnar
2013-09-10 13:56   ` Ingo Molnar
2013-09-10 15:14     ` Peter Zijlstra
2013-09-10 15:29     ` Arjan van de Ven
2013-09-10 15:35       ` Peter Zijlstra
2013-09-10 16:24       ` Linus Torvalds
2013-09-11 16:00         ` H. Peter Anvin
2013-09-10 16:34     ` Linus Torvalds
2013-09-10 16:34       ` Linus Torvalds
2013-09-10 16:45       ` Peter Zijlstra
2013-09-10 17:06         ` Linus Torvalds
2013-09-10 21:25           ` Peter Zijlstra
2013-09-10 21:25             ` Peter Zijlstra
2013-09-10 21:43             ` Linus Torvalds
2013-09-10 21:51               ` H. Peter Anvin
2013-09-10 22:02                 ` Linus Torvalds
2013-09-10 22:06                   ` H. Peter Anvin
2013-09-11 13:13               ` Peter Zijlstra
2013-09-11 13:26                 ` Peter Zijlstra
2013-09-11 13:26                   ` Peter Zijlstra
2013-09-11 15:29                 ` H. Peter Anvin
2013-09-11 15:33                 ` Linus Torvalds
2013-09-11 15:33                   ` Linus Torvalds
2013-09-11 18:59                   ` Peter Zijlstra
2013-09-11 23:02                     ` Linus Torvalds
2013-09-12  2:20                       ` Peter Zijlstra
2013-09-12  2:43                         ` Linus Torvalds [this message]
2013-09-12 11:51                           ` Peter Zijlstra
2013-09-12 12:25                             ` Ingo Molnar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CA+55aFwdS5-U8UtjZ8qsKyn7ap05MPXMFeU-GXgU2PE5KrCrHg@mail.gmail.com \
    --to=torvalds@linux-foundation.org \
    --cc=ak@linux.intel.com \
    --cc=arjan@linux.intel.com \
    --cc=bitbucket@online.de \
    --cc=fweisbec@gmail.com \
    --cc=hpa@zytor.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).