On Sun, Feb 26, 2017 at 07:56:06PM +0100, Andreas Henriksson wrote:
> > But this doesn't work if the driver for the RTC is compiled as a module
> > or if the kernel was compiled with CONFIG_RTC_HCTOSYS disabled.
>
> Second (easy) case first, if CONFIG_RTC_HCTOSYS is disabled then
> I think we should not second guess this at all. period.
If all RTC drivers are built as modules, there's no point in enabling
CONFIG_RTC_HCTOSYS as it just bloats the kernel with code that's never
executed.
Hence disabling CONFIG_RTC_HCTOSYS cannot be interpreted as an unambiguous
policy decision that the system time should never be set from an RTC.
> For the module part, I'd consider this a problem on the kernel side
> primarily. If this problem won't/can't be fixed in the kernel itself
> then I'd rather see the entire kernel mechanism deprecated and
> a proper userspace component designed to handle it all.
As said, the kernel's rtc_hctosys() is a late_initcall, i.e. it is
executed prior to loading modules. Indeed it would be better if
the kernel would execute rtc_hctosys() once an RTC becomes available.
Regardless, user space should respond to this failure gracefully.
Currently if systemd is used, the system time is never set from the
RTC. Worse, the kernel periodically syncs the system time *back* to
the RTC. So even if the user sets both the system time and hwclock
manually, both will be reset to the epoch on the next reboot. It's
as if no RTC was present and that behaviour is almost never what the
user wants.
Googling for "systemd" "hwclock-set" gets 1230 results, the vast majority
are recipes to remove the systemd condition from hwclock-set, cargo-culted
over and over again for almost 6 years. This awfulness needs to stop.
> A better approach would be for the RPi solutions to ship (a package
> containing) a separate rules file and a script that (in best of
> worlds detects it's being run on the targeted hardware) and does
> what's needed. No hacking of util-linux packaged files needed
> and no unwanted overwrites.
This isn't limited to the Raspberry Pi, it's just the #1 example
because that's what most of the recipes you can find with Google
pertain to.
> Not a total fan of your patch. Consider the case where a system has
> more than one RTC. Consider specifically when CONFIG_RTC_HCTOSYS_DEVICE
> is set to rtc1. With your patch applied that would mean that we'd
> start running:
> /sbin/hwclock --rtc=rtc0 --hctosys
> (without first running: /sbin/hwclock --rtc=rtc0 --systz)
> right?
>
> That seems bad.
>
> (That the current udev rule only targets rtc0 is another thing I'd
> consider a bug. But that might be another discussion.)
Okay, let's define a policy first. How about:
(1) Users can set HWCLOCKACCESS=no in /etc/default/hwclock to prohibit
setting the system time or time zone from any RTC. (The parameter
is already defined in the config file, but it's not honored by
hwclock-set.)
(2) Users can set HCTOSYS_DEVICE in /etc/default/hwclock to constrain
setting the system time or time zone to a specific RTC. (Same as
above, parameter is already defined but not honored by hwclock-set.
This will also fix #785445.) If this parameter is not set,
hwclock-set will pick the first RTC that becomes available.
(3) If the kernel has already set the system time from *any* hwclock,
we don't set it once more in hwclock-set. (We only adjust the
timezone.)
(4) If systemd is present, we don't adjust the timezone once more in
hwclock-set.
Does this work for you? I've already coded the above up in my local repo
but haven't tested it yet, pending your response. (Tentative patch is
attached.)
Thanks,
Lukas
diff --git a/debian/hwclock-set b/debian/hwclock-set
index eacf948..516563c 100755
--- a/debian/hwclock-set
+++ b/debian/hwclock-set
@@ -4,10 +4,6 @@
dev=$1
-if [ -e /run/systemd/system ] ; then
- exit 0
-fi
-
if [ -e /run/udev/hwclock-set ]; then
exit 0
fi
@@ -20,17 +16,33 @@ fi
BADYEAR=no
HWCLOCKACCESS=yes
HWCLOCKPARS=
-HCTOSYS_DEVICE=rtc0
+HCTOSYS_DEVICE=
if [ -f /etc/default/hwclock ] ; then
. /etc/default/hwclock
fi
+if [ $HWCLOCKACCESS != yes -o \
+ ( -n $HCTOSYS_DEVICE -a $HCTOSYS_DEVICE != $2 ) ]
+ exit 0
+fi
+
+# If the kernel has already set the system time from the hwclock,
+# we only adjust the timezone
+cat /sys/class/rtc/*/hctosys 2>/dev/null | while read kernel_hctosys ; do
+ if [ $kernel_hctosys = 1 ] ; then
+ break
+ fi
+done
+
if [ yes = "$BADYEAR" ] ; then
- /sbin/hwclock --rtc=$dev --systz --badyear
- /sbin/hwclock --rtc=$dev --hctosys --badyear
-else
- /sbin/hwclock --rtc=$dev --systz
- /sbin/hwclock --rtc=$dev --hctosys
+ badyear="--badyear"
+fi
+
+if [ ! -e /run/systemd/system ] ; then
+ /sbin/hwclock --rtc=$dev --systz $badyear
+fi
+if [ $kernel_hctosys != 1 ] ; then
+ /sbin/hwclock --rtc=$dev --hctosys $badyear
fi
# Note 'touch' may not be available in initramfs
diff --git a/debian/hwclock.rules b/debian/hwclock.rules
index 972e4aa..8584deb 100644
--- a/debian/hwclock.rules
+++ b/debian/hwclock.rules
@@ -1,4 +1,4 @@
# Set the System Time from the Hardware Clock and set the kernel's timezone
# value to the local timezone when the kernel clock module is loaded.
-KERNEL=="rtc0", RUN+="/lib/udev/hwclock-set $root/$name"
+KERNEL=="rtc*", RUN+="/lib/udev/hwclock-set $root/$name $kernel"
