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

[Gabriel.Moyano at dlr.de]

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