[2/2] posix clocks: introduce a sysfs presence.

This patch adds a 'timesource' class into sysfs. Each registered POSIX
clock appears by name under /sys/class/timesource. The idea is to
expose to user space the dynamic mapping between clock devices and
clock IDs.

Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
---
 Documentation/ABI/testing/sysfs-timesource |   24 ++++++++++++++++
 drivers/char/mmtimer.c                     |    1 +
 include/linux/posix-timers.h               |    4 +++
 kernel/posix-cpu-timers.c                  |    2 +
 kernel/posix-timers.c                      |   40 ++++++++++++++++++++++++++++
 5 files changed, 71 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-timesource

On Fri, Sep 3, 2010 at 2:30 AM, Richard Cochran
<richardcochran@gmail.com> wrote:
> This patch adds a 'timesource' class into sysfs. Each registered POSIX
> clock appears by name under /sys/class/timesource. The idea is to
> expose to user space the dynamic mapping between clock devices and
> clock IDs.

So I'm not a fan of this one. Timesource is way to close to
clocksource. Really something like "posix_clockid" would make way more
sense.

But really, I'd prefer we not consolidate the clock_ids into a
dictionary style interface, where we list them all.

Really, I'd much prefer that there be a clockid attribute that hangs
off of the sysfs entry for the device where the PTP clock resides (or
is connected to).  That way we solve the issue of how do we map the
device to the clockid, and provide a method for ptpd to get the
clockid.

It also makes these special ids a little less easy to find for normal
applications (which should be ok, as only special applications that
really need to know the underlying hardware values should be mucking
with these clockids).

thanks
-john


> Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
> ---
>  Documentation/ABI/testing/sysfs-timesource |   24 ++++++++++++++++
>  drivers/char/mmtimer.c                     |    1 +
>  include/linux/posix-timers.h               |    4 +++
>  kernel/posix-cpu-timers.c                  |    2 +
>  kernel/posix-timers.c                      |   40 ++++++++++++++++++++++++++++
>  5 files changed, 71 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-timesource
>
> diff --git a/Documentation/ABI/testing/sysfs-timesource b/Documentation/ABI/testing/sysfs-timesource
> new file mode 100644
> index 0000000..f991de2
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-timesource
> @@ -0,0 +1,24 @@
> +What:          /sys/class/timesource/
> +Date:          September 2010
> +Contact:       Richard Cochran <richardcochran@gmail.com>
> +Description:
> +               This directory contains files and directories
> +               providing a standardized interface to the available
> +               time sources.
> +
> +What:          /sys/class/timesource/<name>/
> +Date:          September 2010
> +Contact:       Richard Cochran <richardcochran@gmail.com>
> +Description:
> +               This directory contains the attributes of a time
> +               source registered with the POSIX clock subsystem.
> +
> +What:          /sys/class/timesource/<name>/id
> +Date:          September 2010
> +Contact:       Richard Cochran <richardcochran@gmail.com>
> +Description:
> +               This file contains the clock ID (a non-negative
> +               integer) of the named time source registered with the
> +               POSIX clock subsystem. This value may be passed as the
> +               first argument to the POSIX clock and timer system
> +               calls. See man CLOCK_GETRES(2) and TIMER_CREATE(2).
> diff --git a/drivers/char/mmtimer.c b/drivers/char/mmtimer.c
> index ea7c99f..e9173e3 100644
> --- a/drivers/char/mmtimer.c
> +++ b/drivers/char/mmtimer.c
> @@ -758,6 +758,7 @@ static int sgi_timer_set(struct k_itimer *timr, int flags,
>  }
>
>  static struct k_clock sgi_clock = {
> +       .name = "sgi_cycle",
>        .res = 0,
>        .clock_set = sgi_clock_set,
>        .clock_get = sgi_clock_get,
> diff --git a/include/linux/posix-timers.h b/include/linux/posix-timers.h
> index 08aa4da..64e6fee 100644
> --- a/include/linux/posix-timers.h
> +++ b/include/linux/posix-timers.h
> @@ -67,7 +67,11 @@ struct k_itimer {
>        } it;
>  };
>
> +#define KCLOCK_MAX_NAME 32
> +
>  struct k_clock {
> +       char name[KCLOCK_MAX_NAME];
> +       struct device *dev;
>        clockid_t id;
>        int res;                /* in nanoseconds */
>        int (*clock_getres) (const clockid_t which_clock, struct timespec *tp);
> diff --git a/kernel/posix-cpu-timers.c b/kernel/posix-cpu-timers.c
> index e1c2e7b..df9cbab 100644
> --- a/kernel/posix-cpu-timers.c
> +++ b/kernel/posix-cpu-timers.c
> @@ -1611,6 +1611,7 @@ static long thread_cpu_nsleep_restart(struct restart_block *restart_block)
>  static __init int init_posix_cpu_timers(void)
>  {
>        struct k_clock process = {
> +               .name = "process_cputime",
>                .clock_getres = process_cpu_clock_getres,
>                .clock_get = process_cpu_clock_get,
>                .clock_set = do_posix_clock_nosettime,
> @@ -1619,6 +1620,7 @@ static __init int init_posix_cpu_timers(void)
>                .nsleep_restart = process_cpu_nsleep_restart,
>        };
>        struct k_clock thread = {
> +               .name = "thread_cputime",
>                .clock_getres = thread_cpu_clock_getres,
>                .clock_get = thread_cpu_clock_get,
>                .clock_set = do_posix_clock_nosettime,
> diff --git a/kernel/posix-timers.c b/kernel/posix-timers.c
> index 67fba5c..719aa11 100644
> --- a/kernel/posix-timers.c
> +++ b/kernel/posix-timers.c
> @@ -46,6 +46,7 @@
>  #include <linux/wait.h>
>  #include <linux/workqueue.h>
>  #include <linux/module.h>
> +#include <linux/device.h>
>
>  /*
>  * Management arrays for POSIX timers.  Timers are kept in slab memory
> @@ -135,6 +136,8 @@ static struct k_clock posix_clocks[MAX_CLOCKS];
>  static DECLARE_BITMAP(clocks_map, MAX_CLOCKS);
>  static DEFINE_MUTEX(clocks_mux); /* protects 'posix_clocks' and 'clocks_map' */
>
> +static struct class *timesource_class;
> +
>  /*
>  * These ones are defined below.
>  */
> @@ -271,20 +274,40 @@ static int posix_get_coarse_res(const clockid_t which_clock, struct timespec *tp
>        *tp = ktime_to_timespec(KTIME_LOW_RES);
>        return 0;
>  }
> +
> +/*
> + * sysfs attributes
> + */
> +
> +static ssize_t show_clock_id(struct device *dev,
> +                            struct device_attribute *attr, char *page)
> +{
> +       struct k_clock *kc = dev_get_drvdata(dev);
> +       return snprintf(page, PAGE_SIZE-1, "%d\n", kc->id);
> +}
> +
> +static struct device_attribute timesource_dev_attrs[] = {
> +       __ATTR(id,   0444, show_clock_id,   NULL),
> +       __ATTR_NULL,
> +};
> +
>  /*
>  * Initialize everything, well, just everything in Posix clocks/timers ;)
>  */
>  static __init int init_posix_timers(void)
>  {
>        struct k_clock clock_realtime = {
> +               .name = "realtime",
>                .clock_getres = hrtimer_get_res,
>        };
>        struct k_clock clock_monotonic = {
> +               .name = "monotonic",
>                .clock_getres = hrtimer_get_res,
>                .clock_get = posix_ktime_get_ts,
>                .clock_set = do_posix_clock_nosettime,
>        };
>        struct k_clock clock_monotonic_raw = {
> +               .name = "monotonic_raw",
>                .clock_getres = hrtimer_get_res,
>                .clock_get = posix_get_monotonic_raw,
>                .clock_set = do_posix_clock_nosettime,
> @@ -292,6 +315,7 @@ static __init int init_posix_timers(void)
>                .nsleep = no_nsleep,
>        };
>        struct k_clock clock_realtime_coarse = {
> +               .name = "realtime_coarse",
>                .clock_getres = posix_get_coarse_res,
>                .clock_get = posix_get_realtime_coarse,
>                .clock_set = do_posix_clock_nosettime,
> @@ -299,6 +323,7 @@ static __init int init_posix_timers(void)
>                .nsleep = no_nsleep,
>        };
>        struct k_clock clock_monotonic_coarse = {
> +               .name = "monotonic_coarse",
>                .clock_getres = posix_get_coarse_res,
>                .clock_get = posix_get_monotonic_coarse,
>                .clock_set = do_posix_clock_nosettime,
> @@ -306,6 +331,13 @@ static __init int init_posix_timers(void)
>                .nsleep = no_nsleep,
>        };
>
> +       timesource_class = class_create(NULL, "timesource");
> +       if (IS_ERR(timesource_class)) {
> +               pr_err("posix-timers: failed to allocate timesource class\n");
> +               return PTR_ERR(timesource_class);
> +       }
> +       timesource_class->dev_attrs = timesource_dev_attrs;
> +
>        register_posix_clock(CLOCK_REALTIME, &clock_realtime);
>        register_posix_clock(CLOCK_MONOTONIC, &clock_monotonic);
>        register_posix_clock(CLOCK_MONOTONIC_RAW, &clock_monotonic_raw);
> @@ -500,6 +532,14 @@ int register_posix_clock(const clockid_t id, struct k_clock *clock)
>        kc = &posix_clocks[id];
>        *kc = *clock;
>        kc->id = id;
> +       kc->dev = device_create(timesource_class, NULL, MKDEV(0, 0),
> +                               kc, "%s", kc->name);
> +       if (IS_ERR(kc->dev)) {
> +               pr_err("failed to create device clock_id %d\n", id);
> +               err = PTR_ERR(kc->dev);
> +               goto out;
> +       }
> +       dev_set_drvdata(kc->dev, kc);
>        set_bit(id, clocks_map);
>  out:
>        mutex_unlock(&clocks_mux);
> --
> 1.7.0.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Fri, 3 Sep 2010 11:30:13 +0200
Richard Cochran <richardcochran@gmail.com> wrote:

> This patch adds a 'timesource' class into sysfs. Each registered POSIX
> clock appears by name under /sys/class/timesource. The idea is to
> expose to user space the dynamic mapping between clock devices and
> clock IDs.

This is intrinsically racy so not a good API at all.

By the time I have acquired an id from sysfs it may have been re-issued
or changed.

POSIX habit of enumerating objects not properties of objects is one of
the brain dead aspects of POSIX that we should have nothing to do with.

Also we have existing time sources that don't follow the poxix clock
model - I can open /dev/rtc and I can open the hpet and so on.

I like /sys/class/time* *but* you need to be able to open the sysfs
device and apply operations to it in order for it to work when your closk
can be dynamically created and destroyed and to get a sane Unix API.

To start with try applying permissions to clock sources via the POSIX
API. That is something that will be required for some applications.

I need to be able to open sys/clock/foo/something and get a meaningful
handle. Sure it's quite likely the operations it supports are related to
the POSIX timer ops.

Alan
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Fri, Sep 10, 2010 at 12:00:08AM +0100, Alan Cox wrote:
> Also we have existing time sources that don't follow the poxix clock
> model - I can open /dev/rtc and I can open the hpet and so on.
> 
> I like /sys/class/time* *but* you need to be able to open the sysfs
> device and apply operations to it in order for it to work when your closk
> can be dynamically created and destroyed and to get a sane Unix API.
> 
> To start with try applying permissions to clock sources via the POSIX
> API. That is something that will be required for some applications.
> 
> I need to be able to open sys/clock/foo/something and get a meaningful
> handle. Sure it's quite likely the operations it supports are related to
> the POSIX timer ops.

Do you mean this:

   id = read(/sys/clock/foo/lock); /* clock is busy, cannot be removed */
   ...
   clock_gettime(id, ts);
   ...
   write(/sys/clock/foo/release, id); /* all done with clock */

I am not sure what you are describing. Can you give an example, like
pseudocode or something?

Thanks,
Richard
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Fri, Sep 10, 2010 at 11:31:35AM +0200, Richard Cochran wrote:
> On Fri, Sep 10, 2010 at 12:00:08AM +0100, Alan Cox wrote:
> > Also we have existing time sources that don't follow the poxix clock
> > model - I can open /dev/rtc and I can open the hpet and so on.
> > 
> > I like /sys/class/time* *but* you need to be able to open the sysfs
> > device and apply operations to it in order for it to work when your closk
> > can be dynamically created and destroyed and to get a sane Unix API.
> > 
> > To start with try applying permissions to clock sources via the POSIX
> > API. That is something that will be required for some applications.
> > 
> > I need to be able to open sys/clock/foo/something and get a meaningful
> > handle. Sure it's quite likely the operations it supports are related to
> > the POSIX timer ops.
> 
> Do you mean this:
> 
>    id = read(/sys/clock/foo/lock); /* clock is busy, cannot be removed */

That's not for sysfs, if you want to do something like this, create
clockfs please :)

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html