[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <1288898940.2766.17.camel@work-vm>
Date: Thu, 04 Nov 2010 12:29:00 -0700
From: John Stultz <john.stultz@...aro.org>
To: Richard Cochran <richardcochran@...il.com>
Cc: LKML <linux-kernel@...r.kernel.org>,
Arve Hjønnevåg <arve@...roid.com>,
Thomas Gleixner <tglx@...utronix.de>,
Alessandro Zummo <a.zummo@...ertech.it>
Subject: Re: [PATCH 7/7] [RFC] Introduce Alarm (hybrid) timers
On Thu, 2010-11-04 at 08:52 +0100, Richard Cochran wrote:
> On Wed, Nov 03, 2010 at 11:31:19AM -0700, John Stultz wrote:
>
> > Another large distinction is that while the in-kernel interface
> > is pretty similar, the user-space interface for android alarm
> > timers is via ioctls. I've instead chosen to export this
> > functionality via the posix interface, as it seemed a little
> > simpler and avoids creating duplicate interfaces to things like
> > CLOCK_REALTIME and CLOCK_MONOTONIC under alternate names (ie:
> > RTC and ELAPSED_REALTIME). Instead, if one wants to use a
> > alarm timer, simply create a posix timer against either
> > CLOCK_REALTIME_ALARM or CLOCK_MONOTONIC_ALARM.
>
> I have a comment on this, below ...
>
> > diff --git a/include/linux/posix-timers.h b/include/linux/posix-timers.h
> > index e6b46b5..9d1ace6 100644
> > --- a/include/linux/posix-timers.h
> > +++ b/include/linux/posix-timers.h
> > @@ -5,6 +5,7 @@
> > #include <linux/list.h>
> > #include <linux/sched.h>
> > #include <linux/rtc.h>
> > +#include <linux/alarmtimer.h>
> >
> > union cpu_time_count {
> > cputime_t cpu;
> > @@ -65,6 +66,7 @@ struct k_itimer {
> > unsigned long expires;
> > } mmtimer;
> > struct rtc_timer rtctimer;
> > + struct alarm alarmtimer;
> > } it;
> > };
>
> I have an initial dynamic clock patch set ready for review and will
> post it later today if I can. In implementing the timer_ calls, I
> began to wonder about this 'it' union. If we really allow and
> implement many kinds of dynamic clocks, then it would seem ugly to me
> to simply add yet another union member for each new clock. Wouldn't it
> be better to provide a private void pointer for the underlying
> driver's use?
Yea. That union is getting a bit ugly, but it lets us avoid having to
manage allocating and freeing a parallel timer structure that points
back to the k_itimer. Its not a huge issue, but the code in this case is
much simpler, but at the cost of making the k_itimer it union a little
gross. So yea, I'm fine changing it if there's a real desire for it to
be done.
> > diff --git a/include/linux/time.h b/include/linux/time.h
> > index 914c48d..4791858 100644
> > --- a/include/linux/time.h
> > +++ b/include/linux/time.h
> > @@ -290,6 +290,8 @@ struct itimerval {
> > #define CLOCK_MONOTONIC_RAW 4
> > #define CLOCK_REALTIME_COARSE 5
> > #define CLOCK_MONOTONIC_COARSE 6
> > +#define CLOCK_REALTIME_ALARM 7
> > +#define CLOCK_MONOTONIC_ALARM 8
>
> I have thought about and have taken Alan Cox's anti-SYS5.4/SuS/POSIX
> enumeration arguments to heart. If you need a really good example of
> the weaknesses of that way, take a look at clock_getcpuclockid,
> pthread_getcpuclockid, and their implementation in posix-cpu-timers.c
>
> If we are serious about dynamic clocks, then we will never again touch
> the CLOCK_ list. I hope that, after reviewing the coming patch set,
> you will also agree ;)
Well, I'm eager to see you patch, and I do think dynamic clockids are a
very useful concept that we need.
However, I suspect there will still be a need for static clockids for
those cases where its a conceptual api that doesn't deal with specific
hardware that has lifetime issues, such as the MONOTONIC/REALTIME_ALARM
cases above.
I look forward to your patches!
thanks
-john
--
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/
Powered by blists - more mailing lists