Re: [PATCH] timekeeping: move multigrain ctime floor handling into timekeeper
From: Jeff Layton
Date: Thu Sep 12 2024 - 08:39:45 EST
On Thu, 2024-09-12 at 14:31 +0200, Christian Brauner wrote:
> On Wed, Sep 11, 2024 at 08:56:56AM GMT, Jeff Layton wrote:
> > The kernel test robot reported a performance regression in some
> > will-it-scale tests due to the multigrain timestamp patches. The data
> > showed that coarse_ctime() was slowing down current_time(), which is
> > called frequently in the I/O path.
> >
> > Add ktime_get_coarse_real_ts64_with_floor(), which returns either the
> > coarse time or the floor as a realtime value. This avoids some of the
> > conversion overhead of coarse_ctime(), and recovers some of the
> > performance in these tests.
> >
> > The will-it-scale pipe1_threads microbenchmark shows these averages on
> > my test rig:
> >
> > v6.11-rc7: 83830660 (baseline)
> > v6.11-rc7 + mgtime series: 77631748 (93% of baseline)
> > v6.11-rc7 + mgtime + this: 81620228 (97% of baseline)
> >
> > Reported-by: kernel test robot <oliver.sang@xxxxxxxxx>
> > Closes: https://lore.kernel.org/oe-lkp/202409091303.31b2b713-oliver.sang@xxxxxxxxx
> > Suggested-by: Arnd Bergmann <arnd@xxxxxxxxxx>
> > Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx>
> > ---
> > Arnd suggested moving this into the timekeeper when reviewing an earlier
> > version of this series, and that turns out to be better for performance.
> >
> > I'm not sure how this should go in (if acceptable). The multigrain
> > timestamp patches that this would affect are in Christian's tree, so
> > that may be best if the timekeeper maintainers are OK with this
> > approach.
>
> We will need this as otherwise we can't really merge the multigrain
> timestamp work with known performance regressions?
Yes, I think we'll need something here. Arnd suggested an alternative
way to do this that might be even better. I'm not 100% sure that it'll
work though since the approach is a bit different.
I'd still like to see this go in for v6.12, so what I'd probably prefer
is to take this patch initially (with the variable name change that
John suggested), and then we can work on the alternative approach in
the meantime
Would that be acceptable?
--
Jeff Layton <jlayton@xxxxxxxxxx>