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

[Sebastian Huber]

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.

Which sequence of function calls and timings cases the problem? This 
should be definitely a test case.

-- 
embedded brains GmbH
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.huber at embedded-brains.de
phone: +49-89-18 94 741 - 16
fax:   +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/