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

2010-09-24 Thread Richard Cochran
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
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


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

2010-09-24 Thread Richard Cochran
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-netdevm=127273157912358

I'll leave it off next time.

Thanks,
Richard
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


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

2010-09-24 Thread john stultz
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


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


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

2010-09-24 Thread Benjamin Herrenschmidt

  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-netdevm=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.

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


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

2010-09-23 Thread Richard Cochran
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.coch...@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 

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

2010-09-23 Thread john stultz
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

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


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

2010-09-23 Thread Benjamin Herrenschmidt
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.coch...@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_syscalls326
 +#define __NR_syscalls327
  
  #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

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

2010-09-23 Thread Thomas Gleixner
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
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


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

2010-09-23 Thread Benjamin Herrenschmidt
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.


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev