[1/8] posix clocks: introduce a syscall for clock tuning.

A new syscall is introduced that allows tuning of a POSIX clock. The
syscall is implemented for four architectures: arm, blackfin, powerpc,
and x86.

The new syscall, clock_adjtime, takes two parameters, the clock ID,
and a pointer to a struct timex. The semantics of the timex struct
have been expanded by one additional mode flag, which allows an
absolute offset correction. When specificied, the clock offset is
immediately corrected by adding the given time value to the current
time value.

Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
---
 arch/arm/include/asm/unistd.h      |    1 +
 arch/arm/kernel/calls.S            |    1 +
 arch/blackfin/include/asm/unistd.h |    3 +-
 arch/blackfin/mach-common/entry.S  |    1 +
 arch/powerpc/include/asm/systbl.h  |    1 +
 arch/powerpc/include/asm/unistd.h  |    3 +-
 arch/x86/ia32/ia32entry.S          |    1 +
 arch/x86/include/asm/unistd_32.h   |    3 +-
 arch/x86/include/asm/unistd_64.h   |    2 +
 arch/x86/kernel/syscall_table_32.S |    1 +
 include/linux/posix-timers.h       |    3 +
 include/linux/syscalls.h           |    2 +
 include/linux/timex.h              |    3 +-
 kernel/compat.c                    |  136 +++++++++++++++++++++++-------------
 kernel/posix-cpu-timers.c          |    4 +
 kernel/posix-timers.c              |   17 +++++
 16 files changed, 130 insertions(+), 52 deletions(-)

On Thu, 2010-09-23 at 19:31 +0200, Richard Cochran wrote:
> A new syscall is introduced that allows tuning of a POSIX clock. The
> syscall is implemented for four architectures: arm, blackfin, powerpc,
> and x86.
> 
> The new syscall, clock_adjtime, takes two parameters, the clock ID,
> and a pointer to a struct timex. The semantics of the timex struct
> have been expanded by one additional mode flag, which allows an
> absolute offset correction. When specificied, the clock offset is
> immediately corrected by adding the given time value to the current
> time value.


So I'd still split this patch up a little bit more.

1) Patch that implements the ADJ_SETOFFSET  (*and its implementation*)
in do_adjtimex.

2) Patch that adds the new syscall and clock_id multiplexing.

3) Patches that wire it up to the rest of the architectures (there's
still a bunch missing here).



And one little nit in the code:

> diff --git a/kernel/posix-timers.c b/kernel/posix-timers.c
> index 9ca4973..446b566 100644
> --- a/kernel/posix-timers.c
> +++ b/kernel/posix-timers.c
> @@ -197,6 +197,14 @@ static int common_timer_create(struct k_itimer *new_timer)
>  	return 0;
>  }
> 
> +static inline int common_clock_adj(const clockid_t which_clock, struct timex *t)
> +{
> +	if (CLOCK_REALTIME == which_clock)
> +		return do_adjtimex(t);
> +	else
> +		return -EOPNOTSUPP;
> +}


Would it make sense to point to the do_adjtimex() in the k_clock
definition for CLOCK_REALTIME rather then conditionalizing it here?



thanks
-john

On Thu, 2010-09-23 at 19:31 +0200, Richard Cochran wrote:
> A new syscall is introduced that allows tuning of a POSIX clock. The
> syscall is implemented for four architectures: arm, blackfin, powerpc,
> and x86.
> 
> The new syscall, clock_adjtime, takes two parameters, the clock ID,
> and a pointer to a struct timex. The semantics of the timex struct
> have been expanded by one additional mode flag, which allows an
> absolute offset correction. When specificied, the clock offset is
> immediately corrected by adding the given time value to the current
> time value.

Any reason why you CC'ed device-tree discuss ?

This list is getting way too much unrelated stuff, which I find
annoying, it would be nice if we were all a bit more careful here with
our CC lists.

Cheers,
Ben.

> Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
> ---
>  arch/arm/include/asm/unistd.h      |    1 +
>  arch/arm/kernel/calls.S            |    1 +
>  arch/blackfin/include/asm/unistd.h |    3 +-
>  arch/blackfin/mach-common/entry.S  |    1 +
>  arch/powerpc/include/asm/systbl.h  |    1 +
>  arch/powerpc/include/asm/unistd.h  |    3 +-
>  arch/x86/ia32/ia32entry.S          |    1 +
>  arch/x86/include/asm/unistd_32.h   |    3 +-
>  arch/x86/include/asm/unistd_64.h   |    2 +
>  arch/x86/kernel/syscall_table_32.S |    1 +
>  include/linux/posix-timers.h       |    3 +
>  include/linux/syscalls.h           |    2 +
>  include/linux/timex.h              |    3 +-
>  kernel/compat.c                    |  136 +++++++++++++++++++++++-------------
>  kernel/posix-cpu-timers.c          |    4 +
>  kernel/posix-timers.c              |   17 +++++
>  16 files changed, 130 insertions(+), 52 deletions(-)
> 
> diff --git a/arch/arm/include/asm/unistd.h b/arch/arm/include/asm/unistd.h
> index c891eb7..f58d881 100644
> --- a/arch/arm/include/asm/unistd.h
> +++ b/arch/arm/include/asm/unistd.h
> @@ -396,6 +396,7 @@
>  #define __NR_fanotify_init		(__NR_SYSCALL_BASE+367)
>  #define __NR_fanotify_mark		(__NR_SYSCALL_BASE+368)
>  #define __NR_prlimit64			(__NR_SYSCALL_BASE+369)
> +#define __NR_clock_adjtime		(__NR_SYSCALL_BASE+370)
>  
>  /*
>   * The following SWIs are ARM private.
> diff --git a/arch/arm/kernel/calls.S b/arch/arm/kernel/calls.S
> index 5c26ecc..430de4c 100644
> --- a/arch/arm/kernel/calls.S
> +++ b/arch/arm/kernel/calls.S
> @@ -379,6 +379,7 @@
>  		CALL(sys_fanotify_init)
>  		CALL(sys_fanotify_mark)
>  		CALL(sys_prlimit64)
> +/* 370 */	CALL(sys_clock_adjtime)
>  #ifndef syscalls_counted
>  .equ syscalls_padding, ((NR_syscalls + 3) & ~3) - NR_syscalls
>  #define syscalls_counted
> diff --git a/arch/blackfin/include/asm/unistd.h b/arch/blackfin/include/asm/unistd.h
> index 14fcd25..79ad99b 100644
> --- a/arch/blackfin/include/asm/unistd.h
> +++ b/arch/blackfin/include/asm/unistd.h
> @@ -392,8 +392,9 @@
>  #define __NR_fanotify_init	371
>  #define __NR_fanotify_mark	372
>  #define __NR_prlimit64		373
> +#define __NR_clock_adjtime	374
>  
> -#define __NR_syscall		374
> +#define __NR_syscall		375
>  #define NR_syscalls		__NR_syscall
>  
>  /* Old optional stuff no one actually uses */
> diff --git a/arch/blackfin/mach-common/entry.S b/arch/blackfin/mach-common/entry.S
> index af1bffa..ee68730 100644
> --- a/arch/blackfin/mach-common/entry.S
> +++ b/arch/blackfin/mach-common/entry.S
> @@ -1631,6 +1631,7 @@ ENTRY(_sys_call_table)
>  	.long _sys_fanotify_init
>  	.long _sys_fanotify_mark
>  	.long _sys_prlimit64
> +	.long _sys_clock_adjtime
>  
>  	.rept NR_syscalls-(.-_sys_call_table)/4
>  	.long _sys_ni_syscall
> diff --git a/arch/powerpc/include/asm/systbl.h b/arch/powerpc/include/asm/systbl.h
> index 3d21266..2485d8f 100644
> --- a/arch/powerpc/include/asm/systbl.h
> +++ b/arch/powerpc/include/asm/systbl.h
> @@ -329,3 +329,4 @@ COMPAT_SYS(rt_tgsigqueueinfo)
>  SYSCALL(fanotify_init)
>  COMPAT_SYS(fanotify_mark)
>  SYSCALL_SPU(prlimit64)
> +COMPAT_SYS_SPU(clock_adjtime)
> diff --git a/arch/powerpc/include/asm/unistd.h b/arch/powerpc/include/asm/unistd.h
> index 597e6f9..85d5067 100644
> --- a/arch/powerpc/include/asm/unistd.h
> +++ b/arch/powerpc/include/asm/unistd.h
> @@ -348,10 +348,11 @@
>  #define __NR_fanotify_init	323
>  #define __NR_fanotify_mark	324
>  #define __NR_prlimit64		325
> +#define __NR_clock_adjtime	326
>  
>  #ifdef __KERNEL__
>  
> -#define __NR_syscalls		326
> +#define __NR_syscalls		327
>  
>  #define __NR__exit __NR_exit
>  #define NR_syscalls	__NR_syscalls
> diff --git a/arch/x86/ia32/ia32entry.S b/arch/x86/ia32/ia32entry.S
> index 518bb99..0ed7896 100644
> --- a/arch/x86/ia32/ia32entry.S
> +++ b/arch/x86/ia32/ia32entry.S
> @@ -851,4 +851,5 @@ ia32_sys_call_table:
>  	.quad sys_fanotify_init
>  	.quad sys32_fanotify_mark
>  	.quad sys_prlimit64		/* 340 */
> +	.quad compat_sys_clock_adjtime
>  ia32_syscall_end:
> diff --git a/arch/x86/include/asm/unistd_32.h b/arch/x86/include/asm/unistd_32.h
> index b766a5e..b6f73f1 100644
> --- a/arch/x86/include/asm/unistd_32.h
> +++ b/arch/x86/include/asm/unistd_32.h
> @@ -346,10 +346,11 @@
>  #define __NR_fanotify_init	338
>  #define __NR_fanotify_mark	339
>  #define __NR_prlimit64		340
> +#define __NR_clock_adjtime	341
>  
>  #ifdef __KERNEL__
>  
> -#define NR_syscalls 341
> +#define NR_syscalls 342
>  
>  #define __ARCH_WANT_IPC_PARSE_VERSION
>  #define __ARCH_WANT_OLD_READDIR
> diff --git a/arch/x86/include/asm/unistd_64.h b/arch/x86/include/asm/unistd_64.h
> index 363e9b8..5ee3085 100644
> --- a/arch/x86/include/asm/unistd_64.h
> +++ b/arch/x86/include/asm/unistd_64.h
> @@ -669,6 +669,8 @@ __SYSCALL(__NR_fanotify_init, sys_fanotify_init)
>  __SYSCALL(__NR_fanotify_mark, sys_fanotify_mark)
>  #define __NR_prlimit64				302
>  __SYSCALL(__NR_prlimit64, sys_prlimit64)
> +#define __NR_clock_adjtime			303
> +__SYSCALL(__NR_clock_adjtime, sys_clock_adjtime)
>  
>  #ifndef __NO_STUBS
>  #define __ARCH_WANT_OLD_READDIR
> diff --git a/arch/x86/kernel/syscall_table_32.S b/arch/x86/kernel/syscall_table_32.S
> index b35786d..68c7b9a 100644
> --- a/arch/x86/kernel/syscall_table_32.S
> +++ b/arch/x86/kernel/syscall_table_32.S
> @@ -340,3 +340,4 @@ ENTRY(sys_call_table)
>  	.long sys_fanotify_init
>  	.long sys_fanotify_mark
>  	.long sys_prlimit64		/* 340 */
> +	.long sys_clock_adjtime
> diff --git a/include/linux/posix-timers.h b/include/linux/posix-timers.h
> index 3e23844..abf61cc 100644
> --- a/include/linux/posix-timers.h
> +++ b/include/linux/posix-timers.h
> @@ -4,6 +4,7 @@
>  #include <linux/spinlock.h>
>  #include <linux/list.h>
>  #include <linux/sched.h>
> +#include <linux/timex.h>
>  
>  union cpu_time_count {
>  	cputime_t cpu;
> @@ -71,6 +72,7 @@ struct k_clock {
>  	int (*clock_getres) (const clockid_t which_clock, struct timespec *tp);
>  	int (*clock_set) (const clockid_t which_clock, struct timespec * tp);
>  	int (*clock_get) (const clockid_t which_clock, struct timespec * tp);
> +	int (*clock_adj) (const clockid_t which_clock, struct timex *tx);
>  	int (*timer_create) (struct k_itimer *timer);
>  	int (*nsleep) (const clockid_t which_clock, int flags,
>  		       struct timespec *, struct timespec __user *);
> @@ -97,6 +99,7 @@ int posix_timer_event(struct k_itimer *timr, int si_private);
>  int posix_cpu_clock_getres(const clockid_t which_clock, struct timespec *ts);
>  int posix_cpu_clock_get(const clockid_t which_clock, struct timespec *ts);
>  int posix_cpu_clock_set(const clockid_t which_clock, const struct timespec *ts);
> +int posix_cpu_clock_adj(const clockid_t which_clock, struct timex *tx);
>  int posix_cpu_timer_create(struct k_itimer *timer);
>  int posix_cpu_nsleep(const clockid_t which_clock, int flags,
>  		     struct timespec *rqtp, struct timespec __user *rmtp);
> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> index e6319d1..0b24775 100644
> --- a/include/linux/syscalls.h
> +++ b/include/linux/syscalls.h
> @@ -313,6 +313,8 @@ asmlinkage long sys_clock_settime(clockid_t which_clock,
>  				const struct timespec __user *tp);
>  asmlinkage long sys_clock_gettime(clockid_t which_clock,
>  				struct timespec __user *tp);
> +asmlinkage long sys_clock_adjtime(clockid_t which_clock,
> +				struct timex __user *tx);
>  asmlinkage long sys_clock_getres(clockid_t which_clock,
>  				struct timespec __user *tp);
>  asmlinkage long sys_clock_nanosleep(clockid_t which_clock, int flags,
> diff --git a/include/linux/timex.h b/include/linux/timex.h
> index 32d852f..82d4b24 100644
> --- a/include/linux/timex.h
> +++ b/include/linux/timex.h
> @@ -73,7 +73,7 @@ struct timex {
>  	long tolerance;		/* clock frequency tolerance (ppm)
>  				 * (read only)
>  				 */
> -	struct timeval time;	/* (read only) */
> +	struct timeval time;	/* (read only, except for ADJ_SETOFFSET) */
>  	long tick;		/* (modified) usecs between clock ticks */
>  
>  	long ppsfreq;           /* pps frequency (scaled ppm) (ro) */
> @@ -101,6 +101,7 @@ struct timex {
>  #define ADJ_ESTERROR		0x0008	/* estimated time error */
>  #define ADJ_STATUS		0x0010	/* clock status */
>  #define ADJ_TIMECONST		0x0020	/* pll time constant */
> +#define ADJ_SETOFFSET		0x0040  /* add 'time' to current time */
>  #define ADJ_TAI			0x0080	/* set TAI offset */
>  #define ADJ_MICRO		0x1000	/* select microsecond resolution */
>  #define ADJ_NANO		0x2000	/* select nanosecond resolution */
> diff --git a/kernel/compat.c b/kernel/compat.c
> index c9e2ec0..38b1d2c 100644
> --- a/kernel/compat.c
> +++ b/kernel/compat.c
> @@ -52,6 +52,64 @@ static int compat_put_timeval(struct compat_timeval __user *o,
>  		put_user(i->tv_usec, &o->tv_usec)) ? -EFAULT : 0;
>  }
>  
> +static int compat_get_timex(struct timex *txc, struct compat_timex __user *utp)
> +{
> +	memset(txc, 0, sizeof(struct timex));
> +
> +	if (!access_ok(VERIFY_READ, utp, sizeof(struct compat_timex)) ||
> +			__get_user(txc->modes, &utp->modes) ||
> +			__get_user(txc->offset, &utp->offset) ||
> +			__get_user(txc->freq, &utp->freq) ||
> +			__get_user(txc->maxerror, &utp->maxerror) ||
> +			__get_user(txc->esterror, &utp->esterror) ||
> +			__get_user(txc->status, &utp->status) ||
> +			__get_user(txc->constant, &utp->constant) ||
> +			__get_user(txc->precision, &utp->precision) ||
> +			__get_user(txc->tolerance, &utp->tolerance) ||
> +			__get_user(txc->time.tv_sec, &utp->time.tv_sec) ||
> +			__get_user(txc->time.tv_usec, &utp->time.tv_usec) ||
> +			__get_user(txc->tick, &utp->tick) ||
> +			__get_user(txc->ppsfreq, &utp->ppsfreq) ||
> +			__get_user(txc->jitter, &utp->jitter) ||
> +			__get_user(txc->shift, &utp->shift) ||
> +			__get_user(txc->stabil, &utp->stabil) ||
> +			__get_user(txc->jitcnt, &utp->jitcnt) ||
> +			__get_user(txc->calcnt, &utp->calcnt) ||
> +			__get_user(txc->errcnt, &utp->errcnt) ||
> +			__get_user(txc->stbcnt, &utp->stbcnt))
> +		return -EFAULT;
> +
> +	return 0;
> +}
> +
> +static int compat_put_timex(struct compat_timex __user *utp, struct timex *txc)
> +{
> +	if (!access_ok(VERIFY_WRITE, utp, sizeof(struct compat_timex)) ||
> +			__put_user(txc->modes, &utp->modes) ||
> +			__put_user(txc->offset, &utp->offset) ||
> +			__put_user(txc->freq, &utp->freq) ||
> +			__put_user(txc->maxerror, &utp->maxerror) ||
> +			__put_user(txc->esterror, &utp->esterror) ||
> +			__put_user(txc->status, &utp->status) ||
> +			__put_user(txc->constant, &utp->constant) ||
> +			__put_user(txc->precision, &utp->precision) ||
> +			__put_user(txc->tolerance, &utp->tolerance) ||
> +			__put_user(txc->time.tv_sec, &utp->time.tv_sec) ||
> +			__put_user(txc->time.tv_usec, &utp->time.tv_usec) ||
> +			__put_user(txc->tick, &utp->tick) ||
> +			__put_user(txc->ppsfreq, &utp->ppsfreq) ||
> +			__put_user(txc->jitter, &utp->jitter) ||
> +			__put_user(txc->shift, &utp->shift) ||
> +			__put_user(txc->stabil, &utp->stabil) ||
> +			__put_user(txc->jitcnt, &utp->jitcnt) ||
> +			__put_user(txc->calcnt, &utp->calcnt) ||
> +			__put_user(txc->errcnt, &utp->errcnt) ||
> +			__put_user(txc->stbcnt, &utp->stbcnt) ||
> +			__put_user(txc->tai, &utp->tai))
> +		return -EFAULT;
> +	return 0;
> +}
> +
>  asmlinkage long compat_sys_gettimeofday(struct compat_timeval __user *tv,
>  		struct timezone __user *tz)
>  {
> @@ -617,6 +675,29 @@ long compat_sys_clock_gettime(clockid_t which_clock,
>  	return err;
>  }
>  
> +long compat_sys_clock_adjtime(clockid_t which_clock,
> +		struct compat_timex __user *utp)
> +{
> +	struct timex txc;
> +	mm_segment_t oldfs;
> +	int err, ret;
> +
> +	err = compat_get_timex(&txc, utp);
> +	if (err)
> +		return err;
> +
> +	oldfs = get_fs();
> +	set_fs(KERNEL_DS);
> +	ret = sys_clock_adjtime(which_clock, (struct timex __user *) &txc);
> +	set_fs(oldfs);
> +
> +	err = compat_put_timex(utp, &txc);
> +	if (err)
> +		return err;
> +
> +	return ret;
> +}
> +
>  long compat_sys_clock_getres(clockid_t which_clock,
>  		struct compat_timespec __user *tp)
>  {
> @@ -951,58 +1032,17 @@ asmlinkage long compat_sys_rt_sigsuspend(compat_sigset_t __user *unewset, compat
>  asmlinkage long compat_sys_adjtimex(struct compat_timex __user *utp)
>  {
>  	struct timex txc;
> -	int ret;
> -
> -	memset(&txc, 0, sizeof(struct timex));
> +	int err, ret;
>  
> -	if (!access_ok(VERIFY_READ, utp, sizeof(struct compat_timex)) ||
> -			__get_user(txc.modes, &utp->modes) ||
> -			__get_user(txc.offset, &utp->offset) ||
> -			__get_user(txc.freq, &utp->freq) ||
> -			__get_user(txc.maxerror, &utp->maxerror) ||
> -			__get_user(txc.esterror, &utp->esterror) ||
> -			__get_user(txc.status, &utp->status) ||
> -			__get_user(txc.constant, &utp->constant) ||
> -			__get_user(txc.precision, &utp->precision) ||
> -			__get_user(txc.tolerance, &utp->tolerance) ||
> -			__get_user(txc.time.tv_sec, &utp->time.tv_sec) ||
> -			__get_user(txc.time.tv_usec, &utp->time.tv_usec) ||
> -			__get_user(txc.tick, &utp->tick) ||
> -			__get_user(txc.ppsfreq, &utp->ppsfreq) ||
> -			__get_user(txc.jitter, &utp->jitter) ||
> -			__get_user(txc.shift, &utp->shift) ||
> -			__get_user(txc.stabil, &utp->stabil) ||
> -			__get_user(txc.jitcnt, &utp->jitcnt) ||
> -			__get_user(txc.calcnt, &utp->calcnt) ||
> -			__get_user(txc.errcnt, &utp->errcnt) ||
> -			__get_user(txc.stbcnt, &utp->stbcnt))
> -		return -EFAULT;
> +	err = compat_get_timex(&txc, utp);
> +	if (err)
> +		return err;
>  
>  	ret = do_adjtimex(&txc);
>  
> -	if (!access_ok(VERIFY_WRITE, utp, sizeof(struct compat_timex)) ||
> -			__put_user(txc.modes, &utp->modes) ||
> -			__put_user(txc.offset, &utp->offset) ||
> -			__put_user(txc.freq, &utp->freq) ||
> -			__put_user(txc.maxerror, &utp->maxerror) ||
> -			__put_user(txc.esterror, &utp->esterror) ||
> -			__put_user(txc.status, &utp->status) ||
> -			__put_user(txc.constant, &utp->constant) ||
> -			__put_user(txc.precision, &utp->precision) ||
> -			__put_user(txc.tolerance, &utp->tolerance) ||
> -			__put_user(txc.time.tv_sec, &utp->time.tv_sec) ||
> -			__put_user(txc.time.tv_usec, &utp->time.tv_usec) ||
> -			__put_user(txc.tick, &utp->tick) ||
> -			__put_user(txc.ppsfreq, &utp->ppsfreq) ||
> -			__put_user(txc.jitter, &utp->jitter) ||
> -			__put_user(txc.shift, &utp->shift) ||
> -			__put_user(txc.stabil, &utp->stabil) ||
> -			__put_user(txc.jitcnt, &utp->jitcnt) ||
> -			__put_user(txc.calcnt, &utp->calcnt) ||
> -			__put_user(txc.errcnt, &utp->errcnt) ||
> -			__put_user(txc.stbcnt, &utp->stbcnt) ||
> -			__put_user(txc.tai, &utp->tai))
> -		ret = -EFAULT;
> +	err = compat_put_timex(utp, &txc);
> +	if (err)
> +		return err;
>  
>  	return ret;
>  }
> diff --git a/kernel/posix-cpu-timers.c b/kernel/posix-cpu-timers.c
> index 6842eeb..e1c2e7b 100644
> --- a/kernel/posix-cpu-timers.c
> +++ b/kernel/posix-cpu-timers.c
> @@ -207,6 +207,10 @@ int posix_cpu_clock_set(const clockid_t which_clock, const struct timespec *tp)
>  	return error;
>  }
>  
> +int posix_cpu_clock_adj(const clockid_t which_clock, struct timex *tx)
> +{
> +	return -EOPNOTSUPP;
> +}
>  
>  /*
>   * Sample a per-thread clock for the given task.
> diff --git a/kernel/posix-timers.c b/kernel/posix-timers.c
> index 9ca4973..446b566 100644
> --- a/kernel/posix-timers.c
> +++ b/kernel/posix-timers.c
> @@ -197,6 +197,14 @@ static int common_timer_create(struct k_itimer *new_timer)
>  	return 0;
>  }
>  
> +static inline int common_clock_adj(const clockid_t which_clock, struct timex *t)
> +{
> +	if (CLOCK_REALTIME == which_clock)
> +		return do_adjtimex(t);
> +	else
> +		return -EOPNOTSUPP;
> +}
> +
>  static int no_timer_create(struct k_itimer *new_timer)
>  {
>  	return -EOPNOTSUPP;
> @@ -969,6 +977,15 @@ SYSCALL_DEFINE2(clock_gettime, const clockid_t, which_clock,
>  
>  }
>  
> +SYSCALL_DEFINE2(clock_adjtime, const clockid_t, which_clock,
> +		struct timex __user *, tx)
> +{
> +	if (invalid_clockid(which_clock))
> +		return -EINVAL;
> +
> +	return CLOCK_DISPATCH(which_clock, clock_adj, (which_clock, tx));
> +}
> +
>  SYSCALL_DEFINE2(clock_getres, const clockid_t, which_clock,
>  		struct timespec __user *, tp)
>  {

On Fri, 24 Sep 2010, Benjamin Herrenschmidt wrote:
> On Thu, 2010-09-23 at 19:31 +0200, Richard Cochran wrote:
> > The new syscall, clock_adjtime, takes two parameters, the clock ID,
> > and a pointer to a struct timex. The semantics of the timex struct
> > have been expanded by one additional mode flag, which allows an
> > absolute offset correction. When specificied, the clock offset is
> > immediately corrected by adding the given time value to the current
> > time value.
> 
> Any reason why you CC'ed device-tree discuss ?
> 
> This list is getting way too much unrelated stuff, which I find
> annoying, it would be nice if we were all a bit more careful here with
> our CC lists.

Says the guy who missed to trim the useless context of the original
mail, which made me scroll down all the way just to find out that
there is nothing to see.

Thanks,

	tglx

On Fri, 2010-09-24 at 00:12 +0200, Thomas Gleixner wrote:
> > This list is getting way too much unrelated stuff, which I find
> > annoying, it would be nice if we were all a bit more careful here
> with
> > our CC lists.
> 
> Says the guy who missed to trim the useless context of the original
> mail, which made me scroll down all the way just to find out that
> there is nothing to see. 

Heh, you can usually ignore what's after my signature :-) At least I
didn't put my reply all the way down the bottom !

Cheers,
Ben.

On Thu, Sep 23, 2010 at 12:48:51PM -0700, john stultz wrote:
> On Thu, 2010-09-23 at 19:31 +0200, Richard Cochran wrote:
> > A new syscall is introduced that allows tuning of a POSIX clock. The
> > syscall is implemented for four architectures: arm, blackfin, powerpc,
> > and x86.
> > 
> > The new syscall, clock_adjtime, takes two parameters, the clock ID,
> > and a pointer to a struct timex. The semantics of the timex struct
> > have been expanded by one additional mode flag, which allows an
> > absolute offset correction. When specificied, the clock offset is
> > immediately corrected by adding the given time value to the current
> > time value.
> 
> 
> So I'd still split this patch up a little bit more.
> 
> 1) Patch that implements the ADJ_SETOFFSET  (*and its implementation*)
> in do_adjtimex.
> 
> 2) Patch that adds the new syscall and clock_id multiplexing.
> 
> 3) Patches that wire it up to the rest of the architectures (there's
> still a bunch missing here).

I was not sure what the policy is about adding syscalls. Is it the
syscall author's responsibility to add it into every arch?

The last time (see a2e2725541fad7) the commit only added half of some
archs, and ignored others. In my patch, the syscall *really* works on
the archs that are present in the patch.

(Actually, I did not test blackfin, since I don't have one, but I
included it since I know they have a PTP hardware clock.)

> > +static inline int common_clock_adj(const clockid_t which_clock, struct timex *t)
> > +{
> > +	if (CLOCK_REALTIME == which_clock)
> > +		return do_adjtimex(t);
> > +	else
> > +		return -EOPNOTSUPP;
> > +}
> 
> 
> Would it make sense to point to the do_adjtimex() in the k_clock
> definition for CLOCK_REALTIME rather then conditionalizing it here?

But what about CLOCK_MONOTONIC_RAW, for example?

Does it make sense to allow it to be adjusted?

Thanks,
Richard

On Fri, Sep 24, 2010 at 08:03:43AM +1000, Benjamin Herrenschmidt wrote:
> On Thu, 2010-09-23 at 19:31 +0200, Richard Cochran wrote:
> > A new syscall is introduced that allows tuning of a POSIX clock. The
> > syscall is implemented for four architectures: arm, blackfin, powerpc,
> > and x86.
> > 
> > The new syscall, clock_adjtime, takes two parameters, the clock ID,
> > and a pointer to a struct timex. The semantics of the timex struct
> > have been expanded by one additional mode flag, which allows an
> > absolute offset correction. When specificied, the clock offset is
> > immediately corrected by adding the given time value to the current
> > time value.
> 
> Any reason why you CC'ed device-tree discuss ?
> 
> This list is getting way too much unrelated stuff, which I find
> annoying, it would be nice if we were all a bit more careful here with
> our CC lists.

Sorry, I only added device-tree because some one asked me to do so.

    http://marc.info/?l=linux-netdev&m=127273157912358

I'll leave it off next time.

Thanks,
Richard

On Fri, 2010-09-24 at 09:29 +0200, Richard Cochran wrote:
> On Thu, Sep 23, 2010 at 12:48:51PM -0700, john stultz wrote:
> > So I'd still split this patch up a little bit more.
> > 
> > 1) Patch that implements the ADJ_SETOFFSET  (*and its implementation*)
> > in do_adjtimex.
> > 
> > 2) Patch that adds the new syscall and clock_id multiplexing.
> > 
> > 3) Patches that wire it up to the rest of the architectures (there's
> > still a bunch missing here).
> 
> I was not sure what the policy is about adding syscalls. Is it the
> syscall author's responsibility to add it into every arch?
> 
> The last time (see a2e2725541fad7) the commit only added half of some
> archs, and ignored others. In my patch, the syscall *really* works on
> the archs that are present in the patch.
> 
> (Actually, I did not test blackfin, since I don't have one, but I
> included it since I know they have a PTP hardware clock.)

I'm not sure about policy, but I think for completeness sake you should
make sure every arch supports a new syscall. You're not expected to be
able to test every one, but getting the basic support patch sent to
maintainers should be done.

> > > +static inline int common_clock_adj(const clockid_t which_clock, struct timex *t)
> > > +{
> > > +	if (CLOCK_REALTIME == which_clock)
> > > +		return do_adjtimex(t);
> > > +	else
> > > +		return -EOPNOTSUPP;
> > > +}
> > 
> > 
> > Would it make sense to point to the do_adjtimex() in the k_clock
> > definition for CLOCK_REALTIME rather then conditionalizing it here?
> 
> But what about CLOCK_MONOTONIC_RAW, for example?

-EOPNOTSUPP

> Does it make sense to allow it to be adjusted?

No. I think only CLOCK_REALTIME would make sense of the existing clocks.

I'm just suggesting you conditionalize it from the function pointer,
rather then in the common function.

thanks
-john

> > This list is getting way too much unrelated stuff, which I find
> > annoying, it would be nice if we were all a bit more careful here with
> > our CC lists.
> 
> Sorry, I only added device-tree because some one asked me to do so.
> 
>     http://marc.info/?l=linux-netdev&m=127273157912358
> 
> I'll leave it off next time.

That's allright. I'd rather you just post the bindings there than the
whole patch least but no big deal.

I was just fixing my email filters and notice a lot of seemingly
unrelated stuff landing there :-)

Cheers,
Ben.