All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: shrikanth hegde <sshegde@linux.vnet.ibm.com>
Cc: peterz@infradead.org, arjan@linux.intel.com, mingo@kernel.org,
	Srikar Dronamraju <srikar@linux.vnet.ibm.com>,
	svaidy@linux.ibm.com, linux-kernel@vger.kernel.org,
	bigeasy@linutronix.de
Subject: Re: [RFC PATCH] hrtimer: interleave timers for improved single thread performance at low utilization
Date: Tue, 31 Jan 2023 12:08:06 +0100	[thread overview]
Message-ID: <877cx30xnt.ffs@tglx> (raw)
In-Reply-To: <5ae3cb09-8c9a-11e8-75a7-cc774d9bc283@linux.vnet.ibm.com>

On Tue, Jan 31 2023 at 11:18, shrikanth hegde wrote:
> As per current design of hrtimer, it uses the _softexpires to trigger the
> timer function.  _softexpires is set as multiple of the period/interval value.

Wrong. _softexpires is _hardexpires + slack. The slack allows for
batching which:

> This will benefit the power saving by less wakeups.

But that has absolutely nothing to do with your problem:

> Due to this, different timers of the same period/interval values align
> and the callbacks functions will be called at the same time.

The whole point of hrtimer_forward_now() is to forward the expiry time
of a timer with the given period so that it expires after 'now'.

That's functionality which is used by a lot of callers to implement
proper periodic timers.

> Came up with a naive patch, more of hack.

A broken hack to be precise because any existing user of
hrtimer_forward() will be broken by this hack.

> Other alternative is to use a slightly modified API for cgroups, so
> that all other timers align and wakeups remain reduced.

I'm not seeing why you need a new API for that. The problem is _NOT_ in
the hrtimer code at all.

Lets look at the math:

    expiry = $INITIAL_EXPIRYVALUE + $N * $PERIOD

If $INITIAL_EXPIRYVALUE is the same then for all instances then
obviously the expiry values of all instances will be all aligned on
multiples of $PERIOD, right?

So why the heck do you need a new hrtimer API? There is an obvious
solution, right?

Thanks,

        tglx

  parent reply	other threads:[~2023-01-31 11:08 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-31  5:48 [RFC PATCH] hrtimer: interleave timers for improved single thread performance at low utilization shrikanth hegde
2023-01-31 10:37 ` Ingo Molnar
2023-01-31 12:09   ` shrikanth hegde
2023-01-31 11:08 ` Thomas Gleixner [this message]
2023-01-31 12:27   ` shrikanth hegde
2023-01-31 14:55 ` Arjan van de Ven
2023-01-31 15:50   ` shrikanth hegde

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=877cx30xnt.ffs@tglx \
    --to=tglx@linutronix.de \
    --cc=arjan@linux.intel.com \
    --cc=bigeasy@linutronix.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=srikar@linux.vnet.ibm.com \
    --cc=sshegde@linux.vnet.ibm.com \
    --cc=svaidy@linux.ibm.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.