AW: AW: [PATCH] kern_tc.c: Update pps_event() for uniprocessor configurations

[Sebastian Huber]

On 01/06/2022 11:19, Gabriel.Moyano at dlr.de wrote:
>> On 01/06/2022 08:55,Gabriel.Moyano at dlr.de  wrote:
>>>> On 30/05/2022 09:29, Gabriel Moyano wrote:
>>>>> Since pps->capgen is not used in uniprocessor configurations, there
>>>>> is no need to verified if it is equal to zero
>>>>>
>>>>> Update #2349
>>>>> ---
>>>>>     cpukit/score/src/kern_tc.c | 4 ++++
>>>>>     1 file changed, 4 insertions(+)
>>>>>
>>>>> diff --git a/cpukit/score/src/kern_tc.c b/cpukit/score/src/kern_tc.c
>>>>> index 92739d8edd..897f81511e 100644
>>>>> --- a/cpukit/score/src/kern_tc.c
>>>>> +++ b/cpukit/score/src/kern_tc.c
>>>>> @@ -2165,7 +2165,11 @@ pps_event(struct pps_state *pps, int event)
>>>>>     	if ((event & pps->ppsparam.mode) == 0)
>>>>>     		return;
>>>>>     	/* If the timecounter was wound up underneath us, bail out. */
>>>>> +#if defined(RTEMS_SMP)
>>>>>     	if (pps->capgen == 0 || pps->capgen !=
>>>>> +#else
>>>>> +	if (pps->capgen !=
>>>>> +#endif
>>>>>     	    atomic_load_acq_int(&pps->capth->th_generation))
>>>>>     		return;
>>>>>
>>>> I think this fix is incomplete. What is with:
>>>>
>>>> void
>>>> pps_capture(struct pps_state *pps)
>>>> {
>>>> 	struct timehands *th;
>>>>
>>>> 	KASSERT(pps != NULL, ("NULL pps pointer in pps_capture"));
>>>> 	th = timehands;
>>>> 	pps->capgen = atomic_load_acq_int(&th->th_generation);
>>>> 	pps->capth = th;
>>>> #ifdef FFCLOCK
>>>> 	pps->capffth = fftimehands;
>>>> #endif
>>>> 	pps->capcount = th->th_counter->tc_get_timecount(th->th_counter);
>>>> 	atomic_thread_fence_acq();
>>>> 	if (pps->capgen != th->th_generation)
>>>> 		pps->capgen = 0;
>>>> }
>>>>
>>>> I don't know why there is this "if" in the code. I will ask on a FreeBSD mailing list.
>>>>
>>> I think it is for the case that th_generation has changed in between
>>> saving the th and th_counter. If this happens pps->capgen is set to 0
>>> and later pps_event() returns earlier. Since for uniprocessor
>>> th_generation equal to 0 is not used, I guess we can removed this if
>>> for those configurations
>> I asked on a FreeBSD mailing list:
>>
>> https://lists.freebsd.org/archives/freebsd-hackers/2022-June/001165.html
>>
> Thanks for asking.
> I'll prepare and send a new patch removing the "if" for uniprocessor configurations just in case.

Please wait with a new patch for a response from FreeBSD.

-- 
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/