Re: [PATCH 1/3] sched, balancing: Update rq->max_idle_balance_cost whenever newidle balance is attempted

2014-04-28 Thread Preeti U Murthy
On 04/28/2014 11:34 PM, Jason Low wrote: > On Sun, 2014-04-27 at 14:01 +0530, Preeti Murthy wrote: >> Hi Jason, Peter, >> >> The below patch looks good to me except for one point. >> >> In idle_balance() the below code snippet does not look right: >> >> - if (pulled_task || time_after(jiffies,

Re: [PATCH 1/3] sched, balancing: Update rq->max_idle_balance_cost whenever newidle balance is attempted

2014-04-28 Thread Preeti U Murthy
On 04/28/2014 02:54 PM, Peter Zijlstra wrote: > On Sun, Apr 27, 2014 at 02:01:45PM +0530, Preeti Murthy wrote: >> Hi Jason, Peter, >> >> The below patch looks good to me except for one point. >> >> In idle_balance() the below code snippet does not look right: >> >> - if (pulled_task ||

Re: [PATCH 1/3] sched, balancing: Update rq->max_idle_balance_cost whenever newidle balance is attempted

2014-04-28 Thread Jason Low
On Sun, 2014-04-27 at 14:01 +0530, Preeti Murthy wrote: > Hi Jason, Peter, > > The below patch looks good to me except for one point. > > In idle_balance() the below code snippet does not look right: > > - if (pulled_task || time_after(jiffies, this_rq->next_balance)) { > - /* > - * We are

Re: [PATCH 1/3] sched, balancing: Update rq->max_idle_balance_cost whenever newidle balance is attempted

2014-04-28 Thread Jason Low
On Sat, 2014-04-26 at 16:50 +0200, Peter Zijlstra wrote: > On Fri, Apr 25, 2014 at 12:54:14PM -0700, Jason Low wrote: > > Preeti mentioned that sd->balance_interval is changed during load_balance(). > > Should we also consider updating the interval in rebalance_domains() after > > calling

Re: [PATCH 1/3] sched, balancing: Update rq->max_idle_balance_cost whenever newidle balance is attempted

2014-04-28 Thread Peter Zijlstra
On Sun, Apr 27, 2014 at 02:01:45PM +0530, Preeti Murthy wrote: > Hi Jason, Peter, > > The below patch looks good to me except for one point. > > In idle_balance() the below code snippet does not look right: > > - if (pulled_task || time_after(jiffies, this_rq->next_balance)) { > - /* > - * We

Re: [PATCH 1/3] sched, balancing: Update rq-max_idle_balance_cost whenever newidle balance is attempted

2014-04-28 Thread Peter Zijlstra
On Sun, Apr 27, 2014 at 02:01:45PM +0530, Preeti Murthy wrote: Hi Jason, Peter, The below patch looks good to me except for one point. In idle_balance() the below code snippet does not look right: - if (pulled_task || time_after(jiffies, this_rq-next_balance)) { - /* - * We are going

Re: [PATCH 1/3] sched, balancing: Update rq-max_idle_balance_cost whenever newidle balance is attempted

2014-04-28 Thread Jason Low
On Sat, 2014-04-26 at 16:50 +0200, Peter Zijlstra wrote: On Fri, Apr 25, 2014 at 12:54:14PM -0700, Jason Low wrote: Preeti mentioned that sd-balance_interval is changed during load_balance(). Should we also consider updating the interval in rebalance_domains() after calling load_balance(),

Re: [PATCH 1/3] sched, balancing: Update rq-max_idle_balance_cost whenever newidle balance is attempted

2014-04-28 Thread Jason Low
On Sun, 2014-04-27 at 14:01 +0530, Preeti Murthy wrote: Hi Jason, Peter, The below patch looks good to me except for one point. In idle_balance() the below code snippet does not look right: - if (pulled_task || time_after(jiffies, this_rq-next_balance)) { - /* - * We are going idle.

Re: [PATCH 1/3] sched, balancing: Update rq-max_idle_balance_cost whenever newidle balance is attempted

2014-04-28 Thread Preeti U Murthy
On 04/28/2014 02:54 PM, Peter Zijlstra wrote: On Sun, Apr 27, 2014 at 02:01:45PM +0530, Preeti Murthy wrote: Hi Jason, Peter, The below patch looks good to me except for one point. In idle_balance() the below code snippet does not look right: - if (pulled_task || time_after(jiffies,

Re: [PATCH 1/3] sched, balancing: Update rq-max_idle_balance_cost whenever newidle balance is attempted

2014-04-28 Thread Preeti U Murthy
On 04/28/2014 11:34 PM, Jason Low wrote: On Sun, 2014-04-27 at 14:01 +0530, Preeti Murthy wrote: Hi Jason, Peter, The below patch looks good to me except for one point. In idle_balance() the below code snippet does not look right: - if (pulled_task || time_after(jiffies,

Re: [PATCH 1/3] sched, balancing: Update rq->max_idle_balance_cost whenever newidle balance is attempted

2014-04-27 Thread Preeti Murthy
Hi Jason, Peter, The below patch looks good to me except for one point. In idle_balance() the below code snippet does not look right: - if (pulled_task || time_after(jiffies, this_rq->next_balance)) { - /* - * We are going idle. next_balance may be set based on - * a busy processor. So reset

Re: [PATCH 1/3] sched, balancing: Update rq-max_idle_balance_cost whenever newidle balance is attempted

2014-04-27 Thread Preeti Murthy
Hi Jason, Peter, The below patch looks good to me except for one point. In idle_balance() the below code snippet does not look right: - if (pulled_task || time_after(jiffies, this_rq-next_balance)) { - /* - * We are going idle. next_balance may be set based on - * a busy processor. So reset

Re: [PATCH 1/3] sched, balancing: Update rq->max_idle_balance_cost whenever newidle balance is attempted

2014-04-26 Thread Peter Zijlstra
On Fri, Apr 25, 2014 at 12:54:14PM -0700, Jason Low wrote: > Preeti mentioned that sd->balance_interval is changed during load_balance(). > Should we also consider updating the interval in rebalance_domains() after > calling load_balance(), Yeah, that might make sense. > and also taking

Re: [PATCH 1/3] sched, balancing: Update rq-max_idle_balance_cost whenever newidle balance is attempted

2014-04-26 Thread Peter Zijlstra
On Fri, Apr 25, 2014 at 12:54:14PM -0700, Jason Low wrote: Preeti mentioned that sd-balance_interval is changed during load_balance(). Should we also consider updating the interval in rebalance_domains() after calling load_balance(), Yeah, that might make sense. and also taking

Re: [PATCH 1/3] sched, balancing: Update rq->max_idle_balance_cost whenever newidle balance is attempted

2014-04-25 Thread Jason Low
On Fri, 2014-04-25 at 11:43 +0200, Peter Zijlstra wrote: > So how about something like this? It tracks the minimal next_balance for > whatever domains we do visit, or the very bottom domain in the > insta-bail case (but yeah, Mike's got a point.. we could think of > removing that). > > The

Re: [PATCH 1/3] sched, balancing: Update rq->max_idle_balance_cost whenever newidle balance is attempted

2014-04-25 Thread Jason Low
On Fri, 2014-04-25 at 09:58 +0200, Mike Galbraith wrote: > On Fri, 2014-04-25 at 00:13 -0700, Jason Low wrote: > > On Fri, 2014-04-25 at 10:42 +0530, Preeti U Murthy wrote: > > > I agree with this. However I am concerned with an additional point that > > > I have mentioned in my reply to Peter's

Re: [PATCH 1/3] sched, balancing: Update rq->max_idle_balance_cost whenever newidle balance is attempted

2014-04-25 Thread Peter Zijlstra
On Fri, Apr 25, 2014 at 10:38:43AM +0530, Preeti U Murthy wrote: > > But if it fails to pull anything, we'll (potentially) iterate the entire > > tree up to the largest domain; and supposedly set next_balanced to the > > largest possible interval. > > *to the smallest possible interval. > > > >

Re: [PATCH 1/3] sched, balancing: Update rq->max_idle_balance_cost whenever newidle balance is attempted

2014-04-25 Thread Mike Galbraith
On Fri, 2014-04-25 at 00:13 -0700, Jason Low wrote: > On Fri, 2014-04-25 at 10:42 +0530, Preeti U Murthy wrote: > > I agree with this. However I am concerned with an additional point that > > I have mentioned in my reply to Peter's mail on this thread. > > > > Should we verify if

Re: [PATCH 1/3] sched, balancing: Update rq->max_idle_balance_cost whenever newidle balance is attempted

2014-04-25 Thread Jason Low
On Fri, 2014-04-25 at 10:42 +0530, Preeti U Murthy wrote: > I agree with this. However I am concerned with an additional point that > I have mentioned in my reply to Peter's mail on this thread. > > Should we verify if rq->next_balance update is independent of > pulled_tasks? sd->balance_interval

Re: [PATCH 1/3] sched, balancing: Update rq-max_idle_balance_cost whenever newidle balance is attempted

2014-04-25 Thread Jason Low
On Fri, 2014-04-25 at 10:42 +0530, Preeti U Murthy wrote: I agree with this. However I am concerned with an additional point that I have mentioned in my reply to Peter's mail on this thread. Should we verify if rq-next_balance update is independent of pulled_tasks? sd-balance_interval is

Re: [PATCH 1/3] sched, balancing: Update rq-max_idle_balance_cost whenever newidle balance is attempted

2014-04-25 Thread Mike Galbraith
On Fri, 2014-04-25 at 00:13 -0700, Jason Low wrote: On Fri, 2014-04-25 at 10:42 +0530, Preeti U Murthy wrote: I agree with this. However I am concerned with an additional point that I have mentioned in my reply to Peter's mail on this thread. Should we verify if rq-next_balance update

Re: [PATCH 1/3] sched, balancing: Update rq-max_idle_balance_cost whenever newidle balance is attempted

2014-04-25 Thread Peter Zijlstra
On Fri, Apr 25, 2014 at 10:38:43AM +0530, Preeti U Murthy wrote: But if it fails to pull anything, we'll (potentially) iterate the entire tree up to the largest domain; and supposedly set next_balanced to the largest possible interval. *to the smallest possible interval. So when we

Re: [PATCH 1/3] sched, balancing: Update rq-max_idle_balance_cost whenever newidle balance is attempted

2014-04-25 Thread Jason Low
On Fri, 2014-04-25 at 09:58 +0200, Mike Galbraith wrote: On Fri, 2014-04-25 at 00:13 -0700, Jason Low wrote: On Fri, 2014-04-25 at 10:42 +0530, Preeti U Murthy wrote: I agree with this. However I am concerned with an additional point that I have mentioned in my reply to Peter's mail on

Re: [PATCH 1/3] sched, balancing: Update rq-max_idle_balance_cost whenever newidle balance is attempted

2014-04-25 Thread Jason Low
On Fri, 2014-04-25 at 11:43 +0200, Peter Zijlstra wrote: So how about something like this? It tracks the minimal next_balance for whatever domains we do visit, or the very bottom domain in the insta-bail case (but yeah, Mike's got a point.. we could think of removing that). The thought is

Re: [PATCH 1/3] sched, balancing: Update rq->max_idle_balance_cost whenever newidle balance is attempted

2014-04-24 Thread Preeti U Murthy
Hi Jason, On 04/25/2014 03:48 AM, Jason Low wrote: > On Thu, 2014-04-24 at 19:14 +0200, Peter Zijlstra wrote: >> On Thu, Apr 24, 2014 at 09:53:37AM -0700, Jason Low wrote: >>> >>> So I thought that the original rationale (commit 1bd77f2d) behind >>> updating rq->next_balance in idle_balance() is

Re: [PATCH 1/3] sched, balancing: Update rq->max_idle_balance_cost whenever newidle balance is attempted

2014-04-24 Thread Preeti U Murthy
On 04/24/2014 10:44 PM, Peter Zijlstra wrote: > On Thu, Apr 24, 2014 at 09:53:37AM -0700, Jason Low wrote: >> >> So I thought that the original rationale (commit 1bd77f2d) behind >> updating rq->next_balance in idle_balance() is that, if we are going >> idle (!pulled_task), we want to ensure that

Re: [PATCH 1/3] sched, balancing: Update rq->max_idle_balance_cost whenever newidle balance is attempted

2014-04-24 Thread Jason Low
On Thu, 2014-04-24 at 19:14 +0200, Peter Zijlstra wrote: > On Thu, Apr 24, 2014 at 09:53:37AM -0700, Jason Low wrote: > > > > So I thought that the original rationale (commit 1bd77f2d) behind > > updating rq->next_balance in idle_balance() is that, if we are going > > idle (!pulled_task), we want

Re: [PATCH 1/3] sched, balancing: Update rq->max_idle_balance_cost whenever newidle balance is attempted

2014-04-24 Thread Peter Zijlstra
On Thu, Apr 24, 2014 at 07:14:53PM +0200, Peter Zijlstra wrote: > On Thu, Apr 24, 2014 at 09:53:37AM -0700, Jason Low wrote: > > > > So I thought that the original rationale (commit 1bd77f2d) behind > > updating rq->next_balance in idle_balance() is that, if we are going > > idle (!pulled_task),

Re: [PATCH 1/3] sched, balancing: Update rq->max_idle_balance_cost whenever newidle balance is attempted

2014-04-24 Thread Peter Zijlstra
On Thu, Apr 24, 2014 at 09:53:37AM -0700, Jason Low wrote: > > So I thought that the original rationale (commit 1bd77f2d) behind > updating rq->next_balance in idle_balance() is that, if we are going > idle (!pulled_task), we want to ensure that the next_balance gets > calculated without the

Re: [PATCH 1/3] sched, balancing: Update rq->max_idle_balance_cost whenever newidle balance is attempted

2014-04-24 Thread Jason Low
On Thu, 2014-04-24 at 14:44 +0200, Peter Zijlstra wrote: > On Thu, Apr 24, 2014 at 02:04:15PM +0200, Peter Zijlstra wrote: > > On Thu, Apr 24, 2014 at 03:44:47PM +0530, Preeti U Murthy wrote: > > > What about the update of next_balance field? See the code snippet below. > > > This will also be

Re: [PATCH 1/3] sched, balancing: Update rq->max_idle_balance_cost whenever newidle balance is attempted

2014-04-24 Thread Peter Zijlstra
On Thu, Apr 24, 2014 at 02:04:15PM +0200, Peter Zijlstra wrote: > On Thu, Apr 24, 2014 at 03:44:47PM +0530, Preeti U Murthy wrote: > > What about the update of next_balance field? See the code snippet below. > > This will also be skipped as a consequence of the commit e5fc6611 right? > > > >

Re: [PATCH 1/3] sched, balancing: Update rq->max_idle_balance_cost whenever newidle balance is attempted

2014-04-24 Thread Peter Zijlstra
On Thu, Apr 24, 2014 at 03:44:47PM +0530, Preeti U Murthy wrote: > What about the update of next_balance field? See the code snippet below. > This will also be skipped as a consequence of the commit e5fc6611 right? > > if (pulled_task || time_after(jiffies, this_rq->next_balance)) { >

Re: [PATCH 1/3] sched, balancing: Update rq->max_idle_balance_cost whenever newidle balance is attempted

2014-04-24 Thread Preeti U Murthy
Hi Jason, On 04/24/2014 07:00 AM, Jason Low wrote: > Commit e5fc6611 can potentially cause rq->max_idle_balance_cost to not be > updated, even when load_balance(NEWLY_IDLE) is attempted and the per-sd > max cost value is updated. > > In this patch, we update the rq->max_idle_balance_cost

Re: [PATCH 1/3] sched, balancing: Update rq-max_idle_balance_cost whenever newidle balance is attempted

2014-04-24 Thread Preeti U Murthy
Hi Jason, On 04/24/2014 07:00 AM, Jason Low wrote: Commit e5fc6611 can potentially cause rq-max_idle_balance_cost to not be updated, even when load_balance(NEWLY_IDLE) is attempted and the per-sd max cost value is updated. In this patch, we update the rq-max_idle_balance_cost regardless of

Re: [PATCH 1/3] sched, balancing: Update rq-max_idle_balance_cost whenever newidle balance is attempted

2014-04-24 Thread Peter Zijlstra
On Thu, Apr 24, 2014 at 03:44:47PM +0530, Preeti U Murthy wrote: What about the update of next_balance field? See the code snippet below. This will also be skipped as a consequence of the commit e5fc6611 right? if (pulled_task || time_after(jiffies, this_rq-next_balance)) {

Re: [PATCH 1/3] sched, balancing: Update rq-max_idle_balance_cost whenever newidle balance is attempted

2014-04-24 Thread Peter Zijlstra
On Thu, Apr 24, 2014 at 02:04:15PM +0200, Peter Zijlstra wrote: On Thu, Apr 24, 2014 at 03:44:47PM +0530, Preeti U Murthy wrote: What about the update of next_balance field? See the code snippet below. This will also be skipped as a consequence of the commit e5fc6611 right? if

Re: [PATCH 1/3] sched, balancing: Update rq-max_idle_balance_cost whenever newidle balance is attempted

2014-04-24 Thread Jason Low
On Thu, 2014-04-24 at 14:44 +0200, Peter Zijlstra wrote: On Thu, Apr 24, 2014 at 02:04:15PM +0200, Peter Zijlstra wrote: On Thu, Apr 24, 2014 at 03:44:47PM +0530, Preeti U Murthy wrote: What about the update of next_balance field? See the code snippet below. This will also be skipped as a

Re: [PATCH 1/3] sched, balancing: Update rq-max_idle_balance_cost whenever newidle balance is attempted

2014-04-24 Thread Peter Zijlstra
On Thu, Apr 24, 2014 at 09:53:37AM -0700, Jason Low wrote: So I thought that the original rationale (commit 1bd77f2d) behind updating rq-next_balance in idle_balance() is that, if we are going idle (!pulled_task), we want to ensure that the next_balance gets calculated without the

Re: [PATCH 1/3] sched, balancing: Update rq-max_idle_balance_cost whenever newidle balance is attempted

2014-04-24 Thread Peter Zijlstra
On Thu, Apr 24, 2014 at 07:14:53PM +0200, Peter Zijlstra wrote: On Thu, Apr 24, 2014 at 09:53:37AM -0700, Jason Low wrote: So I thought that the original rationale (commit 1bd77f2d) behind updating rq-next_balance in idle_balance() is that, if we are going idle (!pulled_task), we want to

Re: [PATCH 1/3] sched, balancing: Update rq-max_idle_balance_cost whenever newidle balance is attempted

2014-04-24 Thread Jason Low
On Thu, 2014-04-24 at 19:14 +0200, Peter Zijlstra wrote: On Thu, Apr 24, 2014 at 09:53:37AM -0700, Jason Low wrote: So I thought that the original rationale (commit 1bd77f2d) behind updating rq-next_balance in idle_balance() is that, if we are going idle (!pulled_task), we want to ensure

Re: [PATCH 1/3] sched, balancing: Update rq-max_idle_balance_cost whenever newidle balance is attempted

2014-04-24 Thread Preeti U Murthy
On 04/24/2014 10:44 PM, Peter Zijlstra wrote: On Thu, Apr 24, 2014 at 09:53:37AM -0700, Jason Low wrote: So I thought that the original rationale (commit 1bd77f2d) behind updating rq-next_balance in idle_balance() is that, if we are going idle (!pulled_task), we want to ensure that the

Re: [PATCH 1/3] sched, balancing: Update rq-max_idle_balance_cost whenever newidle balance is attempted

2014-04-24 Thread Preeti U Murthy
Hi Jason, On 04/25/2014 03:48 AM, Jason Low wrote: On Thu, 2014-04-24 at 19:14 +0200, Peter Zijlstra wrote: On Thu, Apr 24, 2014 at 09:53:37AM -0700, Jason Low wrote: So I thought that the original rationale (commit 1bd77f2d) behind updating rq-next_balance in idle_balance() is that, if we

[PATCH 1/3] sched, balancing: Update rq->max_idle_balance_cost whenever newidle balance is attempted

2014-04-23 Thread Jason Low
Commit e5fc6611 can potentially cause rq->max_idle_balance_cost to not be updated, even when load_balance(NEWLY_IDLE) is attempted and the per-sd max cost value is updated. In this patch, we update the rq->max_idle_balance_cost regardless of whether or not a task has been enqueued while browsing

[PATCH 1/3] sched, balancing: Update rq-max_idle_balance_cost whenever newidle balance is attempted

2014-04-23 Thread Jason Low
Commit e5fc6611 can potentially cause rq-max_idle_balance_cost to not be updated, even when load_balance(NEWLY_IDLE) is attempted and the per-sd max cost value is updated. In this patch, we update the rq-max_idle_balance_cost regardless of whether or not a task has been enqueued while browsing