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

[Gabriel.Moyano at dlr.de]

> On 07/04/2022 11:56, Gabriel.Moyano at dlr.de wrote:
> >> On 07/04/2022 10:36, Gabriel Moyano wrote:
> >>> ---
> >>>    cpukit/score/src/kern_tc.c | 6 ++++++
> >>>    1 file changed, 6 insertions(+)
> >>>
> >>> diff --git a/cpukit/score/src/kern_tc.c b/cpukit/score/src/kern_tc.c
> >>> index a5c7b4b841..a8ba268ea3 100644
> >>> --- a/cpukit/score/src/kern_tc.c
> >>> +++ b/cpukit/score/src/kern_tc.c
> >>> @@ -2109,9 +2109,11 @@ pps_capture(struct pps_state *pps)
> >>>    	pps->capffth = fftimehands;
> >>>    #endif
> >>>    	pps->capcount =
> >>> th->th_counter->tc_get_timecount(th->th_counter);
> >>> +#ifndef __rtems__
> >>>    	atomic_thread_fence_acq();
> >>>    	if (pps->capgen != th->th_generation)
> >>>    		pps->capgen = 0;
> >>> +#endif /* __rtems__ */
> >>>    }
> >>>
> >>>    void
> >>> @@ -2135,10 +2137,12 @@ pps_event(struct pps_state *pps, int event)
> >>>    	/* Nothing to do if not currently set to capture this event type. */
> >>>    	if ((event & pps->ppsparam.mode) == 0)
> >>>    		return;
> >>> +#ifndef __rtems__
> >>>    	/* If the timecounter was wound up underneath us, bail out. */
> >>>    	if (pps->capgen == 0 || pps->capgen !=
> >>>    	    atomic_load_acq_int(&pps->capth->th_generation))
> >>>    		return;
> >>> +#endif /* __rtems__ */
> >>>
> >>>    	/* Things would be easier with arrays. */
> >>>    	if (event == PPS_CAPTUREASSERT) { @@ -2189,10 +2193,12 @@
> >>> pps_event(struct pps_state *pps, int event)
> >>>    	bintime_addx(&bt, pps->capth->th_scale * tcount);
> >>>    	bintime2timespec(&bt, &ts);
> >>>
> >>> +#ifndef __rtems__
> >>>    	/* If the timecounter was wound up underneath us, bail out. */
> >>>    	atomic_thread_fence_acq();
> >>>    	if (pps->capgen != pps->capth->th_generation)
> >>>    		return;
> >>> +#endif /* __rtems__ */
> >>>
> >>>    	*pcount = pps->capcount;
> >>>    	(*pseq)++;
> >> Why is this change justified?
> > Since only one timehands object is used, the generation is updated more frequently. This can even happen between pps_capture()
> and pps_event(), making the synchronization impossible.
> 
> It seems you didn't test with SMP configurations, since here we have two timehands.
>

Yes, you are right, I didn't test it with SMP configurations (for now).
On possible solution will be to have these verifications only for SMP.
 
> 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.