Messages in this thread | | | Date | Sun, 19 Dec 2010 10:43:46 +0100 | From | Tejun Heo <> | Subject | Re: [PATCH RFC tip/core/rcu 13/20] rcu: increase synchronize_sched_expedited() batching |
| |
Hello,
On 12/18/2010 09:14 PM, Paul E. McKenney wrote: >>> diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h >>> index 49e8e16..af56148 100644 >>> --- a/include/linux/rcupdate.h >>> +++ b/include/linux/rcupdate.h >>> @@ -47,6 +47,8 @@ >>> extern int rcutorture_runnable; /* for sysctl */ >>> #endif /* #ifdef CONFIG_RCU_TORTURE_TEST */ >>> >>> +#define UINT_CMP_GE(a, b) (UINT_MAX / 2 >= (a) - (b)) >>> +#define UINT_CMP_LT(a, b) (UINT_MAX / 2 < (a) - (b)) >>> #define ULONG_CMP_GE(a, b) (ULONG_MAX / 2 >= (a) - (b)) >>> #define ULONG_CMP_LT(a, b) (ULONG_MAX / 2 < (a) - (b)) >> >> I don't think the original comparison had overflow problem. (a) < (b) >> gives the wrong result on overflow but (int)((a) - (b)) < 0 is >> correct. > > You are right that it does give the correct result now, but the C > standard never has defined overflow for signed integers, as noted in > Section 6.3.1.3p3 of the N1494 Working Draft of the C standard: > > Otherwise, the new type is signed and the value cannot be > represented in it; either the result is implementation-defined > or an implementation-defined signal is raised. > > I have heard too many compiler guys gleefully discussing optimizations > that they could use if they took full advantage of this clause, so I > am not comfortable relying on the intuitive semantics for signed > arithmetic. (Now atomic_t is another story -- both C and C++ will > be requiring twos-complement semantics, thankfully.) > >> I find the latter approach cleaner and that way the constant in the >> instruction can be avoided too although if the compiler might generate >> the same code regardless. > > I would like your way better if it was defined in the C standard. > But it unfortunately is not. :-(
I see, then would something like the following work?
(int)((unsigned)(a) - (unsigned)(b)) < 0
>> Also, I think the names are misleading. They aren't testing whether >> one is greater or less than the other. They're testing whether one is >> before or after the other where the counters are used as monotonically >> incrementing (with wrapping) sequence, so wouldn't something like the >> following be better? > > They are comparing the twos-complement difference between the two > numbers against zero.
But still GE/LT are way too misleading. Anyways, so with the above change the macro now would look like the following.
#define SEQ_TEST(a, b, op) ({ \ typeof(a) __seq_a = (a); \ typeof(b) __seq_b = (b); \ bool __ret; \ (void)(&__seq_a == &__seq_b); \ switch (sizeof(__seq_a)) { \ case sizeof(s8): \ __ret = (s8)((u8)__seq_a - (u8)__seq_b) op 0; \ break; \ case sizeof(s16): \ __ret = (s16)((u16)__seq_a - (u16)__seq_b) op 0;\ break; \ case sizeof(s32): \ __ret = (s32)((u32)__seq_a - (u32)__seq_b) op 0;\ break; \ case sizeof(s64): \ __ret = (s64)((u64)__seq_a - (u64)__seq_b) op 0;\ break; \ default: \ __make_build_fail; \ } \ __ret; \ })
Would the above work?
Thanks.
-- tejun
| |