Messages in this thread | | | Date | Sun, 29 Nov 2009 17:25:43 +0200 | From | Avi Kivity <> | Subject | Re: [PATCH] sched: Optimize branch hint in context_switch() |
| |
On 11/29/2009 05:20 PM, Peter Zijlstra wrote: > On Sun, 2009-11-29 at 17:12 +0200, Avi Kivity wrote: > >> On 11/29/2009 02:01 PM, Tim Blechmann wrote: >> >>> Branch hint profiling on my nehalem machine showed 88% >>> incorrect branch hints: >>> >>> 42017484 326957902 88 context_switch sched.c 3043 >>> 42038493 326953687 88 context_switch sched.c 3050 >>> >>> @@ -3040,14 +3040,14 @@ context_switch(struct rq *rq, struct task_struct *prev, >>> */ >>> arch_start_context_switch(prev); >>> >>> - if (likely(!mm)) { >>> + if (unlikely(!mm)) { >>> next->active_mm = oldmm; >>> atomic_inc(&oldmm->mm_count); >>> enter_lazy_tlb(oldmm, next); >>> } else >>> switch_mm(oldmm, mm, next); >>> >>> - if (likely(!prev->mm)) { >>> + if (unlikely(!prev->mm)) { >>> prev->active_mm = NULL; >>> rq->prev_mm = oldmm; >>> } >>> >>> >> I don't think either the original or the patch is correct. Whether or >> not a task has an mm is entirely workload dependent, we shouldn't be >> giving hints here. >> > There are reasons to still use branch hints, for example if the unlikely > branch is very expensive anyway and it pays to have the likely branch be > ever so slightly less expensive. > > Now I don't think that applies here, but there are cases where such code > generation issues are the main motivator not the actual usage patterns. >
These should be documented then to avoid patches removing them:
#define slowpath(x) unlikely(x)
if (slowpath(condition)) expensive_operation();
-- error compiling committee.c: too many arguments to function
| |