Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

configure: fix CLOCK_MONOTONIC check for cross-compilation #269

Merged
merged 1 commit into from
Oct 9, 2017
Merged

configure: fix CLOCK_MONOTONIC check for cross-compilation #269

merged 1 commit into from
Oct 9, 2017

Conversation

yann-morin-1998
Copy link
Contributor

In cross-compilation, we can't run test programs, so configure just
bails out. Since there is no cache variable (e.g. ac_cv_blabla, we can't
even provide the correct result.

But in thise case, we don't really need to run to start with; we just
need to check if the toolchain headers know about CLOCK_MONOTONIC.

Signed-off-by: "Yann E. MORIN" [email protected]

In cross-compilation, we can't run test programs, so configure just
bails out. Since there is no cache variable (e.g. ac_cv_blabla, we can't
even provide the correct result.

But in thise case, we don't really need to run to start with; we just
need to check if the toolchain headers know about CLOCK_MONOTONIC.

Signed-off-by: "Yann E. MORIN" <[email protected]>
@yann-morin-1998
Copy link
Contributor Author

Patch resent, to fix a typo in the commit title. Sorry for the noise... :-/

@chrissie-c chrissie-c merged commit ad3da47 into ClusterLabs:master Oct 9, 2017
@chrissie-c
Copy link
Contributor

New patch looks good, Merged, thanks!

@wferi
Copy link
Contributor

wferi commented Oct 12, 2017

Actually, Hurd has a bug here: it defines _POSIX_MONOTONIC_CLOCK to 200809L, returns the same value for a sysconf() query, but clock_getres(CLOCK_MONOTONIC, &ts) still fails with EINVAL. This will be fixed shortly. So this patch breaks the Hurd build, which is fine for now. But the POSIX interface for checking for the monotonic clock option is different. I'm preparing #271 with a supposedly better check.

@jnpkrn
Copy link
Contributor

jnpkrn commented Oct 26, 2017

N.B. clock_getres(CLOCK_MONOTONIC, &ts) failing with Hurd already discussed:
#192 (comment)
(looks like we are running in circles)

@jnpkrn jnpkrn changed the title bconfigure: fix CLOCK_MONOTONIC check for cross-compilation configure: fix CLOCK_MONOTONIC check for cross-compilation Oct 26, 2017
@jnpkrn
Copy link
Contributor

jnpkrn commented Oct 26, 2017

And re AC_RUN_IFELSE suitability for cross-compiling:

I admittedly don't have enough experience with cross-compiles, but merely
from dev's perspective, I claim that intimate pairing up with the target system,
both compile-time and run-time wise, is vital under some circumstances,
especially when less frequented features are to be exercised (relied upon):

Hence it looks to me that it would be highly preferable if ./configure would
supposedly be always run on the target platform, and only its outcomes then
possibly used for cross-compiling (again, not sure if autoconf provides at
least the basic support for such use case). We can indeed have a look on
how to make it easier to inject/pre-cache some such findings to make
cross-compiling achievable, if not easier.

@wferi
Copy link
Contributor

wferi commented Oct 26, 2017

The problem here is that a working CLOCK_MONOTONIC isn't a property of the platform. Under Linux, it depends on the currently running kernel. So the same binary on the same system may find it working one time, then, after a reboot to another kernel, may find it non-working. That's why it needs run-time handling.

@sdake
Copy link

sdake commented Oct 27, 2017

@wferi when the monotonic code (in Corosync) was written ~10 years ago, it was a property of all Linux kernels. Hi-Res timers had just went into the kernel and thank god we were able to use it, as a cluster reset every 6 months (DST change) causes alot of anger among the 20-30k sites using corosync :)

Personally I feel the best solution is to complain furiously when monotonic clocks are not available in the system, but still work correctly at runtime. Correct operation can be achieved at runtime without monotonic clocks, by watching for dst boundaries and adjusting the internal corosync clock. I'm not sure how applicable this approach would be to libqb, but trust me when I say you don't want to run without monotonic without some workaround for DST.

I could not reproduce this field bug (it was poorly reported) and I am in AZ (no DST) so it was never identified until after we added monotonic clocks and operators started reporting Corosync was more reliable on a yearly basis :)

Regarding @jnpkrn's commentary, I did embedded dev for 5 years, and cross compilers (if done right) are truly cross-compiled - they have no dependency on any part of the toolchain in the host os. Only a few LInux vendors managed to get this right.

Cheers
-steve

@wferi
Copy link
Contributor

wferi commented Oct 27, 2017

@sdake I agree that CLOCK_MONOTONIC really should be available and working on every x86 Linux kernel in use nowadays. Still, libc does not guarantee it, so a runtime check is required.
The bizarre thing is that the Hurd libc does guarantee a working CLOCK_MONOTONIC (so runtime checking isn't necessary), then "implements" it by using CLOCK_REALTIME instead...
On the other hand, daylight saving does not affect even CLOCK_REALTIME, that's a higher level representation thing. The realtime clock can go backwards due to settime calls, not DST changes.

@sdake
Copy link

sdake commented Oct 28, 2017

@wferi when timers were implemented in Corosync originally, there was no realtime clock. In other words, libc didn't have clock_gettime(). Instead we were using gettimeofday() and doing math on the clocks. Ya, I know, wrong answer which is why we switched to MONOTONIC. I don't see how a runtime config check can be set on something that sometimes links with the rt shared object, and other times doesn't without dlopen() in the mix. In any regard, eel free to fix as you see fit, but I'd avoid taking out CLOCK_MONOTONIC support as that change drastically improved the stability of Corosync.

Cheers
-steve

@wferi
Copy link
Contributor

wferi commented Oct 30, 2017

@sdake gettimeofday() returns an epoch value, which isn't affected by DST changes either. But let's leave the past... Interesting you brought up the RT library. I removed its last trace in cb5ee92, not knowing the POSIX clock functions also need it below glibc 2.17. Shall I open a PR to put it back? This won't affect the runtime check, though, as the needed libraries depend on the libc version and should be determined by configure as usual. Binary portability across libc versions is out of scope, I think.

@jnpkrn
Copy link
Contributor

jnpkrn commented Nov 15, 2017

Just FYI, eventually managed to get rid of AC_RUN_IFELSE in PR #266.
Though haven't tested how reliable it is beside on-host building.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants