linux-kernel - Re: [rtc-linux] [RFC][PATCH 1/4] RTC: Class device support for persistent clock

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: <1210148655.25560.825.camel@pmac.infradead.org>
Date:	Wed, 07 May 2008 09:24:15 +0100
From:	David Woodhouse <dwmw2@...radead.org>
To:	rtc-linux@...glegroups.com
Cc:	Alessandro Zummo <a.zummo@...ertech.it>,
	Jean Delvare <khali@...ux-fr.org>,
	Ralf Baechle <ralf@...ux-mips.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Andrew Morton <akpm@...ux-foundation.org>, i2c@...sensors.org,
	linux-mips@...ux-mips.org, linux-kernel@...r.kernel.org
Subject: Re: [rtc-linux] [RFC][PATCH 1/4] RTC: Class device support for
	persistent clock

On Wed, 2008-05-07 at 01:40 +0100, Maciej W. Rozycki wrote:
> 
> +int rtc_update_persistent_clock(struct timespec now)
> +{
> +       struct rtc_device *rtc =
> rtc_class_open(CONFIG_RTC_HCTOSYS_DEVICE);
> +       int err;
> +
> +       if (rtc == NULL) {
> +               printk(KERN_ERR "hctosys: unable to open rtc device (%
> s)\n",
> +                      CONFIG_RTC_HCTOSYS_DEVICE);
> +               err = -ENXIO;
> +               goto out;
>         }
> -       else
> +       err = rtc_set_mmss(rtc, now.tv_sec);
> +       if (err < 0) {
>                 dev_err(rtc->dev.parent,
> -                       "hctosys: unable to read the hardware clock
> \n");
> +                       "hctosys: unable to set the hardware clock
> \n");
> +               goto out_close;
> +       }
>  
> +       err = 0;
> +
> +out_close:
>         rtc_class_close(rtc);
> +out:
> +       return err;
> +}

Ooh, shiny -- you saved me the trouble of doing this (and hopefully also
the trouble of looking through it to check whether all the callers of
read_persistent_clock() can sleep, etc.?)

One thing I was going to do in rtc_update_persistent_clock() was make it
use mutex_trylock() for grabbing rtc->lock. We go to great lengths to
make sure we're updating the clock at the correct time -- we don't want
to be doing things which delay the update. So we should probably just
use mutex_trylock() and abort the update (this time) if it fails.

I was also thinking of holding the RTC_HCTOSYS device open all the time,
too. If it's a problem that you then couldn't unload the module, perhaps
a sysfs interface to set/change/clear which device is used for this?

When we discussed it last week, Alessandro was concerned that the
'update at precisely 500ms past the second' rule was not universal to
all RTC devices, although I'm not entirely sure. It might be worth
moving that logic into a 'default' NTP-sync routine provided by the RTC
class, so that if any strange devices exist which require different
treatment, they can override that.

I wouldn't worry too much about leaving the old
update_persistent_clock() and read_persistent_clock() -- I hope we can
plan to remove those entirely in favour of the RTC class methods.

-- 
dwmw2

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