linux-kernel - Re: [PATCH 1/2] sched: push rt tasks only if newly activated tasks have been added

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <480EDD2F.BA47.005A.0@novell.com>
Date:	Wed, 23 Apr 2008 04:54:39 -0600
From:	"Gregory Haskins" <ghaskins@...ell.com>
To:	"Dmitry Adamushko" <dmitry.adamushko@...il.com>
Cc:	<mingo@...e.hu>, "Steven Rostedt" <rostedt@...dmis.org>,
	<chinang.ma@...el.com>, <suresh.b.siddha@...el.com>,
	<arjan@...ux.intel.com>, <willy@...ux.intel.com>,
	<linux-kernel@...r.kernel.org>, <linux-rt-users@...r.kernel.org>
Subject: Re: [PATCH 1/2] sched: push rt tasks only if newly activated
	tasks have been added

>>> On Wed, Apr 23, 2008 at  6:23 AM, in message
<b647ffbd0804230323s20d25697qcf80ca9996e143a7@...l.gmail.com>, "Dmitry
Adamushko" <dmitry.adamushko@...il.com> wrote: 
> 2008/4/23 Gregory Haskins <ghaskins@...ell.com>:
>> [ ... ]
>>
>>  I think we can simplify this further.  We really only need to push here if 
> we are not going to reschedule anytime soon (probably white-space damaged):
>>
>>
>>  --- a/kernel/sched_rt.c
>>
>> +++ b/kernel/sched_rt.c
>>  @@ -1058,11 +1058,14 @@ static void post_schedule_rt(struct rq *rq)
>>         }
>>   }
>>
>>  -
>>  +/*
>>  + * If we are not running and we are not going to reschedule soon, we 
> should
>>  + * try to push tasks away now
>>  + */
>>
>>  static void task_wake_up_rt(struct rq *rq, struct task_struct *p)
>>   {
>>         if (!task_running(rq, p) &&
>>  -           (p->prio >= rq->rt.highest_prio) &&
>>  +           !test_tsk_thread_flag(rq->curr, TIF_NEED_RESCHED) &&
>>             rq->rt.overloaded)
>>                 push_rt_tasks(rq);
>>   }
> 
> 
> It's somewhat suboptimal as it doesn't guarantee that 'p' gets control next.

No it doesn't, but I don't see that as a requirement to work properly.  All we really need is to make sure push_rt_tasks() will be executed in the near future, and a reschedule will do that.  Perhaps I am missing something?

> 
> e.g. 2 tasks (T0 and T1) have been woken up before an actual
> re-schedule takes place. Even if T1 is of lower prio than T0,
> task_wake_up_rt() will see the NEED_RESCHED flag and bail out while it
> would make sense at this moment to push T1 off this cpu.

I think this is "ok".  IMO, we really want to prioritize the push operation from post_schedule() over task_wake_up() because it is the most accurate (since the scheduling decision would be atomic to the rq->lock).  This way we avoid pushing the wrong task away before the schedule has a chance to run.  I think your alg probably approximates this same accuracy, albeit in a more expensive way.  The one thing I can say is an advantage to your approach is that it potentially would migrate the task faster (though it shouldn't be by very much, since the resched is technically pending).  This argument is further compounded by the fact that you would have to subtract the extra overhead of running pick_next_task() et. al. from the gain of avoiding the wait for the reschedule, of course.  This would errode some of the lower-latency argument.

Hmmm...  Im not sure which I like better...simplicity is often nice IMO, but I think either could work.

> 
> 
> p.s. hope you are better today. get well! :-)

Thanks!  I am feeling much better today, though I don't think I am fully out of the woods yet.

> 

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