linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Pavan Kondeti <quic_pkondeti@quicinc.com>
To: Anna-Maria Behnsen <anna-maria@linutronix.de>
Cc: <linux-kernel@vger.kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	John Stultz <jstultz@google.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Eric Dumazet <edumazet@google.com>,
	"Rafael J . Wysocki" <rafael.j.wysocki@intel.com>,
	Arjan van de Ven <arjan@infradead.org>,
	"Paul E . McKenney" <paulmck@kernel.org>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Rik van Riel <riel@surriel.com>
Subject: Re: [PATCH v4 00/16] timer: Move from a push remote at enqueue to a pull at expiry model
Date: Tue, 8 Nov 2022 10:07:08 +0530	[thread overview]
Message-ID: <20221108043708.GA9994@hu-pkondeti-hyd.qualcomm.com> (raw)
In-Reply-To: <20221104145737.71236-1-anna-maria@linutronix.de>

Hi Anna-Maria,

On Fri, Nov 04, 2022 at 03:57:21PM +0100, Anna-Maria Behnsen wrote:
> Placing timers at enqueue time on a target CPU based on dubious heuristics
> does not make any sense:
> 
>  1) Most timer wheel timers are canceled or rearmed before they expire.
> 
>  2) The heuristics to predict which CPU will be busy when the timer expires
>     are wrong by definition.
> 
> So placing the timers at enqueue wastes precious cycles.
> 
> The proper solution to this problem is to always queue the timers on the
> local CPU and allow the non pinned timers to be pulled onto a busy CPU at
> expiry time.
> 
> Therefore split the timer storage into local pinned and global timers:
> Local pinned timers are always expired on the CPU on which they have been
> queued. Global timers can be expired on any CPU.
> 
> As long as a CPU is busy it expires both local and global timers. When a
> CPU goes idle it arms for the first expiring local timer. If the first
> expiring pinned (local) timer is before the first expiring movable timer,
> then no action is required because the CPU will wake up before the first
> movable timer expires. If the first expiring movable timer is before the
> first expiring pinned (local) timer, then this timer is queued into a idle
> timerqueue and eventually expired by some other active CPU.
> 
> To avoid global locking the timerqueues are implemented as a hierarchy. The
> lowest level of the hierarchy holds the CPUs. The CPUs are associated to
> groups of 8, which are seperated per node. If more than one CPU group
> exist, then a second level in the hierarchy collects the groups. Depending
> on the size of the system more than 2 levels are required. Each group has a
> "migrator" which checks the timerqueue during the tick for remote expirable
> timers.
> 
> If the last CPU in a group goes idle it reports the first expiring event in
> the group up to the next group(s) in the hierarchy. If the last CPU goes
> idle it arms its timer for the first system wide expiring timer to ensure
> that no timer event is missed.
> 
> 
> Testing
> ~~~~~~~
> 
> The impact of wasting cycles during enqueue by using the heuristic in
> contrast to always queueing the timer on the local CPU was measured with a
> micro benchmark. Therefore a timer is enqueued and dequeued in a loop with
> 1000 repetitions on a isolated CPU. The time the loop takes is measured. A
> quater of the remaining CPUs was kept busy. This measurement was repeated
> several times. With the patch queue the average duration was reduced by
> approximately 25%.
> 
> 	145ns	plain v6
> 	109ns	v6 with patch queue
> 
> 
> Furthermore the impact of residence in deep idle states of an idle system
> was investigated. The patch queue doesn't downgrade this behavior.
> 
> 
> During testing on a mostly idle machine a ping pong game could be observed:
> a process_timeout timer is expired remotely on a non idle CPU. Then the CPU
> where the schedule_timeout() was executed to enqueue the timer comes out of
> idle and restarts the timer using schedule_timeout() and goes back to idle
> again. This is due to the fair scheduler which tries to keep the task on
> the CPU which it previously executed on.
> 
> 
> Next Steps
> ~~~~~~~~~~
> 
> Simple deferrable timers are no longer required as they can be converted to
> global timers. If a CPU goes idle, a formerly deferrable timer will not
> prevent the CPU to sleep as long as possible. Only the last migrator CPU
> has to take care of them. Deferrable timers with timer pinned flags needs
> to be expired on the specified CPU but must not prevent CPU from going
> idle. They require their own timer base which is never taken into account
> when calculating the next expiry time. This conversation and required
> cleanup will be done in a follow up series.
> 

Taking non-pinned deferrable timers case, they are queued on their own base
and its expiry is not taken into account while programming the next timer
event during idle.

Can you elaborate on "Simple deferrable timers are no longer required as they
can be converted to global timers" statement?

Though they can be on global base, we still need to find a way to distinguish
them aginst the normal global timers so that the last migrator can program
the next timer event without taking these deferrable timer expiry into
account? IOW, a deferrable timer should not bring a completely idle system out
of idle to serve the deferrable timer.

When the deferrable timers will be queued on global base, once a CPU comes out
of idle and serve the timers on global base, the deferrable timers also would
be served. This is a welcoming change. We would see a truly deferrable global
timer something we would be interested in. [1] has some background on this.

[1]
https://lore.kernel.org/lkml/1430188744-24737-1-git-send-email-joonwoop@codeaurora.org/

Thanks,
Pavan

  parent reply	other threads:[~2022-11-08  4:40 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-04 14:57 [PATCH v4 00/16] timer: Move from a push remote at enqueue to a pull at expiry model Anna-Maria Behnsen
2022-11-04 14:57 ` [PATCH v4 01/16] tick-sched: Warn when next tick seems to be in the past Anna-Maria Behnsen
2022-11-04 14:57 ` [PATCH v4 02/16] timer: Move store of next event into __next_timer_interrupt() Anna-Maria Behnsen
2022-11-04 14:57 ` [PATCH v4 03/16] timer: Split next timer interrupt logic Anna-Maria Behnsen
2022-11-04 14:57 ` [PATCH v4 04/16] timer: Rework idle logic Anna-Maria Behnsen
2022-11-04 14:57 ` [PATCH v4 05/16] add_timer_on(): Make sure callers have TIMER_PINNED flag Anna-Maria Behnsen
2022-11-04 16:43   ` Frederic Weisbecker
2022-11-07  8:11     ` Anna-Maria Behnsen
2022-11-07 10:11       ` Frederic Weisbecker
2022-11-04 14:57 ` [PATCH v4 06/16] timer: Keep the pinned timers separate from the others Anna-Maria Behnsen
2022-11-04 14:57 ` [PATCH v4 07/16] timer: Retrieve next expiry of pinned/non-pinned timers seperately Anna-Maria Behnsen
2022-11-07 11:58   ` Frederic Weisbecker
2022-11-04 14:57 ` [PATCH v4 08/16] timer: Rename get_next_timer_interrupt() Anna-Maria Behnsen
2022-11-07 12:13   ` Frederic Weisbecker
2022-11-04 14:57 ` [PATCH v4 09/16] timer: Split out "get next timer interrupt" functionality Anna-Maria Behnsen
2022-11-07 12:42   ` Frederic Weisbecker
2022-11-08 15:30     ` Anna-Maria Behnsen
2022-11-04 14:57 ` [PATCH v4 10/16] timer: Add get next timer interrupt functionality for remote CPUs Anna-Maria Behnsen
2022-11-04 14:57 ` [PATCH v4 11/16] timer: Restructure internal locking Anna-Maria Behnsen
2022-11-04 14:57 ` [PATCH v4 12/16] timer: Check if timers base is handled already Anna-Maria Behnsen
2022-11-04 14:57 ` [PATCH v4 13/16] tick/sched: Split out jiffies update helper function Anna-Maria Behnsen
2022-11-04 14:57 ` [PATCH v4 14/16] timer: Implement the hierarchical pull model Anna-Maria Behnsen
2022-11-07 22:07   ` Frederic Weisbecker
2022-11-08 16:16     ` Anna-Maria Behnsen
2022-11-09 17:12       ` Frederic Weisbecker
2022-11-08 10:47   ` Frederic Weisbecker
2022-11-08 17:02     ` Anna-Maria Behnsen
2022-11-09 17:13       ` Frederic Weisbecker
2022-11-08 11:48   ` Frederic Weisbecker
2022-11-09 16:39   ` Frederic Weisbecker
2022-11-10  6:34     ` Anna-Maria Behnsen
2022-11-14 13:09   ` Frederic Weisbecker
2022-11-15 11:31   ` Frederic Weisbecker
2022-11-24  7:47     ` Anna-Maria Behnsen
2022-11-28 16:20       ` Frederic Weisbecker
2022-11-29 10:30         ` Frederic Weisbecker
2022-11-16 13:40   ` Frederic Weisbecker
2022-11-04 14:57 ` [PATCH v4 15/16] timer_migration: Add tracepoints Anna-Maria Behnsen
2022-11-04 14:57 ` [PATCH v4 16/16] timer: Always queue timers on the local CPU Anna-Maria Behnsen
2022-11-08  4:37 ` Pavan Kondeti [this message]
2022-11-08 15:06   ` [PATCH v4 00/16] timer: Move from a push remote at enqueue to a pull at expiry model Anna-Maria Behnsen
2022-11-08 16:04     ` Pavan Kondeti
2022-11-08 17:39       ` Anna-Maria Behnsen
2022-11-08 18:48         ` Pavan Kondeti

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=20221108043708.GA9994@hu-pkondeti-hyd.qualcomm.com \
    --to=quic_pkondeti@quicinc.com \
    --cc=anna-maria@linutronix.de \
    --cc=arjan@infradead.org \
    --cc=edumazet@google.com \
    --cc=fweisbec@gmail.com \
    --cc=jstultz@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=paulmck@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rafael.j.wysocki@intel.com \
    --cc=riel@surriel.com \
    --cc=tglx@linutronix.de \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).