Re: [PATCH v3 1/4] sched,time: count actually elapsed irq & softirq time

[Frederic Weisbecker]

On Tue, Jul 05, 2016 at 12:47:07PM -0400, Rik van Riel wrote:
> On Tue, 5 Jul 2016 14:40:34 +0200
> Frederic Weisbecker <fweisbec@xxxxxxxxx> wrote:
> 
> > ...
> 
> Thanks for the review Frederic. Here is a fixed version of patch 1,
> the other three still seem to apply cleanly.
> 
> ---8<---
> 
> Subject: sched,time: count actually elapsed irq & softirq time
> 
> Currently, if there was any irq or softirq time during 'ticks'
> jiffies, the entire period will be accounted as irq or softirq
> time.
> 
> This is inaccurate if only a subset of the time was actually spent
> handling irqs, and could conceivably mis-count all of the ticks during
> a period as irq time, when there was some irq and some softirq time.
> 
> This can actually happen when irqtime_account_process_tick is called
> from account_idle_ticks, which can pass a larger number of ticks down
> all at once.
> 
> Fix this by changing irqtime_account_hi_update, irqtime_account_si_update,
> and steal_account_process_ticks to work with cputime_t time units, and
> return the amount of time spent in each mode.
> 
> Rename steal_account_process_ticks to steal_account_process_time, to
> reflect that time is now accounted in cputime_t, instead of ticks.
> 
> Additionally, have irqtime_account_process_tick take into account how
> much time was spent in each of steal, irq, and softirq time.
> 
> The latter could help improve the accuracy of cputime
> accounting when returning from idle on a NO_HZ_IDLE CPU.
> 
> Properly accounting how much time was spent in hardirq and
> softirq time will also allow the NO_HZ_FULL code to re-use
> these same functions for hardirq and softirq accounting.
> 
> Signed-off-by: Rik van Riel <riel@xxxxxxxxxx>
> ---
>  include/asm-generic/cputime_nsecs.h |   2 +
>  kernel/sched/cputime.c              | 124 ++++++++++++++++++++++--------------
>  2 files changed, 79 insertions(+), 47 deletions(-)
> 
> diff --git a/include/asm-generic/cputime_nsecs.h b/include/asm-generic/cputime_nsecs.h
> index 0f1c6f315cdc..918ebb01486c 100644
> --- a/include/asm-generic/cputime_nsecs.h
> +++ b/include/asm-generic/cputime_nsecs.h
> @@ -50,6 +50,8 @@ typedef u64 __nocast cputime64_t;
>  	(__force u64)(__ct)
>  #define nsecs_to_cputime(__nsecs)	\
>  	(__force cputime_t)(__nsecs)
> +#define nsecs_to_cputime64(__nsecs)	\
> +	(__force cputime_t)(__nsecs)

(__force cputime64_t) :-)

Although functionally that shouldn't matter for cputime_nsecs.h. You probably
won't get a warning either.

Reviewed-by: Frederic Weisbecker <fweisbec@xxxxxxxxx>

Thanks!