Messages in this thread | | | Date | Wed, 26 Mar 2014 18:31:18 +0100 (CET) | From | Thomas Gleixner <> | Subject | Re: [PATCH 04/14] hrtimer: use base->index instead of basenum in switch_hrtimer_base() |
| |
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.
| |