2010-08-04 12:51:30

by Alexander Shishkin

[permalink] [raw]
Subject: [PATCH] [RFC] notify userspace about time changes

Certain userspace applications (like "clock" desktop applets or ntpd) might
want to be notified when some other application changes the system time. It
might also be important for an application to be able to distinguish between
its own and somebody else's time changes.

This patch implements a notification interface via eventfd mechanism. Proccess
wishing to be notified about time changes should create an eventfd and echo
its file descriptor to /sys/kernel/time_notify. After that, any calls to
settimeofday()/stime()/adjtimex() made by other processes will be signalled
to this eventfd. Credits for suggesting the eventfd mechanism for this
purpose go te Kirill Shutemov.

So far, this implementation can only filter out notifications caused by
time change calls made by the process that wrote the eventfd descriptor to
sysfs, but not its children which (might) have inherited the eventfd. It
is so far not clear to me whether this is bad and more confusing than
excluding such children as well.

Similar mechanism can also be used for signalling other (all?) system calls
made by certain (all?) processes without resorting to ptrace (which won't
help if you don't know what processes you'd like to look after), given
proper permission checks etc.

Signed-off-by: Alexander Shishkin <[email protected]>
CC: Kirill A. Shutemov <[email protected]>
CC: Thomas Gleixner <[email protected]>
CC: John Stultz <[email protected]>
CC: Martin Schwidefsky <[email protected]>
CC: Andrew Morton <[email protected]>
CC: Jon Hunter <[email protected]>
CC: Ingo Molnar <[email protected]>
CC: Peter Zijlstra <[email protected]>
CC: "Paul E. McKenney" <[email protected]>
CC: David Howells <[email protected]>
CC: Avi Kivity <[email protected]>
CC: "H. Peter Anvin" <[email protected]>
CC: John Kacur <[email protected]>
CC: Alexander Shishkin <[email protected]>
CC: [email protected]
---
include/linux/time.h | 7 ++
init/Kconfig | 7 ++
kernel/Makefile | 1 +
kernel/time.c | 11 +++-
kernel/time_notify.c | 158 ++++++++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 182 insertions(+), 2 deletions(-)
create mode 100644 kernel/time_notify.c

diff --git a/include/linux/time.h b/include/linux/time.h
index ea3559f..9fca62b 100644
--- a/include/linux/time.h
+++ b/include/linux/time.h
@@ -237,6 +237,13 @@ static __always_inline void timespec_add_ns(struct timespec *a, u64 ns)
a->tv_sec += __iter_div_u64_rem(a->tv_nsec + ns, NSEC_PER_SEC, &ns);
a->tv_nsec = ns;
}
+
+#ifdef CONFIG_TIME_NOTIFY
+void time_notify_all(void);
+#else
+#define time_notify_all() do {} while (0)
+#endif
+
#endif /* __KERNEL__ */

#define NFDBITS __NFDBITS
diff --git a/init/Kconfig b/init/Kconfig
index 5cff9a9..f7271f8 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -976,6 +976,13 @@ config PERF_USE_VMALLOC
help
See tools/perf/design.txt for details

+config TIME_NOTIFY
+ bool
+ depends on EVENTFD
+ help
+ Enable time change notification events to userspace via
+ eventfd.
+
menu "Kernel Performance Events And Counters"

config PERF_EVENTS
diff --git a/kernel/Makefile b/kernel/Makefile
index 057472f..7a25ee4 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -105,6 +105,7 @@ obj-$(CONFIG_PERF_EVENTS) += perf_event.o
obj-$(CONFIG_HAVE_HW_BREAKPOINT) += hw_breakpoint.o
obj-$(CONFIG_USER_RETURN_NOTIFIER) += user-return-notifier.o
obj-$(CONFIG_PADATA) += padata.o
+obj-$(CONFIG_TIME_NOTIFY) += time_notify.o

ifneq ($(CONFIG_SCHED_OMIT_FRAME_POINTER),y)
# According to Alan Modra <[email protected]>, the -fno-omit-frame-pointer is
diff --git a/kernel/time.c b/kernel/time.c
index 848b1c2..74d355e 100644
--- a/kernel/time.c
+++ b/kernel/time.c
@@ -92,7 +92,9 @@ SYSCALL_DEFINE1(stime, time_t __user *, tptr)
if (err)
return err;

- do_settimeofday(&tv);
+ err = do_settimeofday(&tv);
+ if (!err)
+ time_notify_all();
return 0;
}

@@ -177,7 +179,10 @@ int do_sys_settimeofday(struct timespec *tv, struct timezone *tz)
/* SMP safe, again the code in arch/foo/time.c should
* globally block out interrupts when it runs.
*/
- return do_settimeofday(tv);
+ error = do_settimeofday(tv);
+ if (!error)
+ time_notify_all();
+ return error;
}
return 0;
}
@@ -215,6 +220,8 @@ SYSCALL_DEFINE1(adjtimex, struct timex __user *, txc_p)
if(copy_from_user(&txc, txc_p, sizeof(struct timex)))
return -EFAULT;
ret = do_adjtimex(&txc);
+ if (!ret)
+ time_notify_all();
return copy_to_user(txc_p, &txc, sizeof(struct timex)) ? -EFAULT : ret;
}

diff --git a/kernel/time_notify.c b/kernel/time_notify.c
new file mode 100644
index 0000000..c674f25
--- /dev/null
+++ b/kernel/time_notify.c
@@ -0,0 +1,158 @@
+/*
+ * linux/kernel/time_notify.c
+ *
+ * Copyright (C) 2010 Nokia Corporation
+ * Alexander Shishkin
+ *
+ * This file implements an interface to communicate time changes to userspace.
+ */
+
+#include <linux/kernel.h>
+#include <linux/types.h>
+#include <linux/eventfd.h>
+#include <linux/kobject.h>
+#include <linux/wait.h>
+#include <linux/workqueue.h>
+#include <linux/sched.h>
+#include <linux/poll.h>
+#include <linux/err.h>
+
+/*
+ * A process can "subscribe" to receive a notification via eventfd that
+ * some other process has called stime/settimeofday/adjtimex.
+ */
+struct time_event {
+ struct eventfd_ctx *eventfd;
+ struct task_struct *watcher;
+ struct work_struct remove;
+ wait_queue_t wq;
+ wait_queue_head_t *wqh;
+ poll_table pt;
+ struct list_head list;
+};
+
+static LIST_HEAD(event_list);
+static DEFINE_SPINLOCK(event_lock);
+
+/*
+ * Do the necessary cleanup when the eventfd is being closed
+ */
+static void time_event_remove(struct work_struct *work)
+{
+ struct time_event *evt = container_of(work, struct time_event, remove);
+
+ kfree(evt);
+}
+
+static int time_event_wakeup(wait_queue_t *wq, unsigned int mode, int sync,
+ void *key)
+{
+ struct time_event *evt = container_of(wq, struct time_event, wq);
+ unsigned long flags = (unsigned long)key;
+
+ if (flags & POLLHUP) {
+ remove_wait_queue_locked(evt->wqh, &evt->wq);
+ spin_lock(&event_lock);
+ list_del(&evt->list);
+ spin_unlock(&event_lock);
+
+ schedule_work(&evt->remove);
+ }
+
+ return 0;
+}
+
+static void time_event_ptable_queue_proc(struct file *file,
+ wait_queue_head_t *wqh, poll_table *pt)
+{
+ struct time_event *evt = container_of(pt, struct time_event, pt);
+
+ evt->wqh = wqh;
+ add_wait_queue(wqh, &evt->wq);
+}
+
+/*
+ * Process wishing to be notified about time changes should write its
+ * eventfd's descriptor to /sys/kernel/time_notify. This eventfd will
+ * then be signalled about any time changes made by any process *but*
+ * this one.
+ */
+static int time_notify_store(struct kobject *kobj, struct kobj_attribute *attr,
+ const char *buf, size_t n)
+{
+ int ret = n;
+ unsigned int fd;
+ struct file *file;
+ struct time_event *evt;
+
+ evt = kmalloc(sizeof(*evt), GFP_KERNEL);
+ if (!evt)
+ return -ENOMEM;
+
+ fd = strict_strtoul(buf, NULL, 10);
+ file = eventfd_fget(fd);
+ if (IS_ERR(file)) {
+ ret = -EINVAL;
+ goto out_free;
+ }
+
+ evt->eventfd = eventfd_ctx_fileget(file);
+ if (!evt->eventfd) {
+ ret = PTR_ERR(evt->eventfd);
+ goto out_fput;
+ }
+
+ INIT_LIST_HEAD(&evt->list);
+ INIT_WORK(&evt->remove, time_event_remove);
+
+ init_waitqueue_func_entry(&evt->wq, time_event_wakeup);
+ init_poll_funcptr(&evt->pt, time_event_ptable_queue_proc);
+
+ if (file->f_op->poll(file, &evt->pt) & POLLHUP) {
+ ret = 0;
+ goto out_fput;
+ }
+
+ evt->watcher = current;
+
+ spin_lock(&event_lock);
+ list_add(&event_list, &evt->list);
+ spin_unlock(&event_lock);
+
+ fput(file);
+
+ return ret;
+
+out_fput:
+ fput(file);
+
+out_free:
+ kfree(evt);
+
+ return ret;
+}
+
+void time_notify_all(void)
+{
+ struct list_head *tmp;
+
+ spin_lock(&event_lock);
+ list_for_each(tmp, &event_list) {
+ struct time_event *e = container_of(tmp, struct time_event,
+ list);
+
+ if (e->watcher != current)
+ eventfd_signal(e->eventfd, 1);
+ }
+ spin_unlock(&event_lock);
+}
+
+static struct kobj_attribute time_notify_attr =
+ __ATTR(time_notify, S_IWUGO, NULL, time_notify_store);
+
+static int time_notify_init(void)
+{
+ return sysfs_create_file(kernel_kobj, &time_notify_attr.attr);
+}
+
+core_initcall(time_notify_init);
--
1.7.1


2010-08-04 12:54:24

by Bastien Roucariès

[permalink] [raw]
Subject: Re: [PATCH] [RFC] notify userspace about time changes

On Wed, Aug 4, 2010 at 2:48 PM, Alexander Shishkin <[email protected]> wrote:
> Certain userspace applications (like "clock" desktop applets or ntpd) might
> want to be notified when some other application changes the system time. It
> might also be important for an application to be able to distinguish between
> its own and somebody else's time changes.

Why not implementing somtehing like http://lwn.net/Articles/323658/ a
la plan 9 with a poll callback ?

It will be plan9 compatible and so unix

Bastien

2010-08-04 15:32:25

by Kirill A. Shutemov

[permalink] [raw]
Subject: Re: [PATCH] [RFC] notify userspace about time changes

On Wed, Aug 04, 2010 at 03:48:28PM +0300, Alexander Shishkin wrote:
> Certain userspace applications (like "clock" desktop applets or ntpd) might
> want to be notified when some other application changes the system time. It
> might also be important for an application to be able to distinguish between
> its own and somebody else's time changes.
>
> This patch implements a notification interface via eventfd mechanism. Proccess
> wishing to be notified about time changes should create an eventfd and echo
> its file descriptor to /sys/kernel/time_notify. After that, any calls to
> settimeofday()/stime()/adjtimex() made by other processes will be signalled
> to this eventfd. Credits for suggesting the eventfd mechanism for this
> purpose go te Kirill Shutemov.
>
> So far, this implementation can only filter out notifications caused by
> time change calls made by the process that wrote the eventfd descriptor to
> sysfs, but not its children which (might) have inherited the eventfd. It
> is so far not clear to me whether this is bad and more confusing than
> excluding such children as well.

I think it's a bad idea to filter notifications. Let's leave it for
userspace. Userspace always can check eventfd counter and understand who
touch time based on its own activity.

> Similar mechanism can also be used for signalling other (all?) system calls
> made by certain (all?) processes without resorting to ptrace (which won't
> help if you don't know what processes you'd like to look after), given
> proper permission checks etc.
>
> Signed-off-by: Alexander Shishkin <[email protected]>
> CC: Kirill A. Shutemov <[email protected]>
> CC: Thomas Gleixner <[email protected]>
> CC: John Stultz <[email protected]>
> CC: Martin Schwidefsky <[email protected]>
> CC: Andrew Morton <[email protected]>
> CC: Jon Hunter <[email protected]>
> CC: Ingo Molnar <[email protected]>
> CC: Peter Zijlstra <[email protected]>
> CC: "Paul E. McKenney" <[email protected]>
> CC: David Howells <[email protected]>
> CC: Avi Kivity <[email protected]>
> CC: "H. Peter Anvin" <[email protected]>
> CC: John Kacur <[email protected]>
> CC: Alexander Shishkin <[email protected]>
> CC: [email protected]
> ---
> include/linux/time.h | 7 ++
> init/Kconfig | 7 ++
> kernel/Makefile | 1 +
> kernel/time.c | 11 +++-
> kernel/time_notify.c | 158 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 182 insertions(+), 2 deletions(-)
> create mode 100644 kernel/time_notify.c
>
> diff --git a/include/linux/time.h b/include/linux/time.h
> index ea3559f..9fca62b 100644
> --- a/include/linux/time.h
> +++ b/include/linux/time.h
> @@ -237,6 +237,13 @@ static __always_inline void timespec_add_ns(struct timespec *a, u64 ns)
> a->tv_sec += __iter_div_u64_rem(a->tv_nsec + ns, NSEC_PER_SEC, &ns);
> a->tv_nsec = ns;
> }
> +
> +#ifdef CONFIG_TIME_NOTIFY
> +void time_notify_all(void);
> +#else
> +#define time_notify_all() do {} while (0)
> +#endif
> +
> #endif /* __KERNEL__ */
>
> #define NFDBITS __NFDBITS
> diff --git a/init/Kconfig b/init/Kconfig
> index 5cff9a9..f7271f8 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -976,6 +976,13 @@ config PERF_USE_VMALLOC
> help
> See tools/perf/design.txt for details
>
> +config TIME_NOTIFY
> + bool
> + depends on EVENTFD
> + help
> + Enable time change notification events to userspace via
> + eventfd.
> +

Do we really need config option? I think better just use
CONFIG_EVENTFD.

[email protected] added to CC list.

--
Kirill A. Shutemov

2010-08-04 15:48:06

by Alexander Shishkin

[permalink] [raw]
Subject: Re: [PATCH] [RFC] notify userspace about time changes

On Wed, Aug 04, 2010 at 06:32:21 +0300, Kirill A. Shutemov wrote:
> On Wed, Aug 04, 2010 at 03:48:28PM +0300, Alexander Shishkin wrote:
> > Certain userspace applications (like "clock" desktop applets or ntpd) might
> > want to be notified when some other application changes the system time. It
> > might also be important for an application to be able to distinguish between
> > its own and somebody else's time changes.
> >
> > This patch implements a notification interface via eventfd mechanism. Proccess
> > wishing to be notified about time changes should create an eventfd and echo
> > its file descriptor to /sys/kernel/time_notify. After that, any calls to
> > settimeofday()/stime()/adjtimex() made by other processes will be signalled
> > to this eventfd. Credits for suggesting the eventfd mechanism for this
> > purpose go te Kirill Shutemov.
> >
> > So far, this implementation can only filter out notifications caused by
> > time change calls made by the process that wrote the eventfd descriptor to
> > sysfs, but not its children which (might) have inherited the eventfd. It
> > is so far not clear to me whether this is bad and more confusing than
> > excluding such children as well.
>
> I think it's a bad idea to filter notifications. Let's leave it for

Why?

> userspace. Userspace always can check eventfd counter and understand who
> touch time based on its own activity.

That's another way to approach this, yes. But I can think of scenarios when
this will complicate userspace implementation. This generally means that the
"time monitor" application will have to increment some internal counter every
time it calls settimeofday() and friends. And when some library that it links
against starts calling one of those functions behind the scenes, it will
become trickier.

It is also true that you can never be too good to userspace.

> > Similar mechanism can also be used for signalling other (all?) system calls
> > made by certain (all?) processes without resorting to ptrace (which won't
> > help if you don't know what processes you'd like to look after), given
> > proper permission checks etc.
> >
> > Signed-off-by: Alexander Shishkin <[email protected]>
> > CC: Kirill A. Shutemov <[email protected]>
> > CC: Thomas Gleixner <[email protected]>
> > CC: John Stultz <[email protected]>
> > CC: Martin Schwidefsky <[email protected]>
> > CC: Andrew Morton <[email protected]>
> > CC: Jon Hunter <[email protected]>
> > CC: Ingo Molnar <[email protected]>
> > CC: Peter Zijlstra <[email protected]>
> > CC: "Paul E. McKenney" <[email protected]>
> > CC: David Howells <[email protected]>
> > CC: Avi Kivity <[email protected]>
> > CC: "H. Peter Anvin" <[email protected]>
> > CC: John Kacur <[email protected]>
> > CC: Alexander Shishkin <[email protected]>
> > CC: [email protected]
> > ---
> > include/linux/time.h | 7 ++
> > init/Kconfig | 7 ++
> > kernel/Makefile | 1 +
> > kernel/time.c | 11 +++-
> > kernel/time_notify.c | 158 ++++++++++++++++++++++++++++++++++++++++++++++++++
> > 5 files changed, 182 insertions(+), 2 deletions(-)
> > create mode 100644 kernel/time_notify.c
> >
> > diff --git a/include/linux/time.h b/include/linux/time.h
> > index ea3559f..9fca62b 100644
> > --- a/include/linux/time.h
> > +++ b/include/linux/time.h
> > @@ -237,6 +237,13 @@ static __always_inline void timespec_add_ns(struct timespec *a, u64 ns)
> > a->tv_sec += __iter_div_u64_rem(a->tv_nsec + ns, NSEC_PER_SEC, &ns);
> > a->tv_nsec = ns;
> > }
> > +
> > +#ifdef CONFIG_TIME_NOTIFY
> > +void time_notify_all(void);
> > +#else
> > +#define time_notify_all() do {} while (0)
> > +#endif
> > +
> > #endif /* __KERNEL__ */
> >
> > #define NFDBITS __NFDBITS
> > diff --git a/init/Kconfig b/init/Kconfig
> > index 5cff9a9..f7271f8 100644
> > --- a/init/Kconfig
> > +++ b/init/Kconfig
> > @@ -976,6 +976,13 @@ config PERF_USE_VMALLOC
> > help
> > See tools/perf/design.txt for details
> >
> > +config TIME_NOTIFY
> > + bool
> > + depends on EVENTFD
> > + help
> > + Enable time change notification events to userspace via
> > + eventfd.
> > +
>
> Do we really need config option? I think better just use
> CONFIG_EVENTFD.

This is quite obscure. If anything, it's better to get rid of CONFIG_EVENTFD
and always unconditionally compile it in, while this functionality (time_notify)
may well be optional.

Regards,
--
Alex

2010-08-04 15:51:14

by Alexander Shishkin

[permalink] [raw]
Subject: Re: [PATCH] [RFC] notify userspace about time changes

On Wed, Aug 04, 2010 at 02:54:12 +0200, Bastien ROUCARIES wrote:
> On Wed, Aug 4, 2010 at 2:48 PM, Alexander Shishkin <[email protected]> wrote:
> > Certain userspace applications (like "clock" desktop applets or ntpd) might
> > want to be notified when some other application changes the system time. It
> > might also be important for an application to be able to distinguish between
> > its own and somebody else's time changes.
>
> Why not implementing somtehing like http://lwn.net/Articles/323658/ a
> la plan 9 with a poll callback ?

Thanks for pointing this out, I'll investigate this approach.

> It will be plan9 compatible and so unix

I don't think that plan9 == unix statement is broadly correct, but I don't
know enough on the topic yet.

Regards,
--
Alex

2010-08-04 15:59:12

by john stultz

[permalink] [raw]
Subject: Re: [PATCH] [RFC] notify userspace about time changes

On Wed, 2010-08-04 at 15:48 +0300, Alexander Shishkin wrote:
> Certain userspace applications (like "clock" desktop applets or ntpd) might
> want to be notified when some other application changes the system time. It
> might also be important for an application to be able to distinguish between
> its own and somebody else's time changes.

So NTP doesn't actually care, as it will notice the STA_UNSYNC flag is
set the next time it checks adjtimex.

The clock apps example seems reasonable, but maybe isn't the most
compelling argument for adding a new kernel api.

Is there a actual use case that you need this for? I don't really have
an issue with the code I just really want to make sure the feature would
be useful enough to justify the API and code maintenance going forward.

Doing some of my own googling, I see Apple added such a notification
method, which suggests there are apps that want this.
http://stackoverflow.com/questions/690326/how-can-i-get-notified-of-a-system-time-change-in-my-cocoa-application

But it doesn't give any specific examples either.

thanks
-john

2010-08-04 18:49:32

by Chris Friesen

[permalink] [raw]
Subject: Re: [PATCH] [RFC] notify userspace about time changes

On 08/04/2010 09:58 AM, john stultz wrote:

> Is there a actual use case that you need this for? I don't really have
> an issue with the code I just really want to make sure the feature would
> be useful enough to justify the API and code maintenance going forward.

We actually added a time-change-notification mechanism internally a long
time ago but never saw demand for it and so never bothered trying to
push it upstream. Ours is signal-based.

Among other things we use it to pass on time-change notifications to an
emulator running a proprietary OS that really cares about having an
accurate time-of-day but can't afford a syscall to retrieve it every time.

Chris

--
Chris Friesen
Software Developer
GENBAND
[email protected]
http://www.genband.com

2010-08-05 00:52:52

by john stultz

[permalink] [raw]
Subject: Re: [PATCH] [RFC] notify userspace about time changes

On Wed, 2010-08-04 at 12:48 -0600, Chris Friesen wrote:
> On 08/04/2010 09:58 AM, john stultz wrote:
>
> > Is there a actual use case that you need this for? I don't really have
> > an issue with the code I just really want to make sure the feature would
> > be useful enough to justify the API and code maintenance going forward.
>
> We actually added a time-change-notification mechanism internally a long
> time ago but never saw demand for it and so never bothered trying to
> push it upstream. Ours is signal-based.
>
> Among other things we use it to pass on time-change notifications to an
> emulator running a proprietary OS that really cares about having an
> accurate time-of-day but can't afford a syscall to retrieve it every time.

So the eventfd based method (and the filtering) proposed would work for
you?

thanks
-john

2010-08-05 10:05:39

by Kirill A. Shutemov

[permalink] [raw]
Subject: Re: [PATCH] [RFC] notify userspace about time changes

On Wed, Aug 04, 2010 at 06:46:38PM +0300, Alexander Shishkin wrote:
> On Wed, Aug 04, 2010 at 06:32:21 +0300, Kirill A. Shutemov wrote:
> > On Wed, Aug 04, 2010 at 03:48:28PM +0300, Alexander Shishkin wrote:
> > > Certain userspace applications (like "clock" desktop applets or ntpd) might
> > > want to be notified when some other application changes the system time. It
> > > might also be important for an application to be able to distinguish between
> > > its own and somebody else's time changes.
> > >
> > > This patch implements a notification interface via eventfd mechanism. Proccess
> > > wishing to be notified about time changes should create an eventfd and echo
> > > its file descriptor to /sys/kernel/time_notify. After that, any calls to
> > > settimeofday()/stime()/adjtimex() made by other processes will be signalled
> > > to this eventfd. Credits for suggesting the eventfd mechanism for this
> > > purpose go te Kirill Shutemov.
> > >
> > > So far, this implementation can only filter out notifications caused by
> > > time change calls made by the process that wrote the eventfd descriptor to
> > > sysfs, but not its children which (might) have inherited the eventfd. It
> > > is so far not clear to me whether this is bad and more confusing than
> > > excluding such children as well.
> >
> > I think it's a bad idea to filter notifications. Let's leave it for
>
> Why?

Someone might want to recieve notifications about its own activity.
It's a policy. Kernel should provide mechanism, not policy.

--
Kirill A. Shutemov

2010-08-05 12:33:18

by Alexander Shishkin

[permalink] [raw]
Subject: Re: [PATCH] [RFC] notify userspace about time changes

On 4 August 2010 18:58, john stultz <[email protected]> wrote:
> On Wed, 2010-08-04 at 15:48 +0300, Alexander Shishkin wrote:
>> Certain userspace applications (like "clock" desktop applets or ntpd) might
>> want to be notified when some other application changes the system time. It
>> might also be important for an application to be able to distinguish between
>> its own and somebody else's time changes.
>
> So NTP doesn't actually care, as it will notice the STA_UNSYNC flag is
> set the next time it checks adjtimex.
>
> The clock apps example seems reasonable, but maybe isn't the most
> compelling argument for adding a new kernel api.
>
> Is there a actual use case that you need this for?  I don't really have
> an issue with the code I just really want to make sure the feature would
> be useful enough to justify the API and code maintenance going forward.

Yes. What we have here is an application which takes care of different means
of time synchronization (trusted time servers, different GSM operators, etc)
and also different kinds of time-based events/notifications (like "dentist
appointment next thursday"). When it encounters a time change that is
made by some other application, it basically wants to disable automatic
time adjustment and trigger the events/notifications which are due at this
(new) time.

Regards,
--
Alex

2010-08-05 12:39:33

by Alexander Shishkin

[permalink] [raw]
Subject: Re: [PATCH] [RFC] notify userspace about time changes

On 5 August 2010 03:52, john stultz <[email protected]> wrote:
> On Wed, 2010-08-04 at 12:48 -0600, Chris Friesen wrote:
>> On 08/04/2010 09:58 AM, john stultz wrote:
>>
>> > Is there a actual use case that you need this for?  I don't really have
>> > an issue with the code I just really want to make sure the feature would
>> > be useful enough to justify the API and code maintenance going forward.
>>
>> We actually added a time-change-notification mechanism internally a long
>> time ago but never saw demand for it and so never bothered trying to
>> push it upstream.  Ours is signal-based.
>>
>> Among other things we use it to pass on time-change notifications to an
>> emulator running a proprietary OS that really cares about having an
>> accurate time-of-day but can't afford a syscall to retrieve it every time.
>
> So the eventfd based method (and the filtering) proposed would work for
> you?

I think that Kirill has a point and the filtering is really there for
no gain. The user
can indeed count his own time changes and compare that against the eventfd
counter.

Regards,
--
Alex

2010-08-05 14:20:19

by Chris Friesen

[permalink] [raw]
Subject: Re: [PATCH] [RFC] notify userspace about time changes

On 08/04/2010 06:52 PM, john stultz wrote:
> On Wed, 2010-08-04 at 12:48 -0600, Chris Friesen wrote:
>> On 08/04/2010 09:58 AM, john stultz wrote:
>>
>>> Is there a actual use case that you need this for? I don't really have
>>> an issue with the code I just really want to make sure the feature would
>>> be useful enough to justify the API and code maintenance going forward.
>>
>> We actually added a time-change-notification mechanism internally a long
>> time ago but never saw demand for it and so never bothered trying to
>> push it upstream. Ours is signal-based.
>>
>> Among other things we use it to pass on time-change notifications to an
>> emulator running a proprietary OS that really cares about having an
>> accurate time-of-day but can't afford a syscall to retrieve it every time.
>
> So the eventfd based method (and the filtering) proposed would work for
> you?

The eventfd based method would work. The filtering is unnecessary for
us and seems somewhat fragile as currently implemented.

Chris

--
Chris Friesen
Software Developer
GENBAND
[email protected]
http://www.genband.com

2010-08-05 21:11:21

by john stultz

[permalink] [raw]
Subject: Re: [PATCH] [RFC] notify userspace about time changes

On Thu, 2010-08-05 at 15:33 +0300, Alexander Shishkin wrote:
> On 4 August 2010 18:58, john stultz <[email protected]> wrote:
> > Is there a actual use case that you need this for? I don't really have
> > an issue with the code I just really want to make sure the feature would
> > be useful enough to justify the API and code maintenance going forward.
>
> Yes. What we have here is an application which takes care of different means
> of time synchronization (trusted time servers, different GSM operators, etc)
> and also different kinds of time-based events/notifications (like "dentist
> appointment next thursday"). When it encounters a time change that is
> made by some other application, it basically wants to disable automatic
> time adjustment and trigger the events/notifications which are due at this
> (new) time.

Ok. Something specific is always more helpful then theoretical uses.

I think the filtering is still a bit controversial, so you might want to
respin it without that. But otherwise I'm ok with it as long as no one
else objects to any of the minor details of the interface

GregKH: Does /sys/kernel/time_notify seem ok by you?

thanks
-john

2010-08-05 21:39:05

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] [RFC] notify userspace about time changes

On Thu, Aug 05, 2010 at 02:11:05PM -0700, john stultz wrote:
> On Thu, 2010-08-05 at 15:33 +0300, Alexander Shishkin wrote:
> > On 4 August 2010 18:58, john stultz <[email protected]> wrote:
> > > Is there a actual use case that you need this for? I don't really have
> > > an issue with the code I just really want to make sure the feature would
> > > be useful enough to justify the API and code maintenance going forward.
> >
> > Yes. What we have here is an application which takes care of different means
> > of time synchronization (trusted time servers, different GSM operators, etc)
> > and also different kinds of time-based events/notifications (like "dentist
> > appointment next thursday"). When it encounters a time change that is
> > made by some other application, it basically wants to disable automatic
> > time adjustment and trigger the events/notifications which are due at this
> > (new) time.
>
> Ok. Something specific is always more helpful then theoretical uses.
>
> I think the filtering is still a bit controversial, so you might want to
> respin it without that. But otherwise I'm ok with it as long as no one
> else objects to any of the minor details of the interface
>
> GregKH: Does /sys/kernel/time_notify seem ok by you?

Um, it depends, what is that file going to do? I don't see a
Documentation/ABI/ entry here that describes it fully :)

thanks,

greg k-h

2010-08-05 22:18:11

by Kay Sievers

[permalink] [raw]
Subject: Re: [PATCH] [RFC] notify userspace about time changes

On Thu, Aug 5, 2010 at 23:11, john stultz <[email protected]> wrote:
> On Thu, 2010-08-05 at 15:33 +0300, Alexander Shishkin wrote:
>> On 4 August 2010 18:58, john stultz <[email protected]> wrote:
>> > Is there a actual use case that you need this for?  I don't really have
>> > an issue with the code I just really want to make sure the feature would
>> > be useful enough to justify the API and code maintenance going forward.

Basically everything that schedules an action based on an absolute
time specification, like at 3pm today, and not in 3 hours from now,
needs to track such system time changes. Otherwise it has to do
nonsense like cron does, to wake up every minute to check the current
time.

Kay

2010-08-05 22:22:46

by Kay Sievers

[permalink] [raw]
Subject: Re: [PATCH] [RFC] notify userspace about time changes

On Thu, Aug 5, 2010 at 23:38, Greg KH <[email protected]> wrote:
> On Thu, Aug 05, 2010 at 02:11:05PM -0700, john stultz wrote:
>> On Thu, 2010-08-05 at 15:33 +0300, Alexander Shishkin wrote:
>> > On 4 August 2010 18:58, john stultz <[email protected]> wrote:
>> > > Is there a actual use case that you need this for?  I don't really have
>> > > an issue with the code I just really want to make sure the feature would
>> > > be useful enough to justify the API and code maintenance going forward.
>> >
>> > Yes. What we have here is an application which takes care of different means
>> > of time synchronization (trusted time servers, different GSM operators, etc)
>> > and also different kinds of time-based events/notifications (like "dentist
>> > appointment next thursday"). When it encounters a time change that is
>> > made by some other application, it basically wants to disable automatic
>> > time adjustment and trigger the events/notifications which are due at this
>> > (new) time.
>>
>> Ok. Something specific is always more helpful then theoretical uses.
>>
>> I think the filtering is still a bit controversial, so you might want to
>> respin it without that. But otherwise I'm ok with it as long as no one
>> else objects to any of the minor details of the interface
>>
>> GregKH: Does /sys/kernel/time_notify seem ok by you?
>
> Um, it depends, what is that file going to do?  I don't see a
> Documentation/ABI/ entry here that describes it fully :)

I think that's really awkward interface, to pass file descriptor
numbers around and write them to magic sysfs files.

I would very much prefer a file that contains the current time, and
wakes up possible users with a POLL_ERR on changes caused by some
other process. That works very well for things like /proc/mounts, is
easy to get, and does not need a full page of weird instructions to
get stuff done. :)

Kay

2010-08-05 22:30:15

by john stultz

[permalink] [raw]
Subject: Re: [PATCH] [RFC] notify userspace about time changes

On Fri, 2010-08-06 at 00:17 +0200, Kay Sievers wrote:
> On Thu, Aug 5, 2010 at 23:11, john stultz <[email protected]> wrote:
> > On Thu, 2010-08-05 at 15:33 +0300, Alexander Shishkin wrote:
> >> On 4 August 2010 18:58, john stultz <[email protected]> wrote:
> >> > Is there a actual use case that you need this for? I don't really have
> >> > an issue with the code I just really want to make sure the feature would
> >> > be useful enough to justify the API and code maintenance going forward.
>
> Basically everything that schedules an action based on an absolute
> time specification, like at 3pm today, and not in 3 hours from now,
> needs to track such system time changes. Otherwise it has to do
> nonsense like cron does, to wake up every minute to check the current
> time.

time_create(CLOCK_REALTIME,...) creates absolute (not relative) timers
that should be adjusted when the clock is changed. Is that not the case?

-john

2010-08-05 22:31:42

by john stultz

[permalink] [raw]
Subject: Re: [PATCH] [RFC] notify userspace about time changes

On Thu, 2010-08-05 at 15:29 -0700, john stultz wrote:
> On Fri, 2010-08-06 at 00:17 +0200, Kay Sievers wrote:
> > On Thu, Aug 5, 2010 at 23:11, john stultz <[email protected]> wrote:
> > > On Thu, 2010-08-05 at 15:33 +0300, Alexander Shishkin wrote:
> > >> On 4 August 2010 18:58, john stultz <[email protected]> wrote:
> > >> > Is there a actual use case that you need this for? I don't really have
> > >> > an issue with the code I just really want to make sure the feature would
> > >> > be useful enough to justify the API and code maintenance going forward.
> >
> > Basically everything that schedules an action based on an absolute
> > time specification, like at 3pm today, and not in 3 hours from now,
> > needs to track such system time changes. Otherwise it has to do
> > nonsense like cron does, to wake up every minute to check the current
> > time.
>
> time_create(CLOCK_REALTIME,...) creates absolute (not relative) timers

Sorry, *timer_create*.

thanks
-john

2010-08-05 22:35:33

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH] [RFC] notify userspace about time changes

On 08/05/2010 03:22 PM, Kay Sievers wrote:
>
> I think that's really awkward interface, to pass file descriptor
> numbers around and write them to magic sysfs files.
>
> I would very much prefer a file that contains the current time, and
> wakes up possible users with a POLL_ERR on changes caused by some
> other process. That works very well for things like /proc/mounts, is
> easy to get, and does not need a full page of weird instructions to
> get stuff done. :)
>

Okay, what's wrong with having a file descriptor that gets *written to*
on a notification? Why POLL_ERR?

-hpa

2010-08-05 22:39:22

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] [RFC] notify userspace about time changes

On Fri, Aug 06, 2010 at 12:22:27AM +0200, Kay Sievers wrote:
> On Thu, Aug 5, 2010 at 23:38, Greg KH <[email protected]> wrote:
> > On Thu, Aug 05, 2010 at 02:11:05PM -0700, john stultz wrote:
> >> On Thu, 2010-08-05 at 15:33 +0300, Alexander Shishkin wrote:
> >> > On 4 August 2010 18:58, john stultz <[email protected]> wrote:
> >> > > Is there a actual use case that you need this for? ?I don't really have
> >> > > an issue with the code I just really want to make sure the feature would
> >> > > be useful enough to justify the API and code maintenance going forward.
> >> >
> >> > Yes. What we have here is an application which takes care of different means
> >> > of time synchronization (trusted time servers, different GSM operators, etc)
> >> > and also different kinds of time-based events/notifications (like "dentist
> >> > appointment next thursday"). When it encounters a time change that is
> >> > made by some other application, it basically wants to disable automatic
> >> > time adjustment and trigger the events/notifications which are due at this
> >> > (new) time.
> >>
> >> Ok. Something specific is always more helpful then theoretical uses.
> >>
> >> I think the filtering is still a bit controversial, so you might want to
> >> respin it without that. But otherwise I'm ok with it as long as no one
> >> else objects to any of the minor details of the interface
> >>
> >> GregKH: Does /sys/kernel/time_notify seem ok by you?
> >
> > Um, it depends, what is that file going to do? ?I don't see a
> > Documentation/ABI/ entry here that describes it fully :)
>
> I think that's really awkward interface, to pass file descriptor
> numbers around and write them to magic sysfs files.

Ick, really? That's not ok for sysfs.

> I would very much prefer a file that contains the current time, and
> wakes up possible users with a POLL_ERR on changes caused by some
> other process. That works very well for things like /proc/mounts, is
> easy to get, and does not need a full page of weird instructions to
> get stuff done. :)

That sounds more reasonable.

thanks,

greg k-h

2010-08-05 22:40:22

by Kay Sievers

[permalink] [raw]
Subject: Re: [PATCH] [RFC] notify userspace about time changes

On Fri, Aug 6, 2010 at 00:34, H. Peter Anvin <[email protected]> wrote:
> On 08/05/2010 03:22 PM, Kay Sievers wrote:
>>
>> I think that's really awkward interface, to pass file descriptor
>> numbers around and write them to magic sysfs files.
>>
>> I would very much prefer a file that contains the current time, and
>> wakes up possible users with a POLL_ERR on changes caused by some
>> other process. That works very well for things like /proc/mounts, is
>> easy to get, and does not need a full page of weird instructions to
>> get stuff done. :)
>>
>
> Okay, what's wrong with having a file descriptor that gets *written to*
> on a notification?

Because it needs documentation, and is just not needed for such a
simple thing, I think. Why would you want to write a fd number to a
magic file, which can be your fd right away, even passing you the data
on read().

> Why POLL_ERR?

Because normal files can not be poll()ed, and it's not that new data
has arrived, it just tells you to rewind and read it again. It's
commonly used:
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=5addc5dd8836aa061f6efc4a0d9ba6323726297a

Kay

2010-08-05 22:44:25

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH] [RFC] notify userspace about time changes

On 08/05/2010 03:39 PM, Kay Sievers wrote:
>>
>> Okay, what's wrong with having a file descriptor that gets *written to*
>> on a notification?
>
> Because it needs documentation, and is just not needed for such a
> simple thing, I think. Why would you want to write a fd number to a
> magic file, which can be your fd right away, even passing you the data
> on read().
>

I didn't mean that, I meant a note that you open and get a pipe/socket/*.

>> Why POLL_ERR?
>
> Because normal files can not be poll()ed, and it's not that new data
> has arrived, it just tells you to rewind and read it again. It's
> commonly used:
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=5addc5dd8836aa061f6efc4a0d9ba6323726297a

It makes sense there, I guess, as some kind of sideband notification is
highly useful. Too bad we don't have a generic mechanism on files other
than inotify... a lot of things could use it (including tail, which I
think uses inotify now.)

-hpa

2010-08-05 23:51:08

by Kay Sievers

[permalink] [raw]
Subject: Re: [PATCH] [RFC] notify userspace about time changes

On Fri, Aug 6, 2010 at 00:29, john stultz <[email protected]> wrote:
> On Fri, 2010-08-06 at 00:17 +0200, Kay Sievers wrote:
>> On Thu, Aug 5, 2010 at 23:11, john stultz <[email protected]> wrote:
>> > On Thu, 2010-08-05 at 15:33 +0300, Alexander Shishkin wrote:
>> >> On 4 August 2010 18:58, john stultz <[email protected]> wrote:
>> >> > Is there a actual use case that you need this for?  I don't really have
>> >> > an issue with the code I just really want to make sure the feature would
>> >> > be useful enough to justify the API and code maintenance going forward.
>>
>> Basically everything that schedules an action based on an absolute
>> time specification, like at 3pm today, and not in 3 hours from now,
>> needs to track such system time changes. Otherwise it has to do
>> nonsense like cron does, to wake up every minute to check the current
>> time.
>
> time_create(CLOCK_REALTIME,...) creates absolute (not relative) timers
> that should be adjusted when the clock is changed. Is that not the case?

That works, yes. The created timer is still is a fixed value, and it
gets automatically adjusted when the system time changes.

This is the example Lennart and I thought about when we considered
adding cron-like stuff to systemd's timer configs, but didn't want to
do silly things like scheduled checks for the actual time, so we
delayed this feature until such a notification becomes available.

Consider we want stuff like "wakeup every day at 3pm", the next wakeup
might be earlier than the timer we calculated last time, on system
time changes. We need to re-calculate it. This is necessary for all
repeating events.

Say we want to wakeup at 3pm, now it's 4pm, so we schedule it in 23
hours. Now the system time changes to 2pm, and we would expect to
wakeup in one hour, but we take 25.

Kay

2010-08-06 00:16:57

by john stultz

[permalink] [raw]
Subject: Re: [PATCH] [RFC] notify userspace about time changes

On Fri, 2010-08-06 at 01:50 +0200, Kay Sievers wrote:
> On Fri, Aug 6, 2010 at 00:29, john stultz <[email protected]> wrote:
> > On Fri, 2010-08-06 at 00:17 +0200, Kay Sievers wrote:
> >> On Thu, Aug 5, 2010 at 23:11, john stultz <[email protected]> wrote:
> >> > On Thu, 2010-08-05 at 15:33 +0300, Alexander Shishkin wrote:
> >> >> On 4 August 2010 18:58, john stultz <[email protected]> wrote:
> >> >> > Is there a actual use case that you need this for? I don't really have
> >> >> > an issue with the code I just really want to make sure the feature would
> >> >> > be useful enough to justify the API and code maintenance going forward.
> >>
> >> Basically everything that schedules an action based on an absolute
> >> time specification, like at 3pm today, and not in 3 hours from now,
> >> needs to track such system time changes. Otherwise it has to do
> >> nonsense like cron does, to wake up every minute to check the current
> >> time.
> >
> > time_create(CLOCK_REALTIME,...) creates absolute (not relative) timers
> > that should be adjusted when the clock is changed. Is that not the case?
>
> That works, yes. The created timer is still is a fixed value, and it
> gets automatically adjusted when the system time changes.
>
> This is the example Lennart and I thought about when we considered
> adding cron-like stuff to systemd's timer configs, but didn't want to
> do silly things like scheduled checks for the actual time, so we
> delayed this feature until such a notification becomes available.
>
> Consider we want stuff like "wakeup every day at 3pm", the next wakeup
> might be earlier than the timer we calculated last time, on system
> time changes. We need to re-calculate it. This is necessary for all
> repeating events.
>
> Say we want to wakeup at 3pm, now it's 4pm, so we schedule it in 23
> hours. Now the system time changes to 2pm, and we would expect to
> wakeup in one hour, but we take 25.

Ah. Yea. So its not really an issue scheduling absolute times, but
issues around absolute recurrences when time might go backwards.

So yea, that's another good example in favor of Alexander's patch (at
least the basic functionality, modulo the interface
details/documentation).

Thanks for the clarification!
-john



2010-08-06 07:16:14

by Alexander Shishkin

[permalink] [raw]
Subject: Re: [PATCH] [RFC] notify userspace about time changes

On Fri, Aug 06, 2010 at 12:22:27 +0200, Kay Sievers wrote:
> On Thu, Aug 5, 2010 at 23:38, Greg KH <[email protected]> wrote:
> > On Thu, Aug 05, 2010 at 02:11:05PM -0700, john stultz wrote:
> >> On Thu, 2010-08-05 at 15:33 +0300, Alexander Shishkin wrote:
> >> > On 4 August 2010 18:58, john stultz <[email protected]> wrote:
> >> > > Is there a actual use case that you need this for? ?I don't really have
> >> > > an issue with the code I just really want to make sure the feature would
> >> > > be useful enough to justify the API and code maintenance going forward.
> >> >
> >> > Yes. What we have here is an application which takes care of different means
> >> > of time synchronization (trusted time servers, different GSM operators, etc)
> >> > and also different kinds of time-based events/notifications (like "dentist
> >> > appointment next thursday"). When it encounters a time change that is
> >> > made by some other application, it basically wants to disable automatic
> >> > time adjustment and trigger the events/notifications which are due at this
> >> > (new) time.
> >>
> >> Ok. Something specific is always more helpful then theoretical uses.
> >>
> >> I think the filtering is still a bit controversial, so you might want to
> >> respin it without that. But otherwise I'm ok with it as long as no one
> >> else objects to any of the minor details of the interface
> >>
> >> GregKH: Does /sys/kernel/time_notify seem ok by you?
> >
> > Um, it depends, what is that file going to do? ?I don't see a
> > Documentation/ABI/ entry here that describes it fully :)
>
> I think that's really awkward interface, to pass file descriptor
> numbers around and write them to magic sysfs files.

Unfortunately, the magic file descriptor is the only viable option for us
(see the usecase I described earlier), for a simple reason that it gives
you the *number of times* the system time has changed so that the user
can see when someone else is changing the time, for example.

Plus, using the magic sysfs file you can specify what kind of events you
want to subscribe to, eventually. Like "everything but my changes", "only
my changes", "everything", etc. It can be extendable.

> I would very much prefer a file that contains the current time, and
> wakes up possible users with a POLL_ERR on changes caused by some
> other process. That works very well for things like /proc/mounts, is
> easy to get, and does not need a full page of weird instructions to
> get stuff done. :)

So you get a POLLERR because there was a time change that you expected
and then someone who wants his time change to go unnoticed changes it
again before you started polling again.

Full page of weird instructions you are referring to is as follows (sans
error checking):

efd = eventfd(0, 0);
fd = open("/sys/kernel/time_notify", O_WRONLY);
fdprintf(fd, "%d", efd);
close(fd);

fds[0].fd = efd;
fds[0].events = POLLIN;
while (poll(fds, 1, -1) > 0) {
uint64_t n;

read(efd, &n, 8);
printf("system time has been modified %d time(s)\n", n);
}

Regards,
--
Alex

2010-08-06 07:27:01

by Alexander Shishkin

[permalink] [raw]
Subject: Re: [PATCH] [RFC] notify userspace about time changes

On Thu, Aug 05, 2010 at 03:38:54 -0700, Greg KH wrote:
> On Fri, Aug 06, 2010 at 12:22:27AM +0200, Kay Sievers wrote:
> > On Thu, Aug 5, 2010 at 23:38, Greg KH <[email protected]> wrote:
> > > On Thu, Aug 05, 2010 at 02:11:05PM -0700, john stultz wrote:
> > >> On Thu, 2010-08-05 at 15:33 +0300, Alexander Shishkin wrote:
> > >> > On 4 August 2010 18:58, john stultz <[email protected]> wrote:
> > >> > > Is there a actual use case that you need this for? ?I don't really have
> > >> > > an issue with the code I just really want to make sure the feature would
> > >> > > be useful enough to justify the API and code maintenance going forward.
> > >> >
> > >> > Yes. What we have here is an application which takes care of different means
> > >> > of time synchronization (trusted time servers, different GSM operators, etc)
> > >> > and also different kinds of time-based events/notifications (like "dentist
> > >> > appointment next thursday"). When it encounters a time change that is
> > >> > made by some other application, it basically wants to disable automatic
> > >> > time adjustment and trigger the events/notifications which are due at this
> > >> > (new) time.
> > >>
> > >> Ok. Something specific is always more helpful then theoretical uses.
> > >>
> > >> I think the filtering is still a bit controversial, so you might want to
> > >> respin it without that. But otherwise I'm ok with it as long as no one
> > >> else objects to any of the minor details of the interface
> > >>
> > >> GregKH: Does /sys/kernel/time_notify seem ok by you?
> > >
> > > Um, it depends, what is that file going to do? ?I don't see a
> > > Documentation/ABI/ entry here that describes it fully :)
> >
> > I think that's really awkward interface, to pass file descriptor
> > numbers around and write them to magic sysfs files.
>
> Ick, really? That's not ok for sysfs.

This is how cgroups notifications work [1], for example. One obtains an
eventfd, one then writes its descriptor to a certain control file, one polls
said eventfd for events. The type of event is echoed to the same control file
together with the fd.

So, my idea was, basically to use the sysfs file as means to specify what
kind of time-related events a program wants to subscribe to. For example,

<int:eventfd descriptor> <bool:want other's time changes> <bool:want own time
changes> <bool:want settimeofday> <bool:want adjtime>

> > I would very much prefer a file that contains the current time, and
> > wakes up possible users with a POLL_ERR on changes caused by some
> > other process. That works very well for things like /proc/mounts, is
> > easy to get, and does not need a full page of weird instructions to
> > get stuff done. :)
>
> That sounds more reasonable.

Polling a simple sysfs file is not enough as I described in the other mail,
unless it implements a counter similar to eventfd, which would effectively
make it another eventfd implementation, only with a dentry this time.

[1] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=kernel/cgroup.c;h=a8ce099544049e787464c3e4261ac70625cc0653;hb=HEAD#l3071 onwards

Regards,
--
Alex

2010-08-06 07:43:49

by Bastien Roucariès

[permalink] [raw]
Subject: Re: [PATCH] [RFC] notify userspace about time changes

On Fri, Aug 6, 2010 at 12:22 AM, Kay Sievers <[email protected]> wrote:
> On Thu, Aug 5, 2010 at 23:38, Greg KH <[email protected]> wrote:
>> On Thu, Aug 05, 2010 at 02:11:05PM -0700, john stultz wrote:
>>> On Thu, 2010-08-05 at 15:33 +0300, Alexander Shishkin wrote:
>>> > On 4 August 2010 18:58, john stultz <[email protected]> wrote:
>>> > > Is there a actual use case that you need this for? ?I don't really have
>>> > > an issue with the code I just really want to make sure the feature would
>>> > > be useful enough to justify the API and code maintenance going forward.
>>> >
>>> > Yes. What we have here is an application which takes care of different means
>>> > of time synchronization (trusted time servers, different GSM operators, etc)
>>> > and also different kinds of time-based events/notifications (like "dentist
>>> > appointment next thursday"). When it encounters a time change that is
>>> > made by some other application, it basically wants to disable automatic
>>> > time adjustment and trigger the events/notifications which are due at this
>>> > (new) time.
>>>
>>> Ok. Something specific is always more helpful then theoretical uses.
>>>
>>> I think the filtering is still a bit controversial, so you might want to
>>> respin it without that. But otherwise I'm ok with it as long as no one
>>> else objects to any of the minor details of the interface
>>>
>>> GregKH: Does /sys/kernel/time_notify seem ok by you?
>>
>> Um, it depends, what is that file going to do? ?I don't see a
>> Documentation/ABI/ entry here that describes it fully :)
>
> I think that's really awkward interface, to pass file descriptor
> numbers around and write them to magic sysfs files.
>
> I would very much prefer a file that contains the current time, and
> wakes up possible users with a POLL_ERR on changes caused by some
> other process. That works very well for things like /proc/mounts, is
> easy to get, and does not need a full page of weird instructions to
> get stuff done. :)
>
See http://lwn.net/Articles/323658/ and it is plan9 compatible

Bastien

2010-08-06 23:50:17

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] [RFC] notify userspace about time changes

On Fri, Aug 06, 2010 at 10:21:45AM +0300, Alexander Shishkin wrote:
> On Thu, Aug 05, 2010 at 03:38:54 -0700, Greg KH wrote:
> > On Fri, Aug 06, 2010 at 12:22:27AM +0200, Kay Sievers wrote:
> > > On Thu, Aug 5, 2010 at 23:38, Greg KH <[email protected]> wrote:
> > > > On Thu, Aug 05, 2010 at 02:11:05PM -0700, john stultz wrote:
> > > >> On Thu, 2010-08-05 at 15:33 +0300, Alexander Shishkin wrote:
> > > >> > On 4 August 2010 18:58, john stultz <[email protected]> wrote:
> > > >> > > Is there a actual use case that you need this for? ?I don't really have
> > > >> > > an issue with the code I just really want to make sure the feature would
> > > >> > > be useful enough to justify the API and code maintenance going forward.
> > > >> >
> > > >> > Yes. What we have here is an application which takes care of different means
> > > >> > of time synchronization (trusted time servers, different GSM operators, etc)
> > > >> > and also different kinds of time-based events/notifications (like "dentist
> > > >> > appointment next thursday"). When it encounters a time change that is
> > > >> > made by some other application, it basically wants to disable automatic
> > > >> > time adjustment and trigger the events/notifications which are due at this
> > > >> > (new) time.
> > > >>
> > > >> Ok. Something specific is always more helpful then theoretical uses.
> > > >>
> > > >> I think the filtering is still a bit controversial, so you might want to
> > > >> respin it without that. But otherwise I'm ok with it as long as no one
> > > >> else objects to any of the minor details of the interface
> > > >>
> > > >> GregKH: Does /sys/kernel/time_notify seem ok by you?
> > > >
> > > > Um, it depends, what is that file going to do? ?I don't see a
> > > > Documentation/ABI/ entry here that describes it fully :)
> > >
> > > I think that's really awkward interface, to pass file descriptor
> > > numbers around and write them to magic sysfs files.
> >
> > Ick, really? That's not ok for sysfs.
>
> This is how cgroups notifications work [1], for example.

Yes, but that's not the way that sysfs works. sysfs is
one-value-per-file simple text files. With a few binary files that are
"pass-through" to the hardware.

It doesn't have these types of "magic" files, please don't try to add
them without a very large discussion, and boatloads of documentation :)

thanks,

greg k-h