Messages in this thread | | | Date | Thu, 15 Oct 2015 20:34:14 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH v3 1/3] sched: introduce a new migration flag to task_struct |
| |
On Thu, Oct 15, 2015 at 09:26:44PM +0900, Byungchul Park wrote: > On Thu, Oct 15, 2015 at 01:18:13PM +0200, Peter Zijlstra wrote: > > On Thu, Oct 15, 2015 at 06:01:14PM +0900, byungchul.park@lge.com wrote: > > > From: Byungchul Park <byungchul.park@lge.com> > > > > > > This patch removes a weird coupling between se->avg.last_update_time and > > > the condition checking for migration, and introduce a new migration flag. > > > Now, scheduler can use the flag instead of se->avg.last_update_time to > > > check if migration already happened or not. > > > > Was there a problem with that coupling? This does not explain. > > The reason why i introduce the new flag is that 3/3 patch makes > se->avg.last_update_time non-zero consistently, so we cannot use the > condition "se->avg.last_update_time == 0" to check if migration has > happened.
It would have been ever so good to have had that in the Changelog.
> > > diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h > > > index af6f252..66d0552 100644 > > > --- a/kernel/sched/sched.h > > > +++ b/kernel/sched/sched.h > > > @@ -1158,6 +1158,7 @@ static const u32 prio_to_wmult[40] = { > > > #define ENQUEUE_WAKING 0 > > > #endif > > > #define ENQUEUE_REPLENISH 8 > > > +#define ENQUEUE_MIGRATED 16 > > > > Won't actually apply that.. > > Okay, I got your concern, let me think more..
Ah, so what I means was that that hunk will not apply. I see that what I typed did not express that as clearly as I might have wanted.
Current code looks like:
#define ENQUEUE_REPLENISH 0x08 #define ENQUEUE_RESTORE 0x10
> By the way, what do you think about the approach of 3/3 patch?
Again a wee bit of worry. I like it for 64bits as there its nearly free and does the right thing, but on 32bit we add 2 smp_rmb() invocations (which are not free on ARM etc..). [Note that one could rewrite that function to require but a single rmb].
| |