AW: AW: AW: AW: [PATCH 07/12] kern_tc.c: Remove verification of th_generation for pps functions

[Gabriel.Moyano at dlr.de]

> On 27.04.22 15:13, Gabriel.Moyano at dlr.de wrote:
> >> On 07/04/2022 15:16, Sebastian Huber wrote:
> >>> On 07/04/2022 15:10, Gabriel.Moyano at dlr.de wrote:
> >>>>> Which sequence of function calls and timings cases the problem?
> >>>>> This should be definitely a test case.
> >>>> The generation is updated every time tc_windup() is called. So it
> >>>> is more or less a race condition when it happens.
> >>>
> >>> This is not a race condition. The sequence counter is supposed to
> >>> ensure a consistent state. You can't simply remove this.
> >>>
> >>> Maybe the PPS support requires that we use two timehands even for
> >>> the uniprocessor configuration. For this, we have to understand
> >>> under which conditions and use cases the generation change is an issue.
> >>
> >> In kern_tc.c there is this comment, which is a result from a discussion on a FreeBSD mailing list:
> >>
> >> /*
> >>    * In FreeBSD, the timehands count is a tuning option from two to 16.  The
> >>    * tuning option was added since it is inexpensive and some FreeBSD users asked
> >>    * for it to play around.  The default value is two.  One system which did not
> >>    * work with two timehands was a system with one processor and a specific PPS
> >>    * device.
> >>    *
> >>    * For RTEMS, in uniprocessor configurations, just use one timehand since the
> >>    * update is done with interrupt disabled.
> >>    *
> >>    * In SMP configurations, use a fixed set of two timehands until someone
> >>    * reports an issue.
> >>    */
> >>
> >> We definitely need a test case for this corner case. Adding a second
> >> timehand has a size impact. On a 32-bit machine there is
> >> sizeof(struct
> >> timehands) == 120.
> >>
> > What do you think about something in the middle between adding a second timehand and not adding it at all for non-SMP
> configurations?
> > Let me explain better the idea; the pps_event() uses only 3 variables of the object timehand: th_offsetcount, th_bintime and
> th_scale. The values of them could be saved instruct  pps_state during pps_capture(). This will only happen in case of non-SMP
> configurations. Then, the verifications need to be adapted for SMP or for non-SMP, e.g. the th_generation will be checked at the end
> of pps_capture() for both configurations but only for SMP in pps_event().
> 
> I would keep the synchronization as it is in FreeBSD for now and add a test case which fails in non-SMP configurations. This can be
> used to discuss the trade-offs. We need such a test case anyway.
> 
Unfortunately I could not reproduce the error again. Not sure what has changed that now it is not possible to reproduce it. So, I'll prepare new patches (without this trade-off solution) and send them again in order to get new feedback