linux-kernel - Re: [PATCH v2] timekeeping: move multigrain timestamp floor handling into timekeeper

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20240916104139.exqiayn2o7uniw2p@quack3>
Date: Mon, 16 Sep 2024 12:41:39 +0200
From: Jan Kara <jack@...e.cz>
To: Jeff Layton <jlayton@...nel.org>
Cc: Jan Kara <jack@...e.cz>, Alexander Viro <viro@...iv.linux.org.uk>,
	Christian Brauner <brauner@...nel.org>,
	John Stultz <jstultz@...gle.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Stephen Boyd <sboyd@...nel.org>, Arnd Bergmann <arnd@...nel.org>,
	Vadim Fedorenko <vadim.fedorenko@...ux.dev>,
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
	kernel test robot <oliver.sang@...el.com>
Subject: Re: [PATCH v2] timekeeping: move multigrain timestamp floor handling
 into timekeeper

On Fri 13-09-24 08:01:28, Jeff Layton wrote:
> On Fri, 2024-09-13 at 13:26 +0200, Jan Kara wrote:
> > On Thu 12-09-24 14:02:52, Jeff Layton wrote:
> > > +/**
> > > + * ktime_get_real_ts64_mg - attempt to update floor value and return result
> > > + * @ts:		pointer to the timespec to be set
> > > + * @cookie:	opaque cookie from earlier call to ktime_get_coarse_real_ts64_mg()
> > > + *
> > > + * Get a current monotonic fine-grained time value and attempt to swap
> > > + * it into the floor using @cookie as the "old" value. @ts will be
> > > + * filled with the resulting floor value, regardless of the outcome of
> > > + * the swap.
> > > + */
> > > +void ktime_get_real_ts64_mg(struct timespec64 *ts, u64 cookie)
> > > +{
> > > +	struct timekeeper *tk = &tk_core.timekeeper;
> > > +	ktime_t offset, mono, old = (ktime_t)cookie;
> > > +	unsigned int seq;
> > > +	u64 nsecs;
> > 
> > So what would be the difference if we did instead:
> > 
> > 	old = atomic64_read(&mg_floor);
> > 
> > and not bother with the cookie? AFAIU this could result in somewhat more
> > updates to mg_floor (the contention on the mg_floor cacheline would be the
> > same but there would be more invalidates of the cacheline). OTOH these
> > updates can happen only if max(current_coarse_time, mg_floor) ==
> > inode->i_ctime which is presumably rare? What is your concern that I'm
> > missing?
> > 
> 
> My main concern is the "somewhat more updates to mg_floor". mg_floor is
> a global variable, so one of my main goals is to minimize the updates
> to it. There is no correctness issue in doing what you're saying above
> (AFAICT anyway), but the window of time between when we fetch the
> current floor and try to do the swap will be smaller, and we'll end up
> doing more swaps as a result.
> 
> Do you have any objection to adding the cookie to this API?

No objection as such but as John said, I had also some trouble
understanding what the cookie value is about and what are the constraints
in using it. So if we can live without cookie, it would be a simplification
of the API. If the cooking indeed brings noticeable performance benefit, we
just need to document that the cookie is about performance and how to use
it to get good performance.

								Honza
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR