Re: [RFC PATCH 2/7] timekeeping: Fix cross-timestamp interpolation corner case decision

From: Peter Hilber
Date: Thu Jul 27 2023 - 06:22:02 EST


On 08.07.23 01:02, John Stultz wrote:
> On Fri, Jun 30, 2023 at 10:12 AM Peter Hilber
> <peter.hilber@xxxxxxxxxxxxxxx> 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.