2011-02-01 13:52:25

by Thomas Gleixner

Subject: [patch 18/28] posix-timers: Convert timer_delete() to clockid_to_kclock()

Set the common function for CLOCK_MONOTONIC and CLOCK_REALTIME kclocks
and use the new decoding function. No need to check for the return
value of it. If we have data corruption in the timer, we explode
somewhere else anyway. Also all kclocks which implement timer_create()
need to provide timer_delete() as well.

Signed-off-by: Thomas Gleixner <[email protected]>
Cc: John Stultz <[email protected]>
Cc: Richard Cochran <[email protected]>
---
kernel/posix-timers.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

Index: linux-2.6-tip/kernel/posix-timers.c
===================================================================
--- linux-2.6-tip.orig/kernel/posix-timers.c
+++ linux-2.6-tip/kernel/posix-timers.c
@@ -254,6 +254,7 @@ static __init int init_posix_timers(void
.timer_create = common_timer_create,
.timer_set = common_timer_set,
.timer_get = common_timer_get,
+ .timer_del = common_timer_del,
};
struct k_clock clock_monotonic = {
.clock_getres = hrtimer_get_res,
@@ -263,6 +264,7 @@ static __init int init_posix_timers(void
.timer_create = common_timer_create,
.timer_set = common_timer_set,
.timer_get = common_timer_get,
+ .timer_del = common_timer_del,
};
struct k_clock clock_monotonic_raw = {
.clock_getres = hrtimer_get_res,
@@ -852,7 +854,9 @@ static inline int common_timer_del(struc

static inline int timer_delete_hook(struct k_itimer *timer)
{
- return CLOCK_DISPATCH(timer->it_clock, timer_del, (timer));
+ struct k_clock *kc = clockid_to_kclock(timer->it_clock);
+
+ return kc->timer_del(timer);
}

/* Delete a POSIX.1b interval timer. */

2011-02-01 21:24:25

by john stultz

Subject: Re: [patch 18/28] posix-timers: Convert timer_delete() to clockid_to_kclock()

On Tue, 2011-02-01 at 13:52 +0000, Thomas Gleixner wrote:
> plain text document attachment (posix-timers-convert-timer-del.patch)
> Set the common function for CLOCK_MONOTONIC and CLOCK_REALTIME kclocks
> and use the new decoding function. No need to check for the return
> value of it. If we have data corruption in the timer, we explode
> somewhere else anyway. Also all kclocks which implement timer_create()
> need to provide timer_delete() as well.
>
> Signed-off-by: Thomas Gleixner <[email protected]>
> Cc: John Stultz <[email protected]>
> Cc: Richard Cochran <[email protected]>
> ---
[snip]
> @@ -852,7 +854,9 @@ static inline int common_timer_del(struc
>
> static inline int timer_delete_hook(struct k_itimer *timer)
> {
> - return CLOCK_DISPATCH(timer->it_clock, timer_del, (timer));
> + struct k_clock *kc = clockid_to_kclock(timer->it_clock);
> +
> + return kc->timer_del(timer);
> }

Again, kc == NULL check.

thanks
-john

2011-02-02 08:29:53

by Richard Cochran

Subject: Re: [patch 18/28] posix-timers: Convert timer_delete() to clockid_to_kclock()

On Tue, Feb 01, 2011 at 01:52:07PM -0000, Thomas Gleixner wrote:
> Index: linux-2.6-tip/kernel/posix-timers.c
> ===================================================================
> --- linux-2.6-tip.orig/kernel/posix-timers.c
> +++ linux-2.6-tip/kernel/posix-timers.c
> @@ -254,6 +254,7 @@ static __init int init_posix_timers(void
> .timer_create = common_timer_create,
> .timer_set = common_timer_set,
> .timer_get = common_timer_get,
> + .timer_del = common_timer_del,
> };
> struct k_clock clock_monotonic = {
> .clock_getres = hrtimer_get_res,
> @@ -263,6 +264,7 @@ static __init int init_posix_timers(void
> .timer_create = common_timer_create,
> .timer_set = common_timer_set,
> .timer_get = common_timer_get,
> + .timer_del = common_timer_del,
> };
> struct k_clock clock_monotonic_raw = {
> .clock_getres = hrtimer_get_res,

The definition of common_timer_del() should not be "inline" since the
address of the function is in these tables. The declaration is not
inline, and gcc 4.2.4 gives a warning.

(But strangly, newer versions do not).

Thanks,
Richard

2011-02-02 09:54:13

by Thomas Gleixner

Subject: Re: [patch 18/28] posix-timers: Convert timer_delete() to clockid_to_kclock()

On Wed, 2 Feb 2011, Richard Cochran wrote:

> On Tue, Feb 01, 2011 at 01:52:07PM -0000, Thomas Gleixner wrote:
> > Index: linux-2.6-tip/kernel/posix-timers.c
> > ===================================================================
> > --- linux-2.6-tip.orig/kernel/posix-timers.c
> > +++ linux-2.6-tip/kernel/posix-timers.c
> > @@ -254,6 +254,7 @@ static __init int init_posix_timers(void
> > .timer_create = common_timer_create,
> > .timer_set = common_timer_set,
> > .timer_get = common_timer_get,
> > + .timer_del = common_timer_del,
> > };
> > struct k_clock clock_monotonic = {
> > .clock_getres = hrtimer_get_res,
> > @@ -263,6 +264,7 @@ static __init int init_posix_timers(void
> > .timer_create = common_timer_create,
> > .timer_set = common_timer_set,
> > .timer_get = common_timer_get,
> > + .timer_del = common_timer_del,
> > };
> > struct k_clock clock_monotonic_raw = {
> > .clock_getres = hrtimer_get_res,
>
> The definition of common_timer_del() should not be "inline" since the
> address of the function is in these tables. The declaration is not
> inline, and gcc 4.2.4 gives a warning.
>
> (But strangly, newer versions do not).

Will fix. Thanks.

tglx

2011-02-02 22:02:30

by Thomas Gleixner

Subject: [tip:timers/core] posix-timers: Convert timer_delete() to clockid_to_kclock()

Commit-ID: 6761c6702e2c647582e1829abe8cf90794f61d9d
Gitweb: http://git.kernel.org/tip/6761c6702e2c647582e1829abe8cf90794f61d9d
Author: Thomas Gleixner <[email protected]>
AuthorDate: Tue, 1 Feb 2011 13:52:07 +0000
Committer: Thomas Gleixner <[email protected]>
CommitDate: Wed, 2 Feb 2011 15:28:17 +0100

posix-timers: Convert timer_delete() to clockid_to_kclock()

Set the common function for CLOCK_MONOTONIC and CLOCK_REALTIME kclocks
and use the new decoding function.

Signed-off-by: Thomas Gleixner <[email protected]>
Acked-by: John Stultz <[email protected]>
Tested-by: Richard Cochran <[email protected]>
LKML-Reference: <[email protected]>
---
kernel/posix-timers.c | 10 ++++++++--
1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/kernel/posix-timers.c b/kernel/posix-timers.c
index c1e2636..ade7dec 100644
--- a/kernel/posix-timers.c
+++ b/kernel/posix-timers.c
@@ -254,6 +254,7 @@ static __init int init_posix_timers(void)
.timer_create = common_timer_create,
.timer_set = common_timer_set,
.timer_get = common_timer_get,
+ .timer_del = common_timer_del,
};
struct k_clock clock_monotonic = {
.clock_getres = hrtimer_get_res,
@@ -263,6 +264,7 @@ static __init int init_posix_timers(void)
.timer_create = common_timer_create,
.timer_set = common_timer_set,
.timer_get = common_timer_get,
+ .timer_del = common_timer_del,
};
struct k_clock clock_monotonic_raw = {
.clock_getres = hrtimer_get_res,
@@ -859,7 +861,7 @@ retry:
return error;
}

-static inline int common_timer_del(struct k_itimer *timer)
+static int common_timer_del(struct k_itimer *timer)
{
timer->it.real.interval.tv64 = 0;

@@ -870,7 +872,11 @@ static inline int common_timer_del(struct k_itimer *timer)

static inline int timer_delete_hook(struct k_itimer *timer)
{
- return CLOCK_DISPATCH(timer->it_clock, timer_del, (timer));
+ struct k_clock *kc = clockid_to_kclock(timer->it_clock);
+
+ if (WARN_ON_ONCE(!kc || !kc->timer_del))
+ return -EINVAL;
+ return kc->timer_del(timer);
}

/* Delete a POSIX.1b interval timer. */