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

Frederic Weisbecker
2016-07-06T14:15:28Z

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@gmail.com> 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@redhat.com>
> ---
> 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@gmail.com>

Thanks!