linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/1] scheduler: minor improvement to pick_next_highest_task_rt in linux-3.3
@ 2012-03-19 22:26 Michael J. Wang
  2012-03-20 13:59 ` Steven Rostedt
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Michael J. Wang @ 2012-03-19 22:26 UTC (permalink / raw)
  To: mingo, peterz; +Cc: Michael J. Wang, linux-kernel, rostedt, yong.zhang0

From: Michael J Wang <mjwang@broadcom.com>

Avoid extra work by continuing on to the next rt_rq if the highest prio task in current rt_rq is the same priority as our candidate task.

Signed-off-by: Michael J Wang <mjwang@broadcom.com>

---

More detailed explanation:  if next is not NULL, then we have found a candidate task, and its priority is next->prio.  Now we are looking for an even higher priority task in the other rt_rq's.  idx is the highest priority in the current candidate rt_rq.  In the current 3.3 code, if idx is equal to next->prio, we would start scanning the tasks in that rt_rq and replace the current candidate task with a task from that rt_rq.  But the new task would only have a priority that is equal to our previous candidate task, so we have not advanced our goal of finding a higher prio task.  So we should avoid the extra work by continuing on to the next rt_rq if idx is equal to next->prio.

--- linux-3.3/kernel/sched/rt.c.orig	2012-03-18 16:15:34.000000000 -0700
+++ linux-3.3/kernel/sched/rt.c	2012-03-19 14:52:54.585391702 -0700
@@ -1403,7 +1403,7 @@ static struct task_struct *pick_next_hig
 next_idx:
 		if (idx >= MAX_RT_PRIO)
 			continue;
-		if (next && next->prio < idx)
+		if (next && next->prio <= idx)
 			continue;
 		list_for_each_entry(rt_se, array->queue + idx, run_list) {
 			struct task_struct *p;







^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/1] scheduler: minor improvement to pick_next_highest_task_rt in linux-3.3
  2012-03-19 22:26 [PATCH 1/1] scheduler: minor improvement to pick_next_highest_task_rt in linux-3.3 Michael J. Wang
@ 2012-03-20 13:59 ` Steven Rostedt
  2012-03-21  1:40 ` Yong Zhang
  2012-03-27 15:35 ` [tip:sched/urgent] sched/rt: Improve pick_next_highest_task_rt() tip-bot for Michael J Wang
  2 siblings, 0 replies; 11+ messages in thread
From: Steven Rostedt @ 2012-03-20 13:59 UTC (permalink / raw)
  To: Michael J. Wang; +Cc: mingo, peterz, linux-kernel, yong.zhang0

On Mon, 2012-03-19 at 22:26 +0000, Michael J. Wang wrote:
> From: Michael J Wang <mjwang@broadcom.com>
> 
> Avoid extra work by continuing on to the next rt_rq if the highest prio task in current rt_rq is the same priority as our candidate task.
> 
> Signed-off-by: Michael J Wang <mjwang@broadcom.com>
> 
> ---
> 
> More detailed explanation:  if next is not NULL, then we have found a candidate task, and its priority is next->prio.  Now we are looking for an even higher priority task in the other rt_rq's.  idx is the highest priority in the current candidate rt_rq.  In the current 3.3 code, if idx is equal to next->prio, we would start scanning the tasks in that rt_rq and replace the current candidate task with a task from that rt_rq.  But the new task would only have a priority that is equal to our previous candidate task, so we have not advanced our goal of finding a higher prio task.  So we should avoid the extra work by continuing on to the next rt_rq if idx is equal to next->prio.
> 
> --- linux-3.3/kernel/sched/rt.c.orig	2012-03-18 16:15:34.000000000 -0700
> +++ linux-3.3/kernel/sched/rt.c	2012-03-19 14:52:54.585391702 -0700
> @@ -1403,7 +1403,7 @@ static struct task_struct *pick_next_hig
>  next_idx:
>  		if (idx >= MAX_RT_PRIO)
>  			continue;
> -		if (next && next->prio < idx)
> +		if (next && next->prio <= idx)
>  			continue;
>  		list_for_each_entry(rt_se, array->queue + idx, run_list) {
>  			struct task_struct *p;

Acked-by: Steven Rostedt <rostedt@goodmis.org>

-- Steve



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/1] scheduler: minor improvement to pick_next_highest_task_rt in linux-3.3
  2012-03-19 22:26 [PATCH 1/1] scheduler: minor improvement to pick_next_highest_task_rt in linux-3.3 Michael J. Wang
  2012-03-20 13:59 ` Steven Rostedt
@ 2012-03-21  1:40 ` Yong Zhang
  2012-03-21  1:56   ` Michael J. Wang
  2012-03-27 15:35 ` [tip:sched/urgent] sched/rt: Improve pick_next_highest_task_rt() tip-bot for Michael J Wang
  2 siblings, 1 reply; 11+ messages in thread
From: Yong Zhang @ 2012-03-21  1:40 UTC (permalink / raw)
  To: Michael J. Wang; +Cc: mingo, peterz, linux-kernel, rostedt

On Mon, Mar 19, 2012 at 10:26:19PM +0000, Michael J. Wang wrote:
> From: Michael J Wang <mjwang@broadcom.com>
> 
> Avoid extra work by continuing on to the next rt_rq if the highest prio task in current rt_rq is the same priority as our candidate task.
> 
> Signed-off-by: Michael J Wang <mjwang@broadcom.com>
> 
> ---
> 
> More detailed explanation:  if next is not NULL, then we have found a candidate task, and its priority is next->prio.  Now we are looking for an even higher priority task in the other rt_rq's.  idx is the highest priority in the current candidate rt_rq.  In the current 3.3 code, if idx is equal to next->prio, we would start scanning the tasks in that rt_rq and replace the current candidate task with a task from that rt_rq.  But the new task would only have a priority that is equal to our previous candidate task, so we have not advanced our goal of finding a higher prio task.  So we should avoid the extra work by continuing on to the next rt_rq if idx is equal to next->prio.
> 

You should limit characters of each line to 80 if possible.

And before sending you patch, linux-source/scripts/checkpatch.pl maybe
give you some clues whether there is some warning/error. If there are,
fix them.

Only for what your patch wants to show:
Reviewed-by: Yong Zhang <yong.zhang0@gmail.com>

Thanks,
Yong

> --- linux-3.3/kernel/sched/rt.c.orig	2012-03-18 16:15:34.000000000 -0700
> +++ linux-3.3/kernel/sched/rt.c	2012-03-19 14:52:54.585391702 -0700
> @@ -1403,7 +1403,7 @@ static struct task_struct *pick_next_hig
>  next_idx:
>  		if (idx >= MAX_RT_PRIO)
>  			continue;
> -		if (next && next->prio < idx)
> +		if (next && next->prio <= idx)
>  			continue;
>  		list_for_each_entry(rt_se, array->queue + idx, run_list) {
>  			struct task_struct *p;
> 
> 
> 
> 
> 
> 
> --
> 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/

-- 
Only stand for myself

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH 1/1] scheduler: minor improvement to pick_next_highest_task_rt in linux-3.3
  2012-03-21  1:40 ` Yong Zhang
@ 2012-03-21  1:56   ` Michael J. Wang
  2012-03-21  2:12     ` Yong Zhang
  0 siblings, 1 reply; 11+ messages in thread
From: Michael J. Wang @ 2012-03-21  1:56 UTC (permalink / raw)
  To: Yong Zhang; +Cc: mingo, peterz, linux-kernel, rostedt, Michael J. Wang

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=utf-8, Size: 2681 bytes --]

Ah, I see.  Forgot about the length of my comment line.

Should I reformat my patch and send it again?

Thanks,
Michael


-----Original Message-----
From: Yong Zhang [mailto:yong.zhang0@gmail.com] 
Sent: Tuesday, March 20, 2012 6:41 PM
To: Michael J. Wang
Cc: mingo@elte.hu; peterz@infradead.org; linux-kernel@vger.kernel.org; rostedt@goodmis.org
Subject: Re: [PATCH 1/1] scheduler: minor improvement to pick_next_highest_task_rt in linux-3.3

On Mon, Mar 19, 2012 at 10:26:19PM +0000, Michael J. Wang wrote:
> From: Michael J Wang <mjwang@broadcom.com>
> 
> Avoid extra work by continuing on to the next rt_rq if the highest prio task in current rt_rq is the same priority as our candidate task.
> 
> Signed-off-by: Michael J Wang <mjwang@broadcom.com>
> 
> ---
> 
> More detailed explanation:  if next is not NULL, then we have found a candidate task, and its priority is next->prio.  Now we are looking for an even higher priority task in the other rt_rq's.  idx is the highest priority in the current candidate rt_rq.  In the current 3.3 code, if idx is equal to next->prio, we would start scanning the tasks in that rt_rq and replace the current candidate task with a task from that rt_rq.  But the new task would only have a priority that is equal to our previous candidate task, so we have not advanced our goal of finding a higher prio task.  So we should avoid the extra work by continuing on to the next rt_rq if idx is equal to next->prio.
> 

You should limit characters of each line to 80 if possible.

And before sending you patch, linux-source/scripts/checkpatch.pl maybe
give you some clues whether there is some warning/error. If there are,
fix them.

Only for what your patch wants to show:
Reviewed-by: Yong Zhang <yong.zhang0@gmail.com>

Thanks,
Yong

> --- linux-3.3/kernel/sched/rt.c.orig	2012-03-18 16:15:34.000000000 -0700
> +++ linux-3.3/kernel/sched/rt.c	2012-03-19 14:52:54.585391702 -0700
> @@ -1403,7 +1403,7 @@ static struct task_struct *pick_next_hig
>  next_idx:
>  		if (idx >= MAX_RT_PRIO)
>  			continue;
> -		if (next && next->prio < idx)
> +		if (next && next->prio <= idx)
>  			continue;
>  		list_for_each_entry(rt_se, array->queue + idx, run_list) {
>  			struct task_struct *p;
> 
> 
> 
> 
> 
> 
> --
> 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/

-- 
Only stand for myself

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/1] scheduler: minor improvement to pick_next_highest_task_rt in linux-3.3
  2012-03-21  1:56   ` Michael J. Wang
@ 2012-03-21  2:12     ` Yong Zhang
  2012-03-21  7:49       ` Michael J. Wang
  0 siblings, 1 reply; 11+ messages in thread
From: Yong Zhang @ 2012-03-21  2:12 UTC (permalink / raw)
  To: Michael J. Wang; +Cc: mingo, peterz, linux-kernel, rostedt

On Wed, Mar 21, 2012 at 01:56:47AM +0000, Michael J. Wang wrote:
> Ah, I see.  Forgot about the length of my comment line.
> 
> Should I reformat my patch and send it again?

It'll be better, and I think Peter/Ingo will happy with it.

Thanks,
Yong

> 
> Thanks,
> Michael
> 
> 
> -----Original Message-----
> From: Yong Zhang [mailto:yong.zhang0@gmail.com] 
> Sent: Tuesday, March 20, 2012 6:41 PM
> To: Michael J. Wang
> Cc: mingo@elte.hu; peterz@infradead.org; linux-kernel@vger.kernel.org; rostedt@goodmis.org
> Subject: Re: [PATCH 1/1] scheduler: minor improvement to pick_next_highest_task_rt in linux-3.3
> 
> On Mon, Mar 19, 2012 at 10:26:19PM +0000, Michael J. Wang wrote:
> > From: Michael J Wang <mjwang@broadcom.com>
> > 
> > Avoid extra work by continuing on to the next rt_rq if the highest prio task in current rt_rq is the same priority as our candidate task.
> > 
> > Signed-off-by: Michael J Wang <mjwang@broadcom.com>
> > 
> > ---
> > 
> > More detailed explanation:  if next is not NULL, then we have found a candidate task, and its priority is next->prio.  Now we are looking for an even higher priority task in the other rt_rq's.  idx is the highest priority in the current candidate rt_rq.  In the current 3.3 code, if idx is equal to next->prio, we would start scanning the tasks in that rt_rq and replace the current candidate task with a task from that rt_rq.  But the new task would only have a priority that is equal to our previous candidate task, so we have not advanced our goal of finding a higher prio task.  So we should avoid the extra work by continuing on to the next rt_rq if idx is equal to next->prio.
> > 
> 
> You should limit characters of each line to 80 if possible.
> 
> And before sending you patch, linux-source/scripts/checkpatch.pl maybe
> give you some clues whether there is some warning/error. If there are,
> fix them.
> 
> Only for what your patch wants to show:
> Reviewed-by: Yong Zhang <yong.zhang0@gmail.com>
> 
> Thanks,
> Yong
> 
> > --- linux-3.3/kernel/sched/rt.c.orig	2012-03-18 16:15:34.000000000 -0700
> > +++ linux-3.3/kernel/sched/rt.c	2012-03-19 14:52:54.585391702 -0700
> > @@ -1403,7 +1403,7 @@ static struct task_struct *pick_next_hig
> >  next_idx:
> >  		if (idx >= MAX_RT_PRIO)
> >  			continue;
> > -		if (next && next->prio < idx)
> > +		if (next && next->prio <= idx)
> >  			continue;
> >  		list_for_each_entry(rt_se, array->queue + idx, run_list) {
> >  			struct task_struct *p;
> > 
> > 
> > 
> > 
> > 
> > 
> > --
> > 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/
> 
> -- 
> Only stand for myself
> 
> N�����r��y���b�X��ǧv�^�)޺{.n�+����{����zX��\x17��ܨ}���Ơz�&j:+v���\a����zZ+��+zf���h���~����i���z�\x1e�w���?����&�)ߢ^[f��^jǫy�m��@A�a��\x7f�\f0��h�\x0f�i\x7f

-- 
Only stand for myself

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH 1/1] scheduler: minor improvement to pick_next_highest_task_rt in linux-3.3
  2012-03-21  2:12     ` Yong Zhang
@ 2012-03-21  7:49       ` Michael J. Wang
  2012-03-21  9:06         ` Peter Zijlstra
  0 siblings, 1 reply; 11+ messages in thread
From: Michael J. Wang @ 2012-03-21  7:49 UTC (permalink / raw)
  To: Yong Zhang; +Cc: mingo, peterz, linux-kernel, rostedt, Michael J. Wang

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=utf-8, Size: 3825 bytes --]


> From: Yong Zhang [mailto:yong.zhang0@gmail.com]
> Sent: Tuesday, March 20, 2012 7:12 PM
> To: Michael J. Wang
> Cc: mingo@elte.hu; peterz@infradead.org; linux-kernel@vger.kernel.org;
> rostedt@goodmis.org
> Subject: Re: [PATCH 1/1] scheduler: minor improvement to
> pick_next_highest_task_rt in linux-3.3
> 
> On Wed, Mar 21, 2012 at 01:56:47AM +0000, Michael J. Wang wrote:
> > Ah, I see.  Forgot about the length of my comment line.
> >
> > Should I reformat my patch and send it again?
> 
> It'll be better, and I think Peter/Ingo will happy with it.
> 

OK.  I will resend now.  Thanks for all your help!

Michael


> Thanks,
> Yong
> 
> >
> > Thanks,
> > Michael
> >
> >
> > -----Original Message-----
> > From: Yong Zhang [mailto:yong.zhang0@gmail.com]
> > Sent: Tuesday, March 20, 2012 6:41 PM
> > To: Michael J. Wang
> > Cc: mingo@elte.hu; peterz@infradead.org; linux-
> kernel@vger.kernel.org; rostedt@goodmis.org
> > Subject: Re: [PATCH 1/1] scheduler: minor improvement to
> pick_next_highest_task_rt in linux-3.3
> >
> > On Mon, Mar 19, 2012 at 10:26:19PM +0000, Michael J. Wang wrote:
> > > From: Michael J Wang <mjwang@broadcom.com>
> > >
> > > Avoid extra work by continuing on to the next rt_rq if the highest
> prio task in current rt_rq is the same priority as our candidate task.
> > >
> > > Signed-off-by: Michael J Wang <mjwang@broadcom.com>
> > >
> > > ---
> > >
> > > More detailed explanation:  if next is not NULL, then we have found
> a candidate task, and its priority is next->prio.  Now we are looking
> for an even higher priority task in the other rt_rq's.  idx is the
> highest priority in the current candidate rt_rq.  In the current 3.3
> code, if idx is equal to next->prio, we would start scanning the tasks
> in that rt_rq and replace the current candidate task with a task from
> that rt_rq.  But the new task would only have a priority that is equal
> to our previous candidate task, so we have not advanced our goal of
> finding a higher prio task.  So we should avoid the extra work by
> continuing on to the next rt_rq if idx is equal to next->prio.
> > >
> >
> > You should limit characters of each line to 80 if possible.
> >
> > And before sending you patch, linux-source/scripts/checkpatch.pl
> maybe
> > give you some clues whether there is some warning/error. If there
> are,
> > fix them.
> >
> > Only for what your patch wants to show:
> > Reviewed-by: Yong Zhang <yong.zhang0@gmail.com>
> >
> > Thanks,
> > Yong
> >
> > > --- linux-3.3/kernel/sched/rt.c.orig	2012-03-18 16:15:34.000000000
> -0700
> > > +++ linux-3.3/kernel/sched/rt.c	2012-03-19 14:52:54.585391702 -0700
> > > @@ -1403,7 +1403,7 @@ static struct task_struct *pick_next_hig
> > >  next_idx:
> > >  		if (idx >= MAX_RT_PRIO)
> > >  			continue;
> > > -		if (next && next->prio < idx)
> > > +		if (next && next->prio <= idx)
> > >  			continue;
> > >  		list_for_each_entry(rt_se, array->queue + idx, run_list) {
> > >  			struct task_struct *p;
> > >
> > >
> > >
> > >
> > >
> > >
> > > --
> > > 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/
> >
> > --
> > Only stand for myself
> >
> >
> N�����r��y���b�X��ǧv�^�)޺{.n�+����{����zX��\x17��ܨ}���Ơz�&j:+v���\r����zZ+�
> �+zf���h���~����i���z�\x1e�w���?����&�)ߢ^[f��^jǫy�m��@A�a��\x7f�
> 
> 0��h�\x0f�i\x7f
> 
> --
> Only stand for myself

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH 1/1] scheduler: minor improvement to pick_next_highest_task_rt in linux-3.3
  2012-03-21  7:49       ` Michael J. Wang
@ 2012-03-21  9:06         ` Peter Zijlstra
  2012-03-21 18:33           ` Michael J. Wang
  0 siblings, 1 reply; 11+ messages in thread
From: Peter Zijlstra @ 2012-03-21  9:06 UTC (permalink / raw)
  To: Michael J. Wang; +Cc: Yong Zhang, mingo, linux-kernel, rostedt

On Wed, 2012-03-21 at 07:49 +0000, Michael J. Wang wrote:
> > > Should I reformat my patch and send it again?
> > 
> > It'll be better, and I think Peter/Ingo will happy with it.
> > 
> 
> OK.  I will resend now.  Thanks for all your help!

The resend didn't include the more details thing, so I fudged it by
hand. The queued thing now looks like this:

---
Subject: sched, rt: Minor improvement to pick_next_highest_task_rt
From: Michael J Wang <mjwang@broadcom.com>
Date: Mon, 19 Mar 2012 22:26:19 +0000

Avoid extra work by continuing on to the next rt_rq if the highest
prio task in current rt_rq is the same priority as our candidate
task.

More detailed explanation:  if next is not NULL, then we have found a
candidate task, and its priority is next->prio.  Now we are looking
for an even higher priority task in the other rt_rq's.  idx is the
highest priority in the current candidate rt_rq.  In the current 3.3
code, if idx is equal to next->prio, we would start scanning the tasks
in that rt_rq and replace the current candidate task with a task from
that rt_rq.  But the new task would only have a priority that is equal
to our previous candidate task, so we have not advanced our goal of
finding a higher prio task.  So we should avoid the extra work by
continuing on to the next rt_rq if idx is equal to next->prio.

Signed-off-by: Michael J Wang <mjwang@broadcom.com>
Acked-by: Steven Rostedt <rostedt@goodmis.org>
Reviewed-by: Yong Zhang <yong.zhang0@gmail.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/2EF88150C0EF2C43A218742ED384C1BC0FC83D6B@IRVEXCHMB08.corp.ad.broadcom.com
---
 kernel/sched/rt.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -1428,7 +1428,7 @@ static struct task_struct *pick_next_hig
 next_idx:
 		if (idx >= MAX_RT_PRIO)
 			continue;
-		if (next && next->prio < idx)
+		if (next && next->prio <= idx)
 			continue;
 		list_for_each_entry(rt_se, array->queue + idx, run_list) {
 			struct task_struct *p;


^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH 1/1] scheduler: minor improvement to pick_next_highest_task_rt in linux-3.3
  2012-03-21  9:06         ` Peter Zijlstra
@ 2012-03-21 18:33           ` Michael J. Wang
  2012-03-21 19:35             ` Peter Zijlstra
  0 siblings, 1 reply; 11+ messages in thread
From: Michael J. Wang @ 2012-03-21 18:33 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: Yong Zhang, mingo, linux-kernel, rostedt, Michael J. Wang

> From: Peter Zijlstra [mailto:peterz@infradead.org]
> Sent: Wednesday, March 21, 2012 2:07 AM
> To: Michael J. Wang
> Cc: Yong Zhang; mingo@elte.hu; linux-kernel@vger.kernel.org;
> rostedt@goodmis.org
> Subject: RE: [PATCH 1/1] scheduler: minor improvement to
> pick_next_highest_task_rt in linux-3.3
> 
> On Wed, 2012-03-21 at 07:49 +0000, Michael J. Wang wrote:
> > > > Should I reformat my patch and send it again?
> > >
> > > It'll be better, and I think Peter/Ingo will happy with it.
> > >
> >
> > OK.  I will resend now.  Thanks for all your help!
> 
> The resend didn't include the more details thing, so I fudged it by
> hand. The queued thing now looks like this:
> 

OK.  Thanks.  I was afraid the details were too verbose when the fix
was obvious to the experts.  Anyways, I now know the format you
are expecting, so I will do better next time.

Michael

> ---
> Subject: sched, rt: Minor improvement to pick_next_highest_task_rt
> From: Michael J Wang <mjwang@broadcom.com>
> Date: Mon, 19 Mar 2012 22:26:19 +0000
> 
> Avoid extra work by continuing on to the next rt_rq if the highest
> prio task in current rt_rq is the same priority as our candidate
> task.
> 
> More detailed explanation:  if next is not NULL, then we have found a
> candidate task, and its priority is next->prio.  Now we are looking
> for an even higher priority task in the other rt_rq's.  idx is the
> highest priority in the current candidate rt_rq.  In the current 3.3
> code, if idx is equal to next->prio, we would start scanning the tasks
> in that rt_rq and replace the current candidate task with a task from
> that rt_rq.  But the new task would only have a priority that is equal
> to our previous candidate task, so we have not advanced our goal of
> finding a higher prio task.  So we should avoid the extra work by
> continuing on to the next rt_rq if idx is equal to next->prio.
> 
> Signed-off-by: Michael J Wang <mjwang@broadcom.com>
> Acked-by: Steven Rostedt <rostedt@goodmis.org>
> Reviewed-by: Yong Zhang <yong.zhang0@gmail.com>
> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Link:
> http://lkml.kernel.org/r/2EF88150C0EF2C43A218742ED384C1BC0FC83D6B@IRVEX
> CHMB08.corp.ad.broadcom.com
> ---
>  kernel/sched/rt.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- a/kernel/sched/rt.c
> +++ b/kernel/sched/rt.c
> @@ -1428,7 +1428,7 @@ static struct task_struct *pick_next_hig
>  next_idx:
>  		if (idx >= MAX_RT_PRIO)
>  			continue;
> -		if (next && next->prio < idx)
> +		if (next && next->prio <= idx)
>  			continue;
>  		list_for_each_entry(rt_se, array->queue + idx, run_list) {
>  			struct task_struct *p;
> 



^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH 1/1] scheduler: minor improvement to pick_next_highest_task_rt in linux-3.3
  2012-03-21 18:33           ` Michael J. Wang
@ 2012-03-21 19:35             ` Peter Zijlstra
  2012-03-22  7:50               ` Ingo Molnar
  0 siblings, 1 reply; 11+ messages in thread
From: Peter Zijlstra @ 2012-03-21 19:35 UTC (permalink / raw)
  To: Michael J. Wang; +Cc: Yong Zhang, mingo, linux-kernel, rostedt

On Wed, 2012-03-21 at 18:33 +0000, Michael J. Wang wrote:
> 
> OK.  Thanks.  I was afraid the details were too verbose when the fix
> was obvious to the experts.  Anyways, I now know the format you
> are expecting, so I will do better next time. 

Since you took the trouble to write it, I thought it worth the trouble
to include.

Very often Changelogs are way too spartan (my own included), I can't
recall the amount of times I've kicked myself for leaving out some - at
the time - obvious details.

So I prefer people to go overboard a bit and err on the side of too much
information :-)

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/1] scheduler: minor improvement to pick_next_highest_task_rt in linux-3.3
  2012-03-21 19:35             ` Peter Zijlstra
@ 2012-03-22  7:50               ` Ingo Molnar
  0 siblings, 0 replies; 11+ messages in thread
From: Ingo Molnar @ 2012-03-22  7:50 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: Michael J. Wang, Yong Zhang, mingo, linux-kernel, rostedt


* Peter Zijlstra <peterz@infradead.org> wrote:

> On Wed, 2012-03-21 at 18:33 +0000, Michael J. Wang wrote:
> > 
> > OK.  Thanks.  I was afraid the details were too verbose when the fix
> > was obvious to the experts.  Anyways, I now know the format you
> > are expecting, so I will do better next time. 
> 
> Since you took the trouble to write it, I thought it worth the trouble
> to include.
> 
> Very often Changelogs are way too spartan (my own included), I can't
> recall the amount of times I've kicked myself for leaving out some - at
> the time - obvious details.
> 
> So I prefer people to go overboard a bit and err on the side 
> of too much information :-)

Yeah. Current trends are: for every 1000 patches sent there's 
maybe one patch that has a tad too much information in its 
changelog - but instead offers good entertainment in the 
changelog so it's still perfectly fine. 990 patches have too 
little information. The remaining 9 are just fine.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [tip:sched/urgent] sched/rt: Improve pick_next_highest_task_rt()
  2012-03-19 22:26 [PATCH 1/1] scheduler: minor improvement to pick_next_highest_task_rt in linux-3.3 Michael J. Wang
  2012-03-20 13:59 ` Steven Rostedt
  2012-03-21  1:40 ` Yong Zhang
@ 2012-03-27 15:35 ` tip-bot for Michael J Wang
  2 siblings, 0 replies; 11+ messages in thread
From: tip-bot for Michael J Wang @ 2012-03-27 15:35 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, a.p.zijlstra, rostedt, mjwang, tglx,
	yong.zhang0

Commit-ID:  1b028abc779b67b699daff55e27d2432f8d92666
Gitweb:     http://git.kernel.org/tip/1b028abc779b67b699daff55e27d2432f8d92666
Author:     Michael J Wang <mjwang@broadcom.com>
AuthorDate: Mon, 19 Mar 2012 22:26:19 +0000
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 27 Mar 2012 14:52:12 +0200

sched/rt: Improve pick_next_highest_task_rt()

Avoid extra work by continuing on to the next rt_rq if the highest
prio task in current rt_rq is the same priority as our candidate
task.

More detailed explanation:  if next is not NULL, then we have found a
candidate task, and its priority is next->prio.  Now we are looking
for an even higher priority task in the other rt_rq's.  idx is the
highest priority in the current candidate rt_rq.  In the current 3.3
code, if idx is equal to next->prio, we would start scanning the tasks
in that rt_rq and replace the current candidate task with a task from
that rt_rq.  But the new task would only have a priority that is equal
to our previous candidate task, so we have not advanced our goal of
finding a higher prio task.  So we should avoid the extra work by
continuing on to the next rt_rq if idx is equal to next->prio.

Signed-off-by: Michael J Wang <mjwang@broadcom.com>
Acked-by: Steven Rostedt <rostedt@goodmis.org>
Reviewed-by: Yong Zhang <yong.zhang0@gmail.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/2EF88150C0EF2C43A218742ED384C1BC0FC83D6B@IRVEXCHMB08.corp.ad.broadcom.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/sched/rt.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index b60dad7..44af55e 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -1428,7 +1428,7 @@ static struct task_struct *pick_next_highest_task_rt(struct rq *rq, int cpu)
 next_idx:
 		if (idx >= MAX_RT_PRIO)
 			continue;
-		if (next && next->prio < idx)
+		if (next && next->prio <= idx)
 			continue;
 		list_for_each_entry(rt_se, array->queue + idx, run_list) {
 			struct task_struct *p;

^ permalink raw reply related	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2012-03-27 15:36 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-03-19 22:26 [PATCH 1/1] scheduler: minor improvement to pick_next_highest_task_rt in linux-3.3 Michael J. Wang
2012-03-20 13:59 ` Steven Rostedt
2012-03-21  1:40 ` Yong Zhang
2012-03-21  1:56   ` Michael J. Wang
2012-03-21  2:12     ` Yong Zhang
2012-03-21  7:49       ` Michael J. Wang
2012-03-21  9:06         ` Peter Zijlstra
2012-03-21 18:33           ` Michael J. Wang
2012-03-21 19:35             ` Peter Zijlstra
2012-03-22  7:50               ` Ingo Molnar
2012-03-27 15:35 ` [tip:sched/urgent] sched/rt: Improve pick_next_highest_task_rt() tip-bot for Michael J Wang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).