lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20201202162723.GJ5487@ziepe.ca>
Date:   Wed, 2 Dec 2020 12:27:23 -0400
From:   Jason Gunthorpe <jgg@...pe.ca>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     Miroslav Lichvar <mlichvar@...hat.com>,
        linux-kernel@...r.kernel.org, John Stultz <john.stultz@...aro.org>,
        Prarit Bhargava <prarit@...hat.com>
Subject: Re: [PATCH] rtc: adapt allowed RTC update error

On Wed, Dec 02, 2020 at 02:44:53PM +0100, Thomas Gleixner wrote:

> So if this ends up in level #1 then again the chance is pretty low that
> the expiry time is aligned to the the level period. If this works then
> it only works by chance.

Yes, I always thought it was timer wheel related, but at least in my
testing long ago it seemed reliable at the short timeout. Maybe I had
a lucky hz value.
 
> > This seems like a more fundamental problem someplace else in the
> > kernel.
> 
> I don't think so. :)
> 
> Something like the completely untested below should make this reliable
> and only needs to retry when the work is running late (busy machine),
> but the wakeup will be on time or at max 1 jiffie off when high
> resolution timers are not available or disabled.

This seems like the right approach

> @@ -629,7 +618,7 @@ void ntp_notify_cmos_timer(void)
>  
>  	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.

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

I wonder if the cmos path could be killed off under the dead HW
principle?

Jason

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ