|Summary:||adjtimex broken when delta is NULL|
|Product:||Timers||Reporter:||Michael Kerrisk (michael.kerrisk)|
|Component:||Other||Assignee:||john stultz (john.stultz)|
|Severity:||normal||CC:||akpm, drepper, ismail, mingo, protasnb, stsp2, tglx, zippel|
Description Michael Kerrisk 2006-06-28 06:07:18 UTC
Comment 1 john stultz 2007-02-09 13:47:03 UTC
Looking over the context, it does seem like the fix is a bit more complicated. It looks like this would require both a kernel and glibc change (this issue appears to stem from sys_adjtimex's multiplexing of current adjtimex interface along w/ the old adjtime interface). I'm worried that the patch below would fix the old adjtime interface but break the adjtimex interface.
Comment 2 Michael Kerrisk 2007-02-09 19:35:23 UTC
Yes, for all I know the patch I gave may well be insufficient. I don't understand the code there well enough. Nevertheless, things do seem to be broken as they stand, and some type of fix is required...
Comment 3 Natalie Protasevich 2007-07-07 13:22:48 UTC
What is the current state of this issue, will fix be needed eventually? it seems like for portablity adjtime() should be handled properly. Are there any plans to find a solution? Thanks.
Comment 4 Andrew Morton 2007-07-27 16:37:40 UTC
guys, that pain in your ribs is my thumb ;)
Comment 5 john stultz 2007-07-27 17:09:44 UTC
:( The issue is the adjtimex interface multiplexes both the old adjtime and new adjtimex interfaces. The problem being, if nothing is passed in on the modes, you get the get new adjtimex()'s offset value, not the adjtime offset. In order to get the current adjtime offset value, you have to use the ADJ_OFFSET_SINGLESHOT mode, which overwrites the current adjtime offset (and returns the old). The only fix I can imagine for this would be to change the adjtimex interface and introduce a new modes flag, something like: #define ADJ_OFFSET_SINGLESHOT_READ 0x2000 And we can add a simple: if (txc->modes == ADJ_OFFSET_SINGLESHOT_READ) txc->offset = saved_adjust; Then glibc can pass it in if adjtime() is called w/ no new adjustment value. Outside of that I'm not sure I see how to solve this.
Comment 6 john stultz 2007-07-27 17:26:09 UTC
Created attachment 12184 [details] Add ADJ_OFFSET_SS_READ Untested patch to add ADJ_OFFSET_SS_READ modes flag. How does this sound: When passed in the modes field to adjtimex() we will return the current singleshot (adjtime style) adjustment in the timex.offset field without modifying the kernel's time_adjust value. Needs glibc to use this flag in its adjtime() implementation in the case where the new offset is null and the old offset is non-null.
Comment 7 john stultz 2007-07-27 17:28:00 UTC
It should be noted that the existing behavior is documented in the man page for adjtime(): "BUGS Currently, if delta is specified as NULL, no valid information about the outstanding clock adjustment is returned in olddelta. (In this circumstance, adjtime() should return the outstanding clock adjustment, without changing it.) This is the result of a kernel limitation."
Comment 8 Michael Kerrisk 2007-07-28 05:09:14 UTC
(In reply to comment #7) > It should be noted that the existing behavior is documented in the man page > for > adjtime(): > "BUGS > Currently, if delta is specified as NULL, no valid information > about > the outstanding clock adjustment is returned in olddelta. (In > this > circumstance, adjtime() should return the outstanding clock > adjustment, > without changing it.) This is the result of a kernel limitation." Yes, it's documented in the man page because as well as creating this bug report, I also wrote the man page text.
Comment 9 Natalie Protasevich 2007-07-29 02:58:38 UTC
*** Bug 7491 has been marked as a duplicate of this bug. ***
Comment 10 Natalie Protasevich 2007-07-29 03:03:10 UTC
Sorry, disregard #9, it was operator error.
Comment 11 Ingo Molnar 2007-11-18 07:52:19 UTC
Ulrich, what do you think about this new proposed ADJ_OFFSET_SINGLESHOT_READ modes flag? glibc would use this flag in its adjtime() implementation in the case where the new offset is null and the old offset is non-null. old kernels just ignore unknown mode flags and return TIME_OK. If this is OK to you then we can add this fix to the upstream kernel.
Comment 12 Ingo Molnar 2007-11-18 08:07:05 UTC
We've added this to the x86 tree's "still cooking" queue of patches. This means it will show up in -mm for testing, but it will need Ulrich's ACK before it can go upstream.
Comment 13 Ismail Donmez 2007-12-11 09:08:43 UTC
Kernel part  and Glibc part  is done, time to close this bug as fixed.  http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=52bfb36050c8529d9031d2c2513b281a360922ec  http://sourceware.org/cgi-bin/cvsweb.cgi/libc/sysdeps/unix/sysv/linux/adjtime.c.diff?cvsroot=glibc&r1=1.13&r2=1.14
Comment 14 Ingo Molnar 2007-12-11 13:26:17 UTC
great - i'm closing it. Thanks to everyone involved.