All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com>
To: Paul Turner <pjt@google.com>
Cc: linux-kernel@vger.kernel.org,
	Bharata B Rao <bharata@linux.vnet.ibm.com>,
	Dhaval Giani <dhaval@linux.vnet.ibm.com>,
	Balbir Singh <balbir@linux.vnet.ibm.com>,
	Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com>,
	Gautham R Shenoy <ego@in.ibm.com>,
	Srivatsa Vaddagiri <vatsa@in.ibm.com>,
	Kamalesh Babulal <kamalesh@linux.vnet.ibm.com>,
	Ingo Molnar <mingo@elte.hu>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Pavel Emelyanov <xemul@openvz.org>,
	Herbert Poetzl <herbert@13thfloor.at>,
	Avi Kivity <avi@redhat.com>, Chris Friesen <cfriesen@nortel.com>
Subject: Re: [CFS Bandwidth Control v4 6/7] sched: hierarchical task accounting for SCHED_OTHER
Date: Wed, 23 Feb 2011 11:02:40 +0900	[thread overview]
Message-ID: <4D646AC0.7030809@jp.fujitsu.com> (raw)
In-Reply-To: <20110216031841.352204682@google.com>

(2011/02/16 12:18), Paul Turner wrote:
> @@ -1428,6 +1464,7 @@ enqueue_task_fair(struct rq *rq, struct 
>  		update_cfs_shares(cfs_rq);
>  	}
>  
> +	account_hier_tasks(&p->se, 1);
>  	hrtick_update(rq);
>  }
>  
> @@ -1461,6 +1498,7 @@ static void dequeue_task_fair(struct rq 
>  		update_cfs_shares(cfs_rq);
>  	}
>  
> +	account_hier_tasks(&p->se, -1);
>  	hrtick_update(rq);
>  }
>  

Why hrtick_update() is need to be delayed after modifying nr_running? 
You should not impact current hrtick logic without once mentioning.

> Index: tip/kernel/sched_rt.c
> ===================================================================
> --- tip.orig/kernel/sched_rt.c
> +++ tip/kernel/sched_rt.c
> @@ -906,6 +906,8 @@ enqueue_task_rt(struct rq *rq, struct ta
>  
>  	if (!task_current(rq, p) && p->rt.nr_cpus_allowed > 1)
>  		enqueue_pushable_task(rq, p);
> +
> +	inc_nr_running(rq);
>  }
>  
>  static void dequeue_task_rt(struct rq *rq, struct task_struct *p, int flags)
> @@ -916,6 +918,8 @@ static void dequeue_task_rt(struct rq *r
>  	dequeue_rt_entity(rt_se);
>  
>  	dequeue_pushable_task(rq, p);
> +
> +	dec_nr_running(rq);
>  }
>  
>  /*

I think similar change for sched_stoptask.c is required.

In fact I could not boot tip/master with this v4 patchset.
It reports rcu stall warns without applying following tweak:

--- a/kernel/sched_stoptask.c
+++ b/kernel/sched_stoptask.c
@@ -35,11 +35,13 @@ static struct task_struct *pick_next_task_stop(struct rq *rq
 static void
 enqueue_task_stop(struct rq *rq, struct task_struct *p, int flags)
 {
+       inc_nr_running(rq);
 }

 static void
 dequeue_task_stop(struct rq *rq, struct task_struct *p, int flags)
 {
+       dec_nr_running(rq);
 }


Thanks,
H.Seto


  parent reply	other threads:[~2011-02-23  2:03 UTC|newest]

Thread overview: 71+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-02-16  3:18 [CFS Bandwidth Control v4 0/7] Introduction Paul Turner
2011-02-16  3:18 ` [CFS Bandwidth Control v4 1/7] sched: introduce primitives to account for CFS bandwidth tracking Paul Turner
2011-02-16 16:52   ` Balbir Singh
2011-02-17  2:54     ` Bharata B Rao
2011-02-23 13:32   ` Peter Zijlstra
2011-02-25  3:11     ` Paul Turner
2011-02-25 20:53     ` Paul Turner
2011-02-16  3:18 ` [CFS Bandwidth Control v4 2/7] sched: accumulate per-cfs_rq cpu usage Paul Turner
2011-02-16 17:45   ` Balbir Singh
2011-02-23 13:32   ` Peter Zijlstra
2011-02-25  3:33     ` Paul Turner
2011-02-25 12:31       ` Peter Zijlstra
2011-02-16  3:18 ` [CFS Bandwidth Control v4 3/7] sched: throttle cfs_rq entities which exceed their local quota Paul Turner
2011-02-18  6:52   ` Balbir Singh
2011-02-23 13:32   ` Peter Zijlstra
2011-02-24  5:21     ` Bharata B Rao
2011-02-24 11:05       ` Peter Zijlstra
2011-02-24 15:45         ` Bharata B Rao
2011-02-24 15:52           ` Peter Zijlstra
2011-02-24 16:39             ` Bharata B Rao
2011-02-24 17:20               ` Peter Zijlstra
2011-02-25  3:59                 ` Paul Turner
2011-02-25  3:41         ` Paul Turner
2011-02-25  3:10     ` Paul Turner
2011-02-25 13:58       ` Bharata B Rao
2011-02-25 20:51         ` Paul Turner
2011-02-28  3:50           ` Bharata B Rao
2011-02-28  6:38             ` Paul Turner
2011-02-28 13:48       ` Peter Zijlstra
2011-03-01  8:31         ` Paul Turner
2011-03-02  7:23   ` Bharata B Rao
2011-03-02  8:05     ` Paul Turner
2011-02-16  3:18 ` [CFS Bandwidth Control v4 4/7] sched: unthrottle cfs_rq(s) who ran out of quota at period refresh Paul Turner
2011-02-18  7:19   ` Balbir Singh
2011-02-18  8:10     ` Bharata B Rao
2011-02-23 12:23   ` Peter Zijlstra
2011-02-23 13:32   ` Peter Zijlstra
2011-02-24  7:04     ` Bharata B Rao
2011-02-24 11:14       ` Peter Zijlstra
2011-02-26  0:02     ` Paul Turner
2011-02-16  3:18 ` [CFS Bandwidth Control v4 5/7] sched: add exports tracking cfs bandwidth control statistics Paul Turner
2011-02-22  3:14   ` Balbir Singh
2011-02-22  4:13     ` Bharata B Rao
2011-02-22  4:40       ` Balbir Singh
2011-02-23  8:03         ` Paul Turner
2011-02-23 10:13           ` Balbir Singh
2011-02-23 13:32   ` Peter Zijlstra
2011-02-25  3:26     ` Paul Turner
2011-02-25  8:54       ` Peter Zijlstra
2011-02-16  3:18 ` [CFS Bandwidth Control v4 6/7] sched: hierarchical task accounting for SCHED_OTHER Paul Turner
2011-02-22  3:17   ` Balbir Singh
2011-02-23  8:05     ` Paul Turner
2011-02-23  2:02   ` Hidetoshi Seto [this message]
2011-02-23  2:20     ` Paul Turner
2011-02-23  2:43     ` Balbir Singh
2011-02-23 13:32   ` Peter Zijlstra
2011-02-25  3:25     ` Paul Turner
2011-02-25 12:17       ` Peter Zijlstra
2011-02-16  3:18 ` [CFS Bandwidth Control v4 7/7] sched: add documentation for bandwidth control Paul Turner
2011-02-21  2:47 ` [CFS Bandwidth Control v4 0/7] Introduction Xiao Guangrong
2011-02-22 10:28   ` Bharata B Rao
2011-02-23  7:42   ` Paul Turner
2011-02-23  7:51     ` Balbir Singh
2011-02-23  7:56       ` Paul Turner
2011-02-23  8:31         ` Bharata B Rao
     [not found] ` <20110224161111.7d83a884@jacob-laptop>
2011-02-25 10:03   ` Paul Turner
2011-02-25 13:06     ` jacob pan
2011-03-08  3:57       ` Balbir Singh
2011-03-08 18:18         ` Jacob Pan
2011-03-09 10:12       ` Paul Turner
2011-03-09 21:57         ` jacob pan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4D646AC0.7030809@jp.fujitsu.com \
    --to=seto.hidetoshi@jp.fujitsu.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=avi@redhat.com \
    --cc=balbir@linux.vnet.ibm.com \
    --cc=bharata@linux.vnet.ibm.com \
    --cc=cfriesen@nortel.com \
    --cc=dhaval@linux.vnet.ibm.com \
    --cc=ego@in.ibm.com \
    --cc=herbert@13thfloor.at \
    --cc=kamalesh@linux.vnet.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=pjt@google.com \
    --cc=svaidy@linux.vnet.ibm.com \
    --cc=vatsa@in.ibm.com \
    --cc=xemul@openvz.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.