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. -- 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/
