On Wed, Dec 02 2020 at 12:27, Jason Gunthorpe wrote:
> On Wed, Dec 02, 2020 at 02:44:53PM +0100, Thomas Gleixner wrote:
>> if (IS_ENABLED(CONFIG_GENERIC_CMOS_UPDATE) ||
>> IS_ENABLED(CONFIG_RTC_SYSTOHC))
>> - queue_delayed_work(system_power_efficient_wq, &sync_work, 0);
>> + queue_work(system_power_efficient_wq, &sync_work);
>
> As Miroslav noted, probably the right thing to do here is to reset the
> hrtimer and remove the sync_work? I think this code was to expedite an
> RTC sync when NTP fixes the clock on boot.
This has two purposes:
1) Initiating the update on boot once ntp is synced.
2) Reinitiating the sync after ntp lost sync and the work did not
reschedule itself because it observed !ntp_synced().
In both cases it's highly unlikely that the write actually happens when
the work is queued because do_adjtimex() would have to be exactly around
the valid update window. So it will not write immediately. It will run
through at least one retry.
I don't think the timer should be canceled if the ntp_synced() state did
not change. Otherwise every do_adtimex() call will cancel/restart
it, which does not make sense. Lemme stare at it some more.
> IIRC this was made somewhat messy due to the dual path with rtclib and
> old cmos.
:)
> It would be very nice to kill the cmos path, things are better
> now.. But PPC still has a long way to go:
>
> arch/powerpc/platforms/52xx/efika.c: .set_rtc_time =
> rtas_set_rtc_time,
> arch/powerpc/platforms/8xx/mpc86xads_setup.c: .set_rtc_time =
> mpc8xx_set_rtc_time,
> arch/powerpc/platforms/8xx/tqm8xx_setup.c: .set_rtc_time =
> mpc8xx_set_rtc_time,
> arch/powerpc/platforms/cell/setup.c: .set_rtc_time =
> rtas_set_rtc_time,
> arch/powerpc/platforms/chrp/setup.c: ppc_md.set_rtc_time =
> rtas_set_rtc_time;
> arch/powerpc/platforms/chrp/setup.c: .set_rtc_time =
> chrp_set_rtc_time,
> arch/powerpc/platforms/maple/setup.c: .set_rtc_time =
> maple_set_rtc_time,
> arch/powerpc/platforms/powermac/setup.c: .set_rtc_time =
> pmac_set_rtc_time,
> arch/powerpc/platforms/pseries/setup.c: .set_rtc_time =
> rtas_set_rtc_time,
>
> Also x86 needs a touch, it already has RTC lib, no idea why it also
> provides this old path too
Because nobody had the stomach and/or cycles to touch it :)
> I wonder if the cmos path could be killed off under the dead HW
> principle?
Unfortunately that code path is not that dead on x86. You need to fix
all the (ab)users first. :)
Thanks,
tglx
