Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1402756 > unrolled thread

Re: [PATCH] sched/cputime: add steal time support to full dynticks CPU time accounting

Started byRik van Riel <riel@redhat.com>
First post2016-05-18 09:20 +0200
Last post2016-05-18 09:50 +0200
Articles 2 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH] sched/cputime: add steal time support to full dynticks  CPU time accounting Rik van Riel <riel@redhat.com> - 2016-05-18 09:20 +0200
    Re: [PATCH] sched/cputime: add steal time support to full dynticks  CPU time accounting Wanpeng Li <kernellwp@gmail.com> - 2016-05-18 09:50 +0200

#1402756 — Re: [PATCH] sched/cputime: add steal time support to full dynticks CPU time accounting

FromRik van Riel <riel@redhat.com>
Date2016-05-18 09:20 +0200
SubjectRe: [PATCH] sched/cputime: add steal time support to full dynticks CPU time accounting
Message-ID<rA3VU-7ii-9@gated-at.bofh.it>

[Multipart message — attachments visible in raw view] — view raw

On Tue, 2016-05-10 at 13:34 +0800, Wanpeng Li wrote:
> 
> +++ b/kernel/sched/cputime.c
> 
> @@ -691,8 +691,11 @@ static cputime_t get_vtime_delta(struct
> task_struct *tsk)
>  
>  static void __vtime_account_system(struct task_struct *tsk)
>  {
> +	unsigned long steal_time = steal_account_process_tick();
>  	cputime_t delta_cpu = get_vtime_delta(tsk);
>  
> +	delta_cpu = steal_time ? (delta_cpu -
> +		jiffies_to_cputime(steal_time)) : delta_cpu;
>  	account_system_time(tsk, irq_count(), delta_cpu,
> cputime_to_scaled(delta_cpu));
>  }
>  

Sorry to have to go back on my previous email, but
this is now a NAK

The above code can end up passing a negative number
to account_system_time(), which in turn can cause a
divide by zero in scale_stime()

The code needs to ensure that if all the time that
passed was accounted as steal time (which could be
more jiffies than expected, due to remaining partial
jiffies in steal_account_process_tick), the function
does not call account_system_time().

-- 
All Rights Reversed.

[toc] | [next] | [standalone]


#1402768

FromWanpeng Li <kernellwp@gmail.com>
Date2016-05-18 09:50 +0200
Message-ID<rA4oW-7vr-11@gated-at.bofh.it>
In reply to#1402756
2016-05-18 15:10 GMT+08:00 Rik van Riel <riel@redhat.com>:
> On Tue, 2016-05-10 at 13:34 +0800, Wanpeng Li wrote:
>>
>> +++ b/kernel/sched/cputime.c
>>
>> @@ -691,8 +691,11 @@ static cputime_t get_vtime_delta(struct
>> task_struct *tsk)
>>
>>  static void __vtime_account_system(struct task_struct *tsk)
>>  {
>> +     unsigned long steal_time = steal_account_process_tick();
>>       cputime_t delta_cpu = get_vtime_delta(tsk);
>>
>> +     delta_cpu = steal_time ? (delta_cpu -
>> +             jiffies_to_cputime(steal_time)) : delta_cpu;
>>       account_system_time(tsk, irq_count(), delta_cpu,
>> cputime_to_scaled(delta_cpu));
>>  }
>>
>
> Sorry to have to go back on my previous email, but
> this is now a NAK
>
> The above code can end up passing a negative number
> to account_system_time(), which in turn can cause a
> divide by zero in scale_stime()
>
> The code needs to ensure that if all the time that
> passed was accounted as steal time (which could be
> more jiffies than expected, due to remaining partial
> jiffies in steal_account_process_tick), the function
> does not call account_system_time().

Yeah, I will fix it in next version, thank Rik very much for debugging
with me to figure out the root cause of divide zero.

Regards,
Wanpeng Li

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web