lkml.org 
[lkml]   [2013]   [Sep]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 0/7] preempt_count rework -v2
On 09/10/2013 02:43 PM, Linus Torvalds wrote:
> On Tue, Sep 10, 2013 at 2:25 PM, Peter Zijlstra <peterz@infradead.org> wrote:
>>
>> Here's one that builds and boots on kvm until wanting to mount root.
>>
>> I'm not entirely sure on the "ir" vs "er" thing and atomic64_t and
>> local_t are inconsistent wrt that so I'm too.
>
> "i" is "any constant", while "e" is "32-bit signed constant".
>
> And I think all of the 64-bit ones should probably be "e", because
> afaik there is no way to add a 64-bit constant directly to memory (you
> have to load it into a register first).
>
> Of course, in reality, the constant is always just 1 or -1 or
> something like that, so nobody will ever notice the incorrect case...
>
> And it doesn't matter for the 32-bit cases, obviously, but we could
> just make them all be "e" for simplicity.
>
> That said, looking at your patch, I get the *very* strong feeling that
> we could make a macro that does all the repetitions for us, and then
> have a
>
> GENERATE_RMW(atomic_sub_and_test, LOCK_PREFIX "subl", "e", "")
> GENERATE_RMW(atomic_dec_and_test, LOCK_PREFIX "decl", "e", "")
> ..
> GENERATE_RMW(atomic_add_negative, LOCK_PREFIX "addl", "s", "")
>
> GENERATE_RMW(local_sub_and_test, "subl", "e", __percpu_prefix)
> ...
>
> etc.
>
> I'm sure the macro would be nasty as hell (and I bet it needs a few
> more arguments), but then we'd avoid the repetition..
>

Actually, the right thing here really is "er" (which I think you meant,
but just to make it clear.) Why? Even if the value is representable as
a signed immediate, if gcc already happens to have it in a register it
will be better to use the register.

"e" doesn't work on versions of gcc older than the first x86-64 release,
but we don't care about that anymore.

A final good question is if we should encapsulate the add/inc and
sub/dec into a single function; one could easily do somethin glike:

static inline int atomic_sub_and_test(int i, atomic_t *)
{
if (__builtin_constant_p(i) && i == 1)
/* Use incl */
else if (__builtin_constant_p(i) && i == -1)
/* Use decl */
else
/* Use addl */
}

-hpa



\
 
 \ /
  Last update: 2013-09-11 00:21    [W:0.092 / U:0.636 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site