On 9 April 2015 at 01:41, Thomas Gleixner <[email protected]> wrote: > I'm really not too excited about this incomprehensible macro mess and > especially not about the code it generates. > > x86_64 i386 ARM power > > Mainline 7668 6942 8077 10253 > > + Patch 8068 7294 8313 10861 > > +400 +352 +236 +608 > > That's insane.
After Peter's mail yesterday, I did check it on x86_64 and it surely looked a lot bigger. > What's wrong with just adding > > if (!(cpu_base->active_bases & (1 << i))) > continue; > > to the iterating sites? > > Index: linux/kernel/time/hrtimer.c > =================================================================== > --- linux.orig/kernel/time/hrtimer.c > +++ linux/kernel/time/hrtimer.c > @@ -451,6 +451,9 @@ static ktime_t __hrtimer_get_next_event( > struct timerqueue_node *next; > struct hrtimer *timer; > > + if (!(cpu_base->active_bases & (1 << i))) > + continue; > + > next = timerqueue_getnext(&base->active); > if (!next) > continue; Isn't the check we already have here lightweight enough for this ? timerqueue_getnext() returns head->next.. What benefit are we getting with this extra check ? Maybe we can drop 'active_bases' from struct hrtimer_cpu_base ? 'active_bases' can be used effectively, if we can quit early from this loop, i.e. by checking for !active_bases on every iteration. But that generates a lot more code and is probably not that helpful for small loop size that we have here. -- viresh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
