[Buildroot] [git commit] package/ntp: Fix building with glibc 2.34+
Peter Seiderer
ps.report at gmx.net
Thu Sep 23 21:34:09 UTC 2021
Hello Arnout, David, all,
On Thu, 23 Sep 2021 11:26:18 +0200, Arnout Vandecappelle <arnout at mind.be> wrote:
> On 23/09/2021 10:23, David Laight wrote:
> > From: Arnout Vandecappelle
> >> Sent: 22 September 2021 21:13
> > ...
> >> On attempt to build ntp with glibc 2.34 the following error happens:
> >> -------------------------------->8------------------------------
> >> In file included from .../output/host/lib/gcc/i586-buildroot-linux-gnu/10.3.0/include-
> >> fixed/pthread.h:42,
> >> from work_thread.c:13:
> >> work_thread.c:45:57: error: missing binary operator before token "("
> >> 45 | #if defined(PTHREAD_STACK_MIN) && THREAD_MINSTACKSIZE < PTHREAD_STACK_MIN
> >> | ^~~~~~~~~~~~~~~~~
> >> -------------------------------->8------------------------------
> >>
> >> That's because starting from glibc 2.34 PTHREAD_STACK_MIN gets determined
> >> dynamically in runtime via sysconf(), see [1].
> > ...
> >> +In glibc 2.34+ PTHREAD_STACK_MIN is not a compile-time constant which
> >> +could mean different stack sizes at runtime on different architectures
> >> +and it also causes compile failure. Default glibc thread stack size
> >> +or 64Kb set by ntp should be good in glibc these days.
> > ...
> >> + #ifndef THREAD_MINSTACKSIZE
> >> + # define THREAD_MINSTACKSIZE (64U * 1024)
> >> + #endif
> >> +-#ifndef __sun
> >> ++#if !defined(__sun) && !defined(__GLIBC__)
> >> + #if defined(PTHREAD_STACK_MIN) && THREAD_MINSTACKSIZE < PTHREAD_STACK_MIN
> >> + # undef THREAD_MINSTACKSIZE
> >> + # define THREAD_MINSTACKSIZE PTHREAD_STACK_MIN
> >
> > I'm not convinced that is the correct fix.
> > The same issues apply to the thread stack as they do to the signal stack.
> > Albeit the sizes are larger.
> >
> > The very reason that PTHREAD_STACK_MIN isn't a compile-time constant
> > is that the value cannot be determined at compile time.
> > While 64k may seem ok for now, there is no reason for it to be enough.
> >
> > Possibly something like:
> > #ifndef PTHREAD_STACK_MIN
> > #define THREAD_MINSTACKSIZE (64U * 1024)
> > #else
> > #define THREAD_MINSTACKSIZE (PTHREAD_STACK_MIN > 64U * 1024 ? PTHREAD_STACK_MIN : 64U * 1024)
> > #endif
> > would be better.
> > Although a form that only evaluates PTHREAD_STACK_MIN once may be better.
>
> It really doesn't matter that much. The only place where THREAD_MINSTACKSIZE
> is used is in the code leading up to this:
>
> if (nstacksize != ostacksize)
> rc = pthread_attr_setstacksize(&thr_attr, nstacksize);
> if (0 != rc)
> msyslog(LOG_ERR,
> "start_blocking_thread:
> pthread_attr_setstacksize(0x%lx -> 0x%lx) -> %s",
> (u_long)ostacksize, (u_long)nstacksize,
> strerror(rc));
>
> Note that there's no error handling other than printing an error message. So,
> if PTHREAD_STACK_MIN turns out to be less than 64K (which is very, very unlikely
> to be the case), then the only thing that will happen is that
> pthread_attr_setstacksize will fail to set the stack size to a lower value and
> we keep the default stack size (and print an error message).
>
> I do agree however that your alternative fix is better (though Id keep the
> existing __sun ifdeffery, just to make the change minimal). So feel free to send
> a patch that changes the existing patch.
>
>
> > Quite why ntp is setting the thread stack size is any bodies guess though.
>
> I guess because there are some threads that need more than 4K stack (which is
> minstacksize on some platforms), and on the other hand they don't want to waste
> stack space on threads that really don't need that much.
>
> > It can only get it wrong in a different way!
>
> For relatively small code, it's not that hard to measure or calculate the
> amount of stack you need.
>
> > Although ISTR one version of ntp thinking itself so important
> > that it locked all its memory.
> > So maybe it is just trying to reduce that locked memory footprint.
>
> I do indeed see an mlockall() in ntpd.c, but I don't see any code doing any
> prefaulting of the stack, so it doesn't seem to be very effective :-)
>
>
> > Given that it's pretty much impossible for ntp to accurately set
> > the time to less than the RTT to the server (it can't tell whether
> > the RTT delays are on the tx or rx side) I can't see that it matters.
>
> Yeah, it looks like ntp is a pretty dead-end project...
Maybe time to provide NTPsec [1] as alternative...
Regards,
Peter
[1] https://www.ntpsec.org/
>
> Regards,
> Arnout
>
> >
> > Anyone who needs sub-ms time sync has bigger problems.
> >
> > David
> >
> > -
> > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> > Registration No: 1397386 (Wales)
> >
> _______________________________________________
> buildroot mailing list
> buildroot at lists.buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot
More information about the buildroot
mailing list