Re: CONFIG_RTC_HCTOSYS lost on x86 with ALWAYS_USE_PERSISTENT_CLOCKchanges?

[John Stultz]

On 04/23/2013 08:33 PM, Kay Sievers wrote:
> On Wed, Apr 24, 2013 at 5:19 AM, John Stultz <john.stultz@xxxxxxxxxx> wrote:
> > On 04/23/2013 08:05 PM, Kay Sievers wrote:
> > > On Wed, Apr 24, 2013 at 4:43 AM, John Stultz <john.stultz@xxxxxxxxxx>
> > > wrote:
> > > > On 04/23/2013 06:34 PM, Kay Sievers wrote:
> > > > > Hey,
> > > > >
> > > > > what's the intention of:
> > > > >      e90c83f757fffdacec8b3c5eee5617dcc038338f ?
> > > > >      x86: Select HAS_PERSISTENT_CLOCK on x86
> > > > >
> > > > > It unconditionally sets:
> > > > >      HAS_PERSISTENT_CLOCK
> > > > > but:
> > > > >      RTC_SYSTOHC
> > > > > got a depends on !HAS_PERSISTENT_CLOCK
> > > > >
> > > > > This makes it impossible to sync the system time from the RTC on x86.
> > > > > What's going on here?
> > > >
> > > > So I suspect just some confusion, but let me know if thats wrong and
> > > > you're
> > > > actually seeing an issue.
> > > >
> > > > SYSTOHC is what *sets the RTC* to the system time when we're synced with
> > > > NTP.
> > > >
> > > > HCTOSYS is what sets the system time from the RTC.
> > > Right, and RTC_HCTOSYS is not NTP related. It just reads the time from
> > > the RTC_HCTOSYS_DEVICE at bootup so we do not boot in 1970 time mode.
> > > We need that it in all cases, at every bootup on x86. But it's no
> > > longer there with the above commits. :)
> > On x86 the persistent_clock() is backed by the CMOS/EFI/kvm-wall/xen/vrtc
> > clock (all via x86_platform.get_wallclock) should be present and we'll
> > initialize the time in timekeeping_init() there.
> >
> > Its only systems where there isn't a persistent_clock is where the RTC layer
> > and the HCTOSYS is helpful.
> >
> > Again, if you're having a problem where an x86 system isn't getting its time
> > initialized correctly, please let me know the details of the system.
> Until the above commits we always needed:
>    CONFIG_RTC_HCTOSYS=y
>    CONFIG_RTC_HCTOSYS_DEVICE="rtc0"
> to get the system time correctly initialized at bootup on x86.

So... always needed to get system time correctly initialized? I'm not 
sure I agree with this. On non-x86 (mostly embedded) platforms that do 
not have persistent_clock support, yes, the above is needed.

But I'm unaware of the above actually being necessary on x86, as its 
always had persistent_clock support.


> These options are gone now and cannot be selected anymore. You are
> saying that this is all fine, that they are gone, but all initial
> clock syncing should still work?

Yes, we're just removing a duplicative initialization of time, and 
compiling out code in the suspend/resume path that would never trigger 
when persistent_clocks were present.



> Also:
>    $ cat /sys/class/rtc/rtc0/hctosys
>    0
> always carried "1", and this now breaks setups which expect an
> automatically created symlink /dev/rtc to the actual "system rtc".

Sigh. So just turning off HCTOSYS on those systems causes them to break?

That is sort of obnoxiously fragile.  I've always been somewhat 
skeptical of the multi-rtc configs - as they're all the "system's" RTCs 
after all. 99% probably only have one rtc device, so checking the 
hctosys in that case is a little silly.

But the terribly annoying interface breakage when /dev/rtc went to 
/dev/rtcN with the generic rtc layer landing shouldn't have happened, so 
I won't begrudge too much the userland hacks needed to fix that up.

So I'll send Thomas a revert for the HCTOSYS optimization. In the kernel 
we'll still avoid using HCTOSYS when the persistent_clock is there, but 
at least userland will still have some /sys/class/rtc/rtcN that has the 
"offical" flag.


> There was also always a line in dmesg that told the rtc_cmos time it
> wrote to the system clock. This is also gone?

More worrisome, I'll turn the question around: is that an expected 
interface never to break?


> I haven't looked what goes wrong, I expected the make(1) errors with
> "time in the future" after bootup before the network is up, which I
> see since a week or two, to be a fallout of that.

I'd be very interested if you can narrow this suspicion down, as it 
would mean there's likely a problem with the persistent_clock code that 
needs fixing.


Thanks again for raising the flag on the userland expectations bit.
-john

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/