Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1210107 > unrolled thread

Re: [PATCH v3 0/5] sync a se with its cfs_rq when att(det)aching it

Started byPeter Zijlstra <peterz@infradead.org>
First post2015-08-20 03:20 +0200
Last post2015-08-21 02:10 +0200
Articles 6 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH v3 0/5] sync a se with its cfs_rq when att(det)aching it Peter Zijlstra <peterz@infradead.org> - 2015-08-20 03:20 +0200
    Re: [PATCH v3 0/5] sync a se with its cfs_rq when att(det)aching it Byungchul Park <byungchul.park@lge.com> - 2015-08-20 07:30 +0200
    Re: [PATCH v3 0/5] sync a se with its cfs_rq when att(det)aching it Byungchul Park <byungchul.park@lge.com> - 2015-08-20 10:40 +0200
      Re: [PATCH v3 0/5] sync a se with its cfs_rq when att(det)aching it Byungchul Park <byungchul.park@lge.com> - 2015-08-20 12:50 +0200
        Re: [PATCH v3 0/5] sync a se with its cfs_rq when att(det)aching it Peter Zijlstra <peterz@infradead.org> - 2015-08-20 23:20 +0200
          Re: [PATCH v3 0/5] sync a se with its cfs_rq when att(det)aching it Byungchul Park <byungchul.park@lge.com> - 2015-08-21 02:10 +0200

#1210107 — Re: [PATCH v3 0/5] sync a se with its cfs_rq when att(det)aching it

FromPeter Zijlstra <peterz@infradead.org>
Date2015-08-20 03:20 +0200
SubjectRe: [PATCH v3 0/5] sync a se with its cfs_rq when att(det)aching it
Message-ID<pZmGm-6kZ-5@gated-at.bofh.it>
I did something like this on top.. please have a look at the XXX and
integrate.

---

--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -2664,8 +2664,8 @@ static inline u64 cfs_rq_clock_task(stru
 /* Group cfs_rq's load_avg is used for task_h_load and update_cfs_share */
 static inline int update_cfs_rq_load_avg(u64 now, struct cfs_rq *cfs_rq)
 {
-	int decayed;
 	struct sched_avg *sa = &cfs_rq->avg;
+	int decayed;
 
 	if (atomic_long_read(&cfs_rq->removed_load_avg)) {
 		long r = atomic_long_xchg(&cfs_rq->removed_load_avg, 0);
@@ -2695,15 +2695,16 @@ static inline int update_cfs_rq_load_avg
 static inline void update_load_avg(struct sched_entity *se, int update_tg)
 {
 	struct cfs_rq *cfs_rq = cfs_rq_of(se);
-	int cpu = cpu_of(rq_of(cfs_rq));
 	u64 now = cfs_rq_clock_task(cfs_rq);
+	int cpu = cpu_of(rq_of(cfs_rq));
 
 	/*
 	 * Track task load average for carrying it to new CPU after migrated, and
 	 * track group sched_entity load average for task_h_load calc in migration
 	 */
 	__update_load_avg(now, cpu, &se->avg,
-		se->on_rq * scale_load_down(se->load.weight), cfs_rq->curr == se, NULL);
+			  se->on_rq * scale_load_down(se->load.weight),
+			  cfs_rq->curr == se, NULL);
 
 	if (update_cfs_rq_load_avg(now, cfs_rq) && update_tg)
 		update_tg_load_avg(cfs_rq, 0);
@@ -2718,9 +2719,10 @@ static void attach_entity_load_avg(struc
 	 * update here. we have to update it with prev cfs_rq just before changing
 	 * se's cfs_rq, and get here soon.
 	 */
-	if (se->avg.last_update_time)
+	if (se->avg.last_update_time) {
 		__update_load_avg(cfs_rq->avg.last_update_time, cpu_of(rq_of(cfs_rq)),
-				&se->avg, 0, 0, NULL);
+				  &se->avg, 0, 0, NULL);
+	}
 
 	se->avg.last_update_time = cfs_rq->avg.last_update_time;
 	cfs_rq->avg.load_avg += se->avg.load_avg;
@@ -2732,17 +2734,13 @@ static void attach_entity_load_avg(struc
 static void detach_entity_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se)
 {
 	__update_load_avg(cfs_rq->avg.last_update_time, cpu_of(rq_of(cfs_rq)),
-			&se->avg, se->on_rq * scale_load_down(se->load.weight),
-			cfs_rq->curr == se, NULL);
+			  &se->avg, se->on_rq * scale_load_down(se->load.weight),
+			  cfs_rq->curr == se, NULL);
 
-	cfs_rq->avg.load_avg =
-		max_t(long, cfs_rq->avg.load_avg - se->avg.load_avg, 0);
-	cfs_rq->avg.load_sum =
-		max_t(s64, cfs_rq->avg.load_sum - se->avg.load_sum, 0);
-	cfs_rq->avg.util_avg =
-		max_t(long, cfs_rq->avg.util_avg - se->avg.util_avg, 0);
-	cfs_rq->avg.util_sum =
-		max_t(s32, cfs_rq->avg.util_sum - se->avg.util_sum, 0);
+	cfs_rq->avg.load_avg = max_t(long, cfs_rq->avg.load_avg - se->avg.load_avg, 0);
+	cfs_rq->avg.load_sum = max_t(s64,  cfs_rq->avg.load_sum - se->avg.load_sum, 0);
+	cfs_rq->avg.util_avg = max_t(long, cfs_rq->avg.util_avg - se->avg.util_avg, 0);
+	cfs_rq->avg.util_sum = max_t(s32,  cfs_rq->avg.util_sum - se->avg.util_sum, 0);
 }
 
 /* Add the load generated by se into cfs_rq's load average */
@@ -2751,14 +2749,14 @@ enqueue_entity_load_avg(struct cfs_rq *c
 {
 	struct sched_avg *sa = &se->avg;
 	u64 now = cfs_rq_clock_task(cfs_rq);
-	int migrated = 0, decayed;
+	int migrated, decayed;
 
-	if (sa->last_update_time == 0)
-		migrated = 1;
-	else
+	migrated = !sa->last_update_time;
+	if (!migrated) {
 		__update_load_avg(now, cpu_of(rq_of(cfs_rq)), sa,
 			se->on_rq * scale_load_down(se->load.weight),
 			cfs_rq->curr == se, NULL);
+	}
 
 	decayed = update_cfs_rq_load_avg(now, cfs_rq);
 
@@ -2781,7 +2779,7 @@ dequeue_entity_load_avg(struct cfs_rq *c
 	cfs_rq->runnable_load_avg =
 		max_t(long, cfs_rq->runnable_load_avg - se->avg.load_avg, 0);
 	cfs_rq->runnable_load_sum =
-		max_t(s64, cfs_rq->runnable_load_sum - se->avg.load_sum, 0);
+		max_t(s64,  cfs_rq->runnable_load_sum - se->avg.load_sum, 0);
 }
 
 /*
@@ -2849,6 +2847,11 @@ static inline void
 dequeue_entity_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se) {}
 static inline void remove_entity_load_avg(struct sched_entity *se) {}
 
+static inline void
+attach_entity_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se) {}
+static inline void
+detach_entity_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se) {}
+
 static inline int idle_balance(struct rq *rq)
 {
 	return 0;
@@ -7915,29 +7918,50 @@ prio_changed_fair(struct rq *rq, struct
 		check_preempt_curr(rq, p, 0);
 }
 
-static void detach_task_cfs_rq(struct task_struct *p, int queued)
+static inline bool vruntime_normalized(struct task_struct *p)
 {
+	int queued = task_on_rq_queued(p);
 	struct sched_entity *se = &p->se;
-	struct cfs_rq *cfs_rq = cfs_rq_of(se);
 
 	/*
-	 * Ensure the task's vruntime is normalized, so that after attaching
-	 * back it to a cfs_rq the enqueue_entity() will do the right thing.
-	 *
 	 * If it's queued, then the dequeue_entity(.flags=0) will already
-	 * have normalized the vruntime, if it's !queued, then only when
-	 * the task is sleeping it still has a non-normalized vruntime.
-	 *
+	 * have normalized the vruntime.
+	 */
+	if (queued)
+		return true;
+
+	/*
 	 * When !queued, vruntime of the task has usually NOT been normalized.
 	 * But there are some cases where it has already been normalized:
-	 *
-	 * - Moving a forked child which is waiting for being woken up by
+	 */
+
+	/*
+	 * - a forked child which is waiting for being woken up by
 	 *   wake_up_new_task().
-	 * - Moving a task which has been woken up by try_to_wake_up() and
+	 */
+	if (!se->sum_exec_runtime || p->state == TASK_WAKING)
+		return true;
+
+	/*
+	 * - a task which has been woken up by try_to_wake_up() and
 	 *   waiting for actually being woken up by sched_ttwu_pending().
+	 *
+	 * XXX this appears wrong!! check history, 
+	 * we appear to always set queued and RUNNING under the same lock instance
+	 * might be from before TASK_WAKING ?
 	 */
-	if (!queued && p->state != TASK_RUNNING &&
-	    se->sum_exec_runtime && p->state != TASK_WAKING) {
+	if (p->state == TASK_RUNNING)
+		return true;
+
+	return false;
+}
+
+static void detach_task_cfs_rq(struct task_struct *p)
+{
+	struct sched_entity *se = &p->se;
+	struct cfs_rq *cfs_rq = cfs_rq_of(se);
+
+	if (!vruntime_normalized(p)) {
 		/*
 		 * Fix up our vruntime so that the current sleep doesn't
 		 * cause 'unlimited' sleep bonus.
@@ -7946,10 +7970,8 @@ static void detach_task_cfs_rq(struct ta
 		se->vruntime -= cfs_rq->min_vruntime;
 	}
 
-#ifdef CONFIG_SMP
 	/* Catch up with the cfs_rq and remove our load when we leave */
 	detach_entity_load_avg(cfs_rq, se);
-#endif
 }
 
 static void attach_task_cfs_rq(struct task_struct *p, int queued)
@@ -7965,49 +7987,23 @@ static void attach_task_cfs_rq(struct ta
 	se->depth = se->parent ? se->parent->depth + 1 : 0;
 #endif
 
-#ifdef CONFIG_SMP
 	/* synchronize task with its cfs_rq */
 	attach_entity_load_avg(cfs_rq, se);
-#endif
 
-	/*
-	 * Ensure the task has a non-normalized vruntime when attaching it
-	 * to a cfs_rq, so that enqueue_entity() at wake-up time will do
-	 * the right thing.
-	 *
-	 * If it's queued, then the enqueue_entity(.flags=0) makes the task
-	 * has non-normalized vruntime, if it's !queued, then it still has
-	 * a normalized vruntime.
-	 *
-	 * When !queued, a task usually should have a non-normalized vruntime
-	 * after being attached to a cfs_rq. But there are some cases where
-	 * we should keep it normalized:
-	 *
-	 * - Moving a forked child which is waiting for being woken up by
-	 *   wake_up_new_task().
-	 * - Moving a task which has been woken up by try_to_wake_up() and
-	 *   waiting for actually being woken up by sched_ttwu_pending().
-	 */
-	if (!queued && p->state != TASK_RUNNING &&
-	    se->sum_exec_runtime && p->state != TASK_WAKING)
+	if (!vruntime_normalized(p))
 		se->vruntime += cfs_rq->min_vruntime;
 }
 
 static void switched_from_fair(struct rq *rq, struct task_struct *p)
 {
-	detach_task_cfs_rq(p, task_on_rq_queued(p));
+	detach_task_cfs_rq(p);
 }
 
-/*
- * We switched to the sched_fair class.
- */
 static void switched_to_fair(struct rq *rq, struct task_struct *p)
 {
-	int queued = task_on_rq_queued(p);
-
-	attach_task_cfs_rq(p, queued);
+	attach_task_cfs_rq(p);
 
-	if (queued) {
+	if (task_on_rq_queued(p)) {
 		/*
 		 * We were most likely switched from sched_rt, so
 		 * kick off the schedule if running, otherwise just see
@@ -8056,8 +8052,7 @@ static void task_move_group_fair(struct
 {
 	detach_task_cfs_rq(p, queued);
 	set_task_rq(p, task_cpu(p));
-
-	/* tell se's cfs_rq has been changed */
+	/* tell se's cfs_rq has been changed -- migrated */
 	p->se.avg.last_update_time = 0;
 	attach_task_cfs_rq(p, queued);
 }
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1210147

FromByungchul Park <byungchul.park@lge.com>
Date2015-08-20 07:30 +0200
Message-ID<pZqAi-3Bi-5@gated-at.bofh.it>
In reply to#1210107
On Thu, Aug 20, 2015 at 03:17:21AM +0200, Peter Zijlstra wrote:
> 
> I did something like this on top.. please have a look at the XXX and
> integrate.

yes, i will do it.

> 
> ---
> 
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -2664,8 +2664,8 @@ static inline u64 cfs_rq_clock_task(stru
>  /* Group cfs_rq's load_avg is used for task_h_load and update_cfs_share */
>  static inline int update_cfs_rq_load_avg(u64 now, struct cfs_rq *cfs_rq)
>  {
> -	int decayed;
>  	struct sched_avg *sa = &cfs_rq->avg;
> +	int decayed;
>  
>  	if (atomic_long_read(&cfs_rq->removed_load_avg)) {
>  		long r = atomic_long_xchg(&cfs_rq->removed_load_avg, 0);
> @@ -2695,15 +2695,16 @@ static inline int update_cfs_rq_load_avg
>  static inline void update_load_avg(struct sched_entity *se, int update_tg)
>  {
>  	struct cfs_rq *cfs_rq = cfs_rq_of(se);
> -	int cpu = cpu_of(rq_of(cfs_rq));
>  	u64 now = cfs_rq_clock_task(cfs_rq);
> +	int cpu = cpu_of(rq_of(cfs_rq));
>  
>  	/*
>  	 * Track task load average for carrying it to new CPU after migrated, and
>  	 * track group sched_entity load average for task_h_load calc in migration
>  	 */
>  	__update_load_avg(now, cpu, &se->avg,
> -		se->on_rq * scale_load_down(se->load.weight), cfs_rq->curr == se, NULL);
> +			  se->on_rq * scale_load_down(se->load.weight),
> +			  cfs_rq->curr == se, NULL);
>  
>  	if (update_cfs_rq_load_avg(now, cfs_rq) && update_tg)
>  		update_tg_load_avg(cfs_rq, 0);
> @@ -2718,9 +2719,10 @@ static void attach_entity_load_avg(struc
>  	 * update here. we have to update it with prev cfs_rq just before changing
>  	 * se's cfs_rq, and get here soon.
>  	 */
> -	if (se->avg.last_update_time)
> +	if (se->avg.last_update_time) {
>  		__update_load_avg(cfs_rq->avg.last_update_time, cpu_of(rq_of(cfs_rq)),
> -				&se->avg, 0, 0, NULL);
> +				  &se->avg, 0, 0, NULL);
> +	}
>  
>  	se->avg.last_update_time = cfs_rq->avg.last_update_time;
>  	cfs_rq->avg.load_avg += se->avg.load_avg;
> @@ -2732,17 +2734,13 @@ static void attach_entity_load_avg(struc
>  static void detach_entity_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se)
>  {
>  	__update_load_avg(cfs_rq->avg.last_update_time, cpu_of(rq_of(cfs_rq)),
> -			&se->avg, se->on_rq * scale_load_down(se->load.weight),
> -			cfs_rq->curr == se, NULL);
> +			  &se->avg, se->on_rq * scale_load_down(se->load.weight),
> +			  cfs_rq->curr == se, NULL);
>  
> -	cfs_rq->avg.load_avg =
> -		max_t(long, cfs_rq->avg.load_avg - se->avg.load_avg, 0);
> -	cfs_rq->avg.load_sum =
> -		max_t(s64, cfs_rq->avg.load_sum - se->avg.load_sum, 0);
> -	cfs_rq->avg.util_avg =
> -		max_t(long, cfs_rq->avg.util_avg - se->avg.util_avg, 0);
> -	cfs_rq->avg.util_sum =
> -		max_t(s32, cfs_rq->avg.util_sum - se->avg.util_sum, 0);
> +	cfs_rq->avg.load_avg = max_t(long, cfs_rq->avg.load_avg - se->avg.load_avg, 0);
> +	cfs_rq->avg.load_sum = max_t(s64,  cfs_rq->avg.load_sum - se->avg.load_sum, 0);
> +	cfs_rq->avg.util_avg = max_t(long, cfs_rq->avg.util_avg - se->avg.util_avg, 0);
> +	cfs_rq->avg.util_sum = max_t(s32,  cfs_rq->avg.util_sum - se->avg.util_sum, 0);
>  }
>  
>  /* Add the load generated by se into cfs_rq's load average */
> @@ -2751,14 +2749,14 @@ enqueue_entity_load_avg(struct cfs_rq *c
>  {
>  	struct sched_avg *sa = &se->avg;
>  	u64 now = cfs_rq_clock_task(cfs_rq);
> -	int migrated = 0, decayed;
> +	int migrated, decayed;
>  
> -	if (sa->last_update_time == 0)
> -		migrated = 1;
> -	else
> +	migrated = !sa->last_update_time;
> +	if (!migrated) {
>  		__update_load_avg(now, cpu_of(rq_of(cfs_rq)), sa,
>  			se->on_rq * scale_load_down(se->load.weight),
>  			cfs_rq->curr == se, NULL);
> +	}
>  
>  	decayed = update_cfs_rq_load_avg(now, cfs_rq);
>  
> @@ -2781,7 +2779,7 @@ dequeue_entity_load_avg(struct cfs_rq *c
>  	cfs_rq->runnable_load_avg =
>  		max_t(long, cfs_rq->runnable_load_avg - se->avg.load_avg, 0);
>  	cfs_rq->runnable_load_sum =
> -		max_t(s64, cfs_rq->runnable_load_sum - se->avg.load_sum, 0);
> +		max_t(s64,  cfs_rq->runnable_load_sum - se->avg.load_sum, 0);
>  }
>  
>  /*
> @@ -2849,6 +2847,11 @@ static inline void
>  dequeue_entity_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se) {}
>  static inline void remove_entity_load_avg(struct sched_entity *se) {}
>  
> +static inline void
> +attach_entity_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se) {}
> +static inline void
> +detach_entity_load_avg(struct cfs_rq *cfs_rq, struct sched_entity *se) {}
> +
>  static inline int idle_balance(struct rq *rq)
>  {
>  	return 0;
> @@ -7915,29 +7918,50 @@ prio_changed_fair(struct rq *rq, struct
>  		check_preempt_curr(rq, p, 0);
>  }
>  
> -static void detach_task_cfs_rq(struct task_struct *p, int queued)
> +static inline bool vruntime_normalized(struct task_struct *p)
>  {
> +	int queued = task_on_rq_queued(p);
>  	struct sched_entity *se = &p->se;
> -	struct cfs_rq *cfs_rq = cfs_rq_of(se);
>  
>  	/*
> -	 * Ensure the task's vruntime is normalized, so that after attaching
> -	 * back it to a cfs_rq the enqueue_entity() will do the right thing.
> -	 *
>  	 * If it's queued, then the dequeue_entity(.flags=0) will already
> -	 * have normalized the vruntime, if it's !queued, then only when
> -	 * the task is sleeping it still has a non-normalized vruntime.
> -	 *
> +	 * have normalized the vruntime.
> +	 */
> +	if (queued)
> +		return true;
> +
> +	/*
>  	 * When !queued, vruntime of the task has usually NOT been normalized.
>  	 * But there are some cases where it has already been normalized:
> -	 *
> -	 * - Moving a forked child which is waiting for being woken up by
> +	 */
> +
> +	/*
> +	 * - a forked child which is waiting for being woken up by
>  	 *   wake_up_new_task().
> -	 * - Moving a task which has been woken up by try_to_wake_up() and
> +	 */
> +	if (!se->sum_exec_runtime || p->state == TASK_WAKING)
> +		return true;
> +
> +	/*
> +	 * - a task which has been woken up by try_to_wake_up() and
>  	 *   waiting for actually being woken up by sched_ttwu_pending().
> +	 *
> +	 * XXX this appears wrong!! check history, 
> +	 * we appear to always set queued and RUNNING under the same lock instance
> +	 * might be from before TASK_WAKING ?
>  	 */
> -	if (!queued && p->state != TASK_RUNNING &&
> -	    se->sum_exec_runtime && p->state != TASK_WAKING) {
> +	if (p->state == TASK_RUNNING)
> +		return true;
> +
> +	return false;
> +}
> +
> +static void detach_task_cfs_rq(struct task_struct *p)
> +{
> +	struct sched_entity *se = &p->se;
> +	struct cfs_rq *cfs_rq = cfs_rq_of(se);
> +
> +	if (!vruntime_normalized(p)) {
>  		/*
>  		 * Fix up our vruntime so that the current sleep doesn't
>  		 * cause 'unlimited' sleep bonus.
> @@ -7946,10 +7970,8 @@ static void detach_task_cfs_rq(struct ta
>  		se->vruntime -= cfs_rq->min_vruntime;
>  	}
>  
> -#ifdef CONFIG_SMP
>  	/* Catch up with the cfs_rq and remove our load when we leave */
>  	detach_entity_load_avg(cfs_rq, se);
> -#endif
>  }
>  
>  static void attach_task_cfs_rq(struct task_struct *p, int queued)
> @@ -7965,49 +7987,23 @@ static void attach_task_cfs_rq(struct ta
>  	se->depth = se->parent ? se->parent->depth + 1 : 0;
>  #endif
>  
> -#ifdef CONFIG_SMP
>  	/* synchronize task with its cfs_rq */
>  	attach_entity_load_avg(cfs_rq, se);
> -#endif
>  
> -	/*
> -	 * Ensure the task has a non-normalized vruntime when attaching it
> -	 * to a cfs_rq, so that enqueue_entity() at wake-up time will do
> -	 * the right thing.
> -	 *
> -	 * If it's queued, then the enqueue_entity(.flags=0) makes the task
> -	 * has non-normalized vruntime, if it's !queued, then it still has
> -	 * a normalized vruntime.
> -	 *
> -	 * When !queued, a task usually should have a non-normalized vruntime
> -	 * after being attached to a cfs_rq. But there are some cases where
> -	 * we should keep it normalized:
> -	 *
> -	 * - Moving a forked child which is waiting for being woken up by
> -	 *   wake_up_new_task().
> -	 * - Moving a task which has been woken up by try_to_wake_up() and
> -	 *   waiting for actually being woken up by sched_ttwu_pending().
> -	 */
> -	if (!queued && p->state != TASK_RUNNING &&
> -	    se->sum_exec_runtime && p->state != TASK_WAKING)
> +	if (!vruntime_normalized(p))
>  		se->vruntime += cfs_rq->min_vruntime;
>  }
>  
>  static void switched_from_fair(struct rq *rq, struct task_struct *p)
>  {
> -	detach_task_cfs_rq(p, task_on_rq_queued(p));
> +	detach_task_cfs_rq(p);
>  }
>  
> -/*
> - * We switched to the sched_fair class.
> - */
>  static void switched_to_fair(struct rq *rq, struct task_struct *p)
>  {
> -	int queued = task_on_rq_queued(p);
> -
> -	attach_task_cfs_rq(p, queued);
> +	attach_task_cfs_rq(p);
>  
> -	if (queued) {
> +	if (task_on_rq_queued(p)) {
>  		/*
>  		 * We were most likely switched from sched_rt, so
>  		 * kick off the schedule if running, otherwise just see
> @@ -8056,8 +8052,7 @@ static void task_move_group_fair(struct
>  {
>  	detach_task_cfs_rq(p, queued);
>  	set_task_rq(p, task_cpu(p));
> -
> -	/* tell se's cfs_rq has been changed */
> +	/* tell se's cfs_rq has been changed -- migrated */
>  	p->se.avg.last_update_time = 0;
>  	attach_task_cfs_rq(p, queued);
>  }
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1210284

FromByungchul Park <byungchul.park@lge.com>
Date2015-08-20 10:40 +0200
Message-ID<pZtya-7Zj-17@gated-at.bofh.it>
In reply to#1210107
On Thu, Aug 20, 2015 at 03:17:21AM +0200, Peter Zijlstra wrote:
> 
> I did something like this on top.. please have a look at the XXX and
> integrate.

i am not sure, what do you intend for me to do.

do you mean that i am supposed to integrate this cleanup patch you gave me
including the XXX comment?

> +	 *
> +	 * XXX this appears wrong!! check history, 
> +	 * we appear to always set queued and RUNNING under the same lock instance
> +	 * might be from before TASK_WAKING ?
>  	 */

is it impossible to happen to check if vruntime is normalized, when doing
something like e.g. active load balance where queued != TASK_ON_RQ_QUEUED
and p->state == TASK_RUNNING?

i think it can happen..

thanks,
byungchul
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1210409

FromByungchul Park <byungchul.park@lge.com>
Date2015-08-20 12:50 +0200
Message-ID<pZvzY-2qY-15@gated-at.bofh.it>
In reply to#1210284
On Thu, Aug 20, 2015 at 05:38:41PM +0900, Byungchul Park wrote:
> On Thu, Aug 20, 2015 at 03:17:21AM +0200, Peter Zijlstra wrote:
> > 
> > I did something like this on top.. please have a look at the XXX and
> > integrate.
> 
> i am not sure, what do you intend for me to do.
> 
> do you mean that i am supposed to integrate this cleanup patch you gave me
> including the XXX comment?
> 
> > +	 *
> > +	 * XXX this appears wrong!! check history, 
> > +	 * we appear to always set queued and RUNNING under the same lock instance
> > +	 * might be from before TASK_WAKING ?
> >  	 */
> 
> is it impossible to happen to check if vruntime is normalized, when doing
> something like e.g. active load balance where queued != TASK_ON_RQ_QUEUED
> and p->state == TASK_RUNNING?

furthermore, in any migration by load balance, it seems to be possible..

> 
> i think it can happen..
> 
> thanks,
> byungchul
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1210736

FromPeter Zijlstra <peterz@infradead.org>
Date2015-08-20 23:20 +0200
Message-ID<pZFpD-8nP-1@gated-at.bofh.it>
In reply to#1210409
On Thu, Aug 20, 2015 at 07:46:09PM +0900, Byungchul Park wrote:
> On Thu, Aug 20, 2015 at 05:38:41PM +0900, Byungchul Park wrote:
> > On Thu, Aug 20, 2015 at 03:17:21AM +0200, Peter Zijlstra wrote:
> > > 
> > > I did something like this on top.. please have a look at the XXX and
> > > integrate.
> > 
> > i am not sure, what do you intend for me to do.
> > 
> > do you mean that i am supposed to integrate this cleanup patch you gave me
> > including the XXX comment?

No, the intent was for you to think about the point marked XXX, which
you've done below.

> > > +	 *
> > > +	 * XXX this appears wrong!! check history, 
> > > +	 * we appear to always set queued and RUNNING under the same lock instance
> > > +	 * might be from before TASK_WAKING ?
> > >  	 */
> > 
> > is it impossible to happen to check if vruntime is normalized, when doing
> > something like e.g. active load balance where queued != TASK_ON_RQ_QUEUED
> > and p->state == TASK_RUNNING?
> 
> furthermore, in any migration by load balance, it seems to be possible..
> 
> > 
> > i think it can happen..

OK, then we need to change the comment to reflect the actual reason the
test is needed. Because I think the currently described scenario is
incorrect.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1210805

FromByungchul Park <byungchul.park@lge.com>
Date2015-08-21 02:10 +0200
Message-ID<pZI49-3QD-7@gated-at.bofh.it>
In reply to#1210736
On Thu, Aug 20, 2015 at 11:11:31PM +0200, Peter Zijlstra wrote:
> On Thu, Aug 20, 2015 at 07:46:09PM +0900, Byungchul Park wrote:
> > On Thu, Aug 20, 2015 at 05:38:41PM +0900, Byungchul Park wrote:
> > > On Thu, Aug 20, 2015 at 03:17:21AM +0200, Peter Zijlstra wrote:
> > > > 
> > > > I did something like this on top.. please have a look at the XXX and
> > > > integrate.
> > > 
> > > i am not sure, what do you intend for me to do.
> > > 
> > > do you mean that i am supposed to integrate this cleanup patch you gave me
> > > including the XXX comment?
> 
> No, the intent was for you to think about the point marked XXX, which
> you've done below.
> 
> > > > +	 *
> > > > +	 * XXX this appears wrong!! check history, 
> > > > +	 * we appear to always set queued and RUNNING under the same lock instance
> > > > +	 * might be from before TASK_WAKING ?
> > > >  	 */
> > > 
> > > is it impossible to happen to check if vruntime is normalized, when doing
> > > something like e.g. active load balance where queued != TASK_ON_RQ_QUEUED
> > > and p->state == TASK_RUNNING?
> > 
> > furthermore, in any migration by load balance, it seems to be possible..
> > 
> > > 
> > > i think it can happen..
> 
> OK, then we need to change the comment to reflect the actual reason the
> test is needed. Because I think the currently described scenario is
> incorrect.

what is the currently described scenario to need to change?

then.. did i change patches as what you suggested, in v4?

> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web