Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <CAHo-OowM2jRNuvyDf-T8rzr6ZgUztXqY7m_JhuFvQ+uB8N3ZrQ@mail.gmail.com>
 <YHXRWoVIYLL4rYG9@kroah.com> <CAKD1Yr1DnDTELUX2DQtPDtAoDMqCz6dV+TZbBuC1CFm32O8MrA@mail.gmail.com>
 <87r1jbv6jc.ffs@nanos.tec.linutronix.de> <CAKD1Yr1o=zN5K9PaB3wag5xOS2oY6AzEsV6dmL7pnTysK_GOhA@mail.gmail.com>
 <87eef5qbrx.ffs@nanos.tec.linutronix.de>
In-Reply-To: <87eef5qbrx.ffs@nanos.tec.linutronix.de>
From:   Lorenzo Colitti <lorenzo@google.com>
Date:   Wed, 21 Apr 2021 23:08:19 +0900
Message-ID: <CAKD1Yr2qeXseNLcQ9r4niob02jGOXdVeta6OwWF3Ta1dyp1V2Q@mail.gmail.com>
Subject: Re: [PATCH] hrtimer: Update softirq_expires_next correctly after __hrtimer_get_next_event()
To:     Thomas Gleixner <tglx@linutronix.de>
Cc:     Greg KH <gregkh@linuxfoundation.org>,
        =?UTF-8?Q?Maciej_=C5=BBenczykowski?= <zenczykowski@gmail.com>,
        Ingo Molnar <mingo@kernel.org>,
        Anna-Maria Behnsen <anna-maria@linutronix.de>,
        lkml <linux-kernel@vger.kernel.org>,
        mikael.beckius@windriver.com,
        =?UTF-8?Q?Maciej_=C5=BBenczykowski?= <maze@google.com>,
        Will Deacon <will@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, Apr 20, 2021 at 11:19 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>   1) hrtimer is contrary to timer_list not really suited for high
>      frequency start/cancel/start/... cycles of a timer. It's optimized
>      for the start and expire precisely case.

Ack. This is not what the f_ncm gadget code needs. It just needs a
timer to fire "about 300us after the last packet submitted". Under
load the timer will almost never fire since there will always be
another packet coming. So the speed of adding/updating the timer is
much more important than the accuracy. We will try to move it to
timer_list.

>      I assume that's an ARM64 system. ARM64 CPUs have an architected per
>      CPU timer where the reprogramming is pretty fast as it's next to
>      the CPU, but who knows what your system is using.

This system appears to be using timer hardware that is... slow to
program (microseconds). We're looking at whether it's possible to use
the arch timer instead.

> Now in the meantime I looked into __hrtimer_start_range_ns() whether
> that double reprogram can be avoided without creating a total trainwreck
> and imposing penalty on all sensible use cases. Completely untested
> patch below should do the trick and it's not ugly enough that I hate it
> with a passion.

I tested this and in my simple benchmark, the double calls are gone
and hrtimer_start calls tick_program_event approximately once (more
precisely, 90.06% of the time; sometimes it doesn't call it at all).
This is not enough to cancel the regression we're seeing because the
previous code would pretty much never call tick_program_event at all.
But it's definitely better.