[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e96ee63e-24c9-2d6e-7ddd-9025bca5f7dd@opensynergy.com>
Date: Thu, 27 Jul 2023 12:21:46 +0200
From: Peter Hilber <peter.hilber@...nsynergy.com>
To: John Stultz <jstultz@...gle.com>
Cc: linux-kernel@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>,
Stephen Boyd <sboyd@...nel.org>,
"Christopher S. Hall" <christopher.s.hall@...el.com>
Subject: Re: [RFC PATCH 2/7] timekeeping: Fix cross-timestamp interpolation
corner case decision
On 08.07.23 01:02, John Stultz wrote:
> On Fri, Jun 30, 2023 at 10:12 AM Peter Hilber
> <peter.hilber@...nsynergy.com> wrote:
>>
>> cycle_between() decides whether get_device_system_crosststamp() will
>> interpolate for older counter readings. So far, cycle_between() checks if
>> parameter test is in the open interval (before, after), when disregarding
>> the special case before > after.
>>
>> The only cycle_between() user, get_device_system_crosststamp(), has the
>> following problem with this: If interval_start == cycles,
>> cycle_between(interval_start, cycles, now) returns false. If a
>> history_begin was supplied to get_device_system_crosststamp(), it will
>> later call cycle_between() again, with effective argument values
>> cycle_between(history_begin->cycles, cycles, cycles). Due to the test
>> against the open interval, cycle_between() returns false again, and
>> get_device_system_crosststamp() returns -EINVAL, when it could have
>> succeeded.
>>
>> Fix this by testing against the closed interval in cycle_between(). This
>> disables interpolation if interval_start == cycles. For the special case
>> before > after, similar arguments hold. Fix this in a similar way.
>>
>> At the second cycle_between() call site, add an extra condition in order to
>> effectively check a half-open interval, which keeps the condition
>> documented above the call site satisfied.
>
> I'm having a little bit of a hard time following this commit message.
> Do you think you might be able to take another swing at it to make it
> a bit clearer?
>
> I get you're going from exclusive to inclusive intervals, but it's not
> very clear why this change is needed.
>
Thanks for the feedback, I'll post v2 soon and will try to come up with
a better commit message.
Powered by blists - more mailing lists