linux-kernel - Re: [PATCH] sched/rt: overrun could happen in start_hrtick

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: <53BB4510.5090704@huawei.com>
Date:	Tue, 8 Jul 2014 09:10:40 +0800
From:	"xiaofeng.yan" <xiaofeng.yan@...wei.com>
To:	Peter Zijlstra <peterz@...radead.org>
CC:	<mingo@...hat.com>, <linux-kernel@...r.kernel.org>,
	<xiaofeng.yan2012@...il.com>, Juri Lelli <juri.lelli@...il.com>
Subject: Re: [PATCH] sched/rt: overrun could happen in start_hrtick_dl

On 2014/7/7 16:41, Peter Zijlstra wrote:
> On Fri, Jul 04, 2014 at 12:02:21PM +0000, xiaofeng.yan wrote:
>> It could be wrong for the precision of runtime and deadline
>> when the precision is within microsecond level. For example:
>> Task runtime deadline period
>>   P1   200us   500us   500us
>>
>> This case need enbale HRTICK feature by the next command
>> PC#echo "HRTICK" > /sys/kernel/debug/sched_features
>> PC#./schedtool -E -t 200000:500000 -e ./test&
>> PC#trace-cmd record -e sched_switch
> Are you actually using HRTICK ?
yes, If HRTICK is close , then all of runtime and deadline will be wrong.
>> Some of runtime and deadline run with millisecond level by
>> reading kernershark.
>> The problem is caused by a conditional judgment "delta > 10000".
>> Because no hrtimer start up to control the runtime when runtime is less than 10us.
>> So the process will continue to run until tick-period coming.
>> For fixing this problem, Let delta is equal to 10us when it is less than 10us.
>> So the hrtimer will start up to control the end of process.
>>
>> Signed-off-by: xiaofeng.yan <xiaofeng.yan@...wei.com>
> Always when sending patches for deadline, also CC Juri.
>
>> ---
>>   kernel/sched/deadline.c |    6 ++----
>>   1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
>> index fc4f98b1..dfefa82 100644
>> --- a/kernel/sched/deadline.c
>> +++ b/kernel/sched/deadline.c
>> @@ -997,10 +997,8 @@ static void check_preempt_curr_dl(struct rq *rq, struct task_struct *p,
>>   #ifdef CONFIG_SCHED_HRTICK
>>   static void start_hrtick_dl(struct rq *rq, struct task_struct *p)
>>   {
>> -	s64 delta = p->dl.dl_runtime - p->dl.runtime;
>> -
>> -	if (delta > 10000)
>> -		hrtick_start(rq, p->dl.runtime);
>> +	s64 delta = p->dl.runtime > 10000 ? p->dl.runtime : 10000;
>> +	hrtick_start(rq, delta);
> Yeah, that looks funny. And seeing how the only other user does
> something similar:
>
> hrtick_start_fair()
> 	delta = max(10000ULL, delta)
> 	hrtick_start(rq, delta)
I will modify my code according to your suggest.
>
> Does it make sense to move this max() into hrtick_start()?
>
> Also; and I don't think you mentioned that but did fix, the argument to
> hrtick_start() is wrong, it should be the delta, not the absolute
> timeout.
Perhaps , if the runtime  is less than 10us,  the context switch 
overhead for system could be closed to 10us.
So it could loss more then you gain.

Thanks for your reply.

Thanks
Yan

--
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/