linux-kernel - Re: [PATCH 7/7] [RFC] Introduce Alarm (hybrid) timers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20101104075229.GA28086@riccoc20.at.omicron.at>
Date:	Thu, 4 Nov 2010 08:52:29 +0100
From:	Richard Cochran <richardcochran@...il.com>
To:	John Stultz <john.stultz@...aro.org>
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 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?

> 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 ;)

Richard

--
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/