On 03/26/2014 11:01 PM, Thomas Gleixner wrote: > On Wed, 26 Mar 2014, Srivatsa S. Bhat wrote: >> On 03/26/2014 04:51 PM, Viresh Kumar wrote: >>> In switch_hrtimer_base() we have created a local variable basenum which is >>> set >>> to base->index. This variable is used at only one place. It makes code more >>> readable if we remove this variable use base->index directly. >>> >> >> No, this doesn't look right. Note that the code can re-execute >> the assignment to new_base, by jumping to the 'again' label. >> See below. >> >>> --- a/kernel/hrtimer.c >>> +++ b/kernel/hrtimer.c >>> @@ -202,11 +202,10 @@ switch_hrtimer_base(struct hrtimer *timer, struct >>> hrtimer_clock_base *base, >>> struct hrtimer_cpu_base *new_cpu_base; >>> int this_cpu = smp_processor_id(); >>> int cpu = get_nohz_timer_target(pinned); >>> - int basenum = base->index; >>> >>> again: >>> new_cpu_base = &per_cpu(hrtimer_bases, cpu); >>> - new_base = &new_cpu_base->clock_base[basenum]; >>> + new_base = &new_cpu_base->clock_base[base->index]; >>> >> >> Further down, timer->base can be altered (and set to NULL too). >> So if we jump back to 'again', we'll end up in trouble. >> So I think its important to cache the value in basenum and >> use it. > > That's irrelevant. base is not changing. >
Sorry, I missed that :-( Regards, Srivatsa S. Bhat -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
