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

Update set_kernel_timezone to fix intermittent fails #814

Merged
merged 1 commit into from Apr 18, 2019

Conversation

lantis1008
Copy link
Contributor

Sometimes set_kernel_timezone doesn't seem to actually do anything... this was raised in the forum here:
https://www.gargoyle-router.com/phpbb/viewtopic.php?f=6&t=11955

A test scenario which produces the bug (most of the time...) was raised here:
https://www.gargoyle-router.com/phpbb/viewtopic.php?f=6&t=11955&start=10#p52776

This fix seems to make set_kernel_timezone more reliable. I can't pick why or how though, so am cautious of including it into the code without more user testing.

@ericpaulbishop , i'd appreciate an opinion on this one if you've got some time please.

@ericpaulbishop
Copy link
Owner

Seems reasonable. My one comment is that I'm not sure why the "warp_clock" behavior was getting triggered given that according to https://linux.die.net/man/2/settimeofday, it gets triggered when the tv arg is NULL, which it wasn't in the original code (it was being loaded from current). If calling it first with 0 tz_minuteswest like you are in the new version, then calling it again demonstrably fixes (or dramatically reduces the prevelance of) the issue that's probably a good solution.

@lantis1008
Copy link
Contributor Author

Under Linux there are some peculiar "warp clock" semantics associated with the settimeofday() system call if on the very first call (after booting) that has a non-NULL tz argument, the tv argument is NULL and the tz_minuteswest field is nonzero. (The tz_dsttime field should be zero for this case.) In such a case it is assumed that the CMOS clock is on local time, and that it has to be incremented by this amount to get UTC system time. No doubt it is a bad idea to use this feature.

Noting the above, this is what the workaround is designed to do. We call it initially with everything zero'd to appease the kernel, and then call it again with the actual update. This should only be necessary on first boot, but we can't reliably tell if this will be the first time it is running on the system.
If we don't provide a timeval (null field) it should neither be updated nor returned so should be ok.

I think i'll merge it in later this week. Unfortunately we only ever get to test this piece of code a couple times a year.

@ericpaulbishop ericpaulbishop merged commit 141de20 into ericpaulbishop:master Apr 18, 2019
@lantis1008
Copy link
Contributor Author

Turns out my solution was causing a few issues due to it invoking the warp clock solution repeatedly.
It seems the original issue was that the wrapped functions for get/settimeofday were failing, and that changing these to syscalls is all that was needed to fix this.
Doh!

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.

None yet

2 participants