On 09/10/2013 02:43 PM, Linus Torvalds wrote:
> On Tue, Sep 10, 2013 at 2:25 PM, Peter Zijlstra <peterz@xxxxxxxxxxxxx> 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
--
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
