Messages in this thread | | | Date | Wed, 10 Feb 2021 11:51:46 +0100 | From | Greg Kroah-Hartman <> | Subject | Re: [RFC PATCH 2/2] drivers/virt: introduce CLOCK_BOOTTIME adjustment sysfs interface driver |
| |
On Wed, Feb 10, 2021 at 07:39:08PM +0900, Hikaru Nishida wrote: > From: Hikaru Nishida <hikalium@chromium.org> > > This adds a sysfs interface /sys/kernel/boottime_adj to enable advancing > CLOCK_BOOTTIME from the userspace without actual susupend/resume cycles. > > This gives a way to mitigate CLOCK_BOOTTIME divergence between guest > and host on virtualized environments after suspend/resume cycles on > the host. > > We observed an issue of a guest application that expects there is a gap > between CLOCK_BOOTTIME and CLOCK_MONOTONIC after the device is suspended > to detect whether the device went into suspend or not. > Since the guest is paused instead of being actually suspended during the > host's suspension, guest kernel doesn't advance CLOCK_BOOTTIME correctly > and there is no way to correct that. > > To solve the problem, this change introduces a way to modify a gap > between those clocks and align the timer behavior to host's one. > > Signed-off-by: Hikaru Nishida <hikalium@chromium.org> > --- > > drivers/virt/Kconfig | 9 ++++++ > drivers/virt/Makefile | 1 + > drivers/virt/boottime_adj.c | 57 +++++++++++++++++++++++++++++++++++++ > 3 files changed, 67 insertions(+) > create mode 100644 drivers/virt/boottime_adj.c > > diff --git a/drivers/virt/Kconfig b/drivers/virt/Kconfig > index 80c5f9c16ec1..149b4e763e4d 100644 > --- a/drivers/virt/Kconfig > +++ b/drivers/virt/Kconfig > @@ -13,6 +13,15 @@ menuconfig VIRT_DRIVERS > > if VIRT_DRIVERS > > +config BOOTTIME_ADJUSTMENT > + tristate "CLOCK_BOOTTIME adjustment sysfs interface" > + help > + The CLOCK_BOOTTIME adjustment sysfs interface driver > + provides a sysfs interface ( /sys/kernel/boottime_adj ) > + to enable adjusting CLOCK_BOOTTIME from the userspace. > + > + If unsure, say N. > + > config FSL_HV_MANAGER > tristate "Freescale hypervisor management driver" > depends on FSL_SOC > diff --git a/drivers/virt/Makefile b/drivers/virt/Makefile > index f28425ce4b39..1bbb476ddba9 100644 > --- a/drivers/virt/Makefile > +++ b/drivers/virt/Makefile > @@ -3,6 +3,7 @@ > # Makefile for drivers that support virtualization > # > > +obj-$(CONFIG_BOOTTIME_ADJUSTMENT) += boottime_adj.o > obj-$(CONFIG_FSL_HV_MANAGER) += fsl_hypervisor.o > obj-y += vboxguest/ > > diff --git a/drivers/virt/boottime_adj.c b/drivers/virt/boottime_adj.c > new file mode 100644 > index 000000000000..9cc717d8accc > --- /dev/null > +++ b/drivers/virt/boottime_adj.c > @@ -0,0 +1,57 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later
Do you really mean "or later"? I have to ask...
> +/* > + * CLOCK_BOOTTIME Adjustment Interface Driver
No copyright, nice! Your company lawyers will be having a word with you... :(
> + */ > + > +#include <linux/kobject.h> > +#include <linux/module.h> > +#include <linux/timekeeping.h> > + > +static struct kobject *kobj_boottime_adj; > + > +/* > + * Write to /sys/kernel/boottime_adj advances CLOCK_BOOTTIME by given delta. > + */ > +static ssize_t boottime_adj_write(struct kobject *kobj, > + struct kobj_attribute *attr, const char *buf, > + size_t count) > +{ > + int error; > + struct timespec64 delta; > + > + if (sscanf(buf, "%lld %ld", &delta.tv_sec, &delta.tv_nsec) != 2) > + return -EINVAL;
sysfs is "one value per file", but as you did not provide documentation on what this does, I can't tell if you really are following this or not.
> + > + error = timekeeping_adjust_boottime(&delta); > + if (error) > + return error; > + > + pr_info("%s: CLOCK_BOOTTIME has been advanced by %+lld seconds and %+ld nanoseconds\n", > + __func__, delta.tv_sec, delta.tv_nsec);
If kernels are working normally, no messages are printed to the log. Do not allow userspace to spam things.
> + return count; > +} > + > +static struct kobj_attribute boottime_adj_attr = > +__ATTR(boottime_adj, 0200, NULL, boottime_adj_write);
__ATTR_RO() please.
> + > +static int __init boottime_adj_init(void) > +{ > + int error; > + > + error = sysfs_create_file(kernel_kobj, &boottime_adj_attr.attr); > + if (error) { > + pr_warn("%s: failed to init\n", __func__);
why warn again?
> + return error; > + } > + return 0; > +} > + > +static void __exit boottime_adj_cleanup(void) > +{ > + kobject_put(kobj_boottime_adj);
You just called put on a kobject you never initialized?????
Are you sure this file has been tested?
odd...
greg k-h
| |