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.