All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: John Stultz <jstultz@google.com>, LKML <linux-kernel@vger.kernel.org>
Cc: John Stultz <jstultz@google.com>,
	Anna-Maria Behnsen <anna-maria@linutronix.de>,
	Frederic Weisbecker <frederic@kernel.org>,
	Ingo Molnar <mingo@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Juri Lelli <juri.lelli@redhat.com>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
	Valentin Schneider <vschneid@redhat.com>,
	Stephen Boyd <sboyd@kernel.org>,
	Yury Norov <yury.norov@gmail.com>,
	Bitao Hu <yaoma@linux.alibaba.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	kernel-team@android.com
Subject: Re: [RFC][PATCH 1/3] time/tick: Pipe tick count down through cputime accounting
Date: Tue, 28 Jan 2025 15:44:43 +0100	[thread overview]
Message-ID: <877c6f80bo.ffs@tglx> (raw)
In-Reply-To: <20250128063301.3879317-2-jstultz@google.com>

On Mon, Jan 27 2025 at 22:32, John Stultz wrote:
> In working up the dynHZ patch, I found that the skipping of
> ticks would result in large latencies for itimers.
>
> As I dug into it, I realized there is still some logic that
> assumes we don't miss ticks, resulting in late expiration of
> cputime timers.
>
> So this patch pipes the actual number of ticks passed down
> through cputime accounting.

This word salad does not qualify as a proper technical changelog. See
Documentation/

>  /*
>   * Must be called with interrupts disabled !
>   */
> -static void tick_do_update_jiffies64(ktime_t now)
> +static unsigned long tick_do_update_jiffies64(ktime_t now)
>  {
>  	unsigned long ticks = 1;
>  	ktime_t delta, nextp;
> @@ -70,7 +70,7 @@ static void tick_do_update_jiffies64(ktime_t now)
>  	 */
>  	if (IS_ENABLED(CONFIG_64BIT)) {
>  		if (ktime_before(now, smp_load_acquire(&tick_next_period)))
> -			return;
> +			return 0;

So if the CPU's tick handler does not update jiffies, then this returns
zero ticks....

> -static void tick_sched_do_timer(struct tick_sched *ts, ktime_t now)
> +static unsigned long tick_sched_do_timer(struct tick_sched *ts, ktime_t now)
>  {
>  	int tick_cpu, cpu = smp_processor_id();
> -
> +	unsigned long ticks = 0;

And you have also zero ticks, when the CPU is not the tick_cpu:

>  	/* Check if jiffies need an update */
>  	if (tick_cpu == cpu)
> -		tick_do_update_jiffies64(now);
> +		ticks = tick_do_update_jiffies64(now);

...

> +	return ticks;
>  }
>  
> -static void tick_sched_handle(struct tick_sched *ts, struct pt_regs *regs)
> +static void tick_sched_handle(struct tick_sched *ts, unsigned long ticks, struct pt_regs *regs)
>  {
>  	/*
>  	 * When we are idle and the tick is stopped, we have to touch
> @@ -264,7 +266,7 @@ static void tick_sched_handle(struct tick_sched *ts, struct pt_regs *regs)
>  	    tick_sched_flag_test(ts, TS_FLAG_STOPPED)) {
>  		touch_softlockup_watchdog_sched();
>  		if (is_idle_task(current))
> -			ts->idle_jiffies++;
> +			ts->idle_jiffies += ticks;
>  		/*
>  		 * In case the current tick fired too early past its expected
>  		 * expiration, make sure we don't bypass the next clock reprogramming
> @@ -273,7 +275,7 @@ static void tick_sched_handle(struct tick_sched *ts, struct pt_regs *regs)
>  		ts->next_tick = 0;
>  	}
>  
> -	update_process_times(user_mode(regs));
> +	update_process_times(ticks, user_mode(regs));

Which is then fed to update_process_times() and subsequently to
account_process_ticks().

IOW, any CPUs tick handler which does not actually advance jiffies is
going to account ZERO ticks...

Seriously?

Thanks,

        tglx

  reply	other threads:[~2025-01-28 14:44 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-28  6:32 [RFC][PATCH 0/3] DynamicHZ: Configuring the timer tick rate at boot time John Stultz
2025-01-28  6:32 ` [RFC][PATCH 1/3] time/tick: Pipe tick count down through cputime accounting John Stultz
2025-01-28 14:44   ` Thomas Gleixner [this message]
2025-01-29  4:10     ` John Stultz
2025-01-28  6:32 ` [RFC][PATCH 2/3] time/tick: Introduce a dyn_hz boot option John Stultz
2025-01-28  9:07   ` Peter Zijlstra
2025-01-28 17:29     ` John Stultz
2025-01-28 19:30       ` Peter Zijlstra
2025-01-28  6:32 ` [RFC][PATCH 3/3] Kconfig: Add CONFIG_DYN_HZ_DEFAULT to specify the default dynhz= boot option value John Stultz
2025-01-28 16:46 ` [RFC][PATCH 0/3] DynamicHZ: Configuring the timer tick rate at boot time Thomas Gleixner
2025-01-29  6:10   ` John Stultz
2025-01-29  8:09     ` Thomas Gleixner
2025-02-10 16:54       ` David Laight
2025-02-03 11:14   ` Peter Zijlstra
2025-02-10  1:14     ` Qais Yousef
2025-02-10  1:09   ` Qais Yousef

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=877c6f80bo.ffs@tglx \
    --to=tglx@linutronix.de \
    --cc=akpm@linux-foundation.org \
    --cc=anna-maria@linutronix.de \
    --cc=bsegall@google.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=frederic@kernel.org \
    --cc=jstultz@google.com \
    --cc=juri.lelli@redhat.com \
    --cc=kernel-team@android.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mgorman@suse.de \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=sboyd@kernel.org \
    --cc=vincent.guittot@linaro.org \
    --cc=vschneid@redhat.com \
    --cc=yaoma@linux.alibaba.com \
    --cc=yury.norov@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.