Messages in this thread | | | Date | Wed, 17 Sep 1997 18:20:46 +0200 (MEST) | From | Torsten Duwe <> | Subject | Re: Two bugs in kernel time code |
| |
Ulrich> Hmm (Not to say I'm getting tired): The message WILL NOT occur Ulrich> every 11 minutes, it will only occur if the clock could not be Ulrich> updated. In fact the message could even reappear after one minute Ulrich> (that's the way the code works), but it should go away after 30 Ulrich> times.
Ok, Ok. Anyway -- let's go for Philip's approach, see below.
>> zone the RTC is running on, so hours are untouched. No one from a >> half-hour or fifteen-minute time zone has complained yet :-|. A >> semi-clever approach would be to watch warp_time() from outside and >> add the kernel time zone conditionally.
Ulrich> If your CMOS clock is (let's say) at 15:55 and the new time is Ulrich> 16:05, the code will currently update your clock to 15:05, Ulrich> because it ignores the hour (and the dirfference is only 10 Ulrich> minutes).
Right. Furstrated about the unknown timezone problem I didn't think about this.
Ulrich> The correct fix to deal with the time zones is to store it in the Ulrich> kernel (at least if the CMOS runs localtime) and use it whenever Ulrich> setting or reading the clock. I think we should have a working Ulrich> routine in the kernel to set the CMOS clock; after all we have Ulrich> the RTC driver.
Agreed. There is a mechanism for this already (in case you haven't found it): check out warp_clock() in kernel/time.c. My tiny ktzset package (ftp://ftp.lst.de/pub/LST/misc/) seeks this routine explicitly without interfering with kernel<->RTC r/w precision. I wouldn't mind if there was a sysctl for this.
>> Please, work these suggestions into your patch, and maybe You can >> provide a diff -Bub next time if You re-indent things ? TIA
Ulrich> I tried not to re-indent things, but when rewriting code I Ulrich> dropped the 2-space indent. I hope you don't feel personally Ulrich> insulted. The style I used in the kernel isn't the style I Ulrich> usually write code, but IHMO 4 spaces is really the minimum for Ulrich> indentation. (No intention to start another flame-war). If you Ulrich> want to see real differences, save the original, apply the patch, Ulrich> and do a space-insensitice diff after that.
To avoid any misunderstanding let me get this straight: there are _very_ few things I care less about than indentation. It's just that I'm too lazy to unpack sources, apply a patch and the re-diff it; I like to see what's _really_ changed on first glance, especially in a mail. Sorry.
Back to Philip's suggestion. I feel it's good as long as some points are satisfied:
1. "minuteswest" suggests the proper granularity -- there exist 15-minute-timezones on this planet.
2. the sysctl must co-exist with the warp_clock()-hack for obvious backward compatibility (we have a few users out there...)
3. a reasonable representation for "unknown" must be found. -1 won't do, because it's obviously a valid "minuteswest" value.
I would suggest two variables: a numeric "minuteswest" value and an "rtc_timezone" enum for {utc=0, localtime=1, unknown=-1}.
Keep in mind that there is another reason for kernel timezone information: some sick so-called filesystems record mtime in localtime (FAT, HPFS, ...?).
Torsten
| |