From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D29639443 for ; Sun, 24 May 2026 15:37:13 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779637034; cv=none; b=kiOfy+GGbTLxRHGk9gex+ggh4xeShb0ppCbN0E6jAShn+1IcGqnRJkFA8el52YABugtG3RJ8/5Qe1uELbv6mEa78Ti/+wGsnGPRCBCq2oOuxSYVH9b7tAD3X7cEcxZKL9nZA3UIQbzE7ghycAlcY0dmZEwjYyn6a2C9IB8EcqaQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779637034; c=relaxed/simple; bh=Dyu5CCTqJCpJpurhCykORdTNa33jckDmQTuVoeus9Wg=; h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID: MIME-Version:Content-Type; b=GcPTlOsK/NCYVjpjxhkEMbbaFjUfNOGD/dKNBmoHIMKUGhCaYEJu67nyoVSeAkFuFFDV2AWbFD8M4clGzDDLeJzNDy/MHb/gDah5LwRAweVpBa1wcY/SL8u0GW7mGOkE6oehkrSyseEi1aOHxCoM06li+VrhmUHTF/QDUxDqpj0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=fKFV+7wN; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="fKFV+7wN" Received: by smtp.kernel.org (Postfix) with UTF8SMTPSA id 94E1F1F000E9; Sun, 24 May 2026 15:37:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1779637033; bh=hT739ekJdyXpwUY9uM7NxtSxttehKaXCf+lOK/58ZiE=; h=From:To:Cc:Subject:In-Reply-To:References:Date; b=fKFV+7wNI0ezm7z3g8t5eFN3q3W1sb7j1jXqt8eB3CRq02JWZI1oUSynqvjZA2XmU IXU1/BdYuXcsybrQkNfBGMO86M3BLl+UU6AgU8jx+vc02PzAG2sTCDHK8EtZpyKO4x lnylB5eO5TzLLjWNVLdxjoRtfeB4JysO2LqnETm8YaqVTgXP8JUmalTV3MJMPM0Wnn PdQ+L7Kmt1zbp0a8v3ufWBUPmoFrN7FrjV3TTX6Zjt37cYAMryiI/OC1cV7DNT4omZ TUBYlUOV8BDcbjgZX3qfw6hdHXX3wAcFCOUI32cz4734jyGnEmbAgtNBxCDiXCT0HM +ROnQ2QOcIOyQ== From: Thomas Gleixner To: David Woodhouse , Miroslav Lichvar Cc: Richard Cochran , Wen Gu , Andrew Lunn , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , John Stultz , Stephen Boyd , Anna-Maria Behnsen , Frederic Weisbecker , Shuah Khan , Peter Zijlstra , Thomas =?utf-8?Q?Wei=C3=9Fschuh?= , Arnd Bergmann , Julien Ridoux , Ryan Luu , linux-kernel@vger.kernel.org, Marcelo Tosatti Subject: Re: [RFC PATCH v2 0/8] timekeeping: Fix draft tracking precision and add feed-forward discipline via vmclock In-Reply-To: <87jyssu9dw.ffs@tglx> References: <20260517220326.4625-1-dwmw2@infradead.org> <0d32da75fa88c92ac0225ef23a9045afdf2ac9fe.camel@infradead.org> <87o6i8vcni.ffs@tglx> <6c7137748ccea288fdf55e7c2e24817ccc1673ef.camel@infradead.org> <874ijzvpl8.ffs@tglx> <00609fa255442648e11e2b5596a3b80a29edcfc8.camel@infradead.org> <87v7cftqey.ffs@tglx> <0b1f41d3709b18c375e6f2743547016afe2ab32c.camel@infradead.org> <87jyssu9dw.ffs@tglx> Date: Sun, 24 May 2026 17:37:09 +0200 Message-ID: <87h5nwu8d6.ffs@tglx> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On Sun, May 24 2026 at 17:15, Thomas Gleixner wrote: > On Fri, May 22 2026 at 17:50, David Woodhouse wrote: >> On Fri, 2026-05-22 at 17:28 +0200, Thomas Gleixner wrote: >>>=20 >>> =C2=A0 git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git timers/pt= p/timekeeping >> >> In 94dd85a8d0a ("timekeeping: Add system_counterval_t to struct >> system_device_crosststamp") my version ditched the system_counterval_t >> on the stack and just used the one in xtstamp directly. > > Which is wrong. I did it the way I did for a very good reason. > >> The convert_base_to_cs() function probably wants to scv->id=3Dcs->id for >> itself anyway; otherwise it's leaving behind an inconsistent >> system_counterval_t object which... will lead to exactly the bug my >> first version of that had, that I see you avoided :) > > No. It can't because that would corrupt the object for the retry case, > which would then hand back the wrong value. > > The object _IS_ consistent because the csid in there is related to the > PTM value and not to the clocksource. The function updates the @cycles > value and leaves everything else untouched. The clock ID for the @cyles > value is guaranteed to be the clock ID of the system clocksource, so > using this is the right thing to do. > > Just because it looks tempting or your AI buddy told you so doesn't make > it correct. And it's worse. We both are wrong :) There is an existing bug in that code for the retry case. Fix below. Thanks, tglx --- --- a/kernel/time/timekeeping.c +++ b/kernel/time/timekeeping.c @@ -1343,12 +1343,14 @@ static bool convert_clock(u64 *val, u32 return true; } =20 -static bool convert_base_to_cs(struct system_counterval_t *scv) +static bool convert_base_to_cs(struct system_counterval_t *scv, u64 *cycle= s) { struct clocksource *cs =3D tk_core.timekeeper.tkr_mono.clock; struct clocksource_base *base; u32 num, den; =20 + *cycles =3D scv->cycles; + /* The timestamp was taken from the time keeper clock source */ if (cs->id =3D=3D scv->cs_id) return true; @@ -1364,10 +1366,10 @@ static bool convert_base_to_cs(struct sy num =3D scv->use_nsecs ? cs->freq_khz : base->numerator; den =3D scv->use_nsecs ? USEC_PER_SEC : base->denominator; =20 - if (!convert_clock(&scv->cycles, num, den)) + if (!convert_clock(cycles, num, den)) return false; =20 - scv->cycles +=3D base->offset; + *cycles +=3D base->offset; return true; } =20 @@ -1479,9 +1481,8 @@ int get_device_system_crosststamp(int (* * installed timekeeper clocksource */ if (system_counterval.cs_id =3D=3D CSID_GENERIC || - !convert_base_to_cs(&system_counterval)) + !convert_base_to_cs(&system_counterval, &cycles)) return -ENODEV; - cycles =3D system_counterval.cycles; =20 /* * Check whether the system counter value provided by the