All of lore.kernel.org
 help / color / mirror / Atom feed
From: Venki Pallipadi <venkatesh.pallipadi@intel.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-kernel <linux-kernel@vger.kernel.org>, shaohua.li@intel.com
Subject: [PATCH] Prevent clockevent event_handler ending up handler_noop
Date: Tue, 2 Sep 2008 16:20:15 -0700	[thread overview]
Message-ID: <20080902232015.GA3180@linux-os.sc.intel.com> (raw)


There is a ordering related problem with clockevents code, due to which
clockevents_register_device() called after tickless/highres switch
will not work. The new clockevent ends up with clockevents_handle_noop as
event handler, resulting in no timer activity.

The problematic path seems to be

* old device already has hrtimer_interrupt as the event_handler
* new clockevent device registers with a higher rating
* tick_check_new_device() is called
  * clockevents_exchange_device() gets called
    * old->event_handler is set to clockevents_handle_noop
  * tick_setup_device() is called for the new device
    * which sets new->event_handler using the old->event_handler which is noop.

Change the ordering so that new device inherits the proper handler.

This does not have any issue in normal case as most likely all the clockevent
devices are setup before the highres switch. But, can potentially be affecting
some corner case where HPET force detect happens after the highres switch.
This was a problem with HPET in MSI mode code that we have been experimenting
with.

Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
Signed-off-by: Shaohua Li <shaohua.li@intel.com>

---
 include/linux/clockchips.h |    2 ++
 kernel/time/clockevents.c  |    3 +--
 kernel/time/tick-common.c  |    1 +
 3 files changed, 4 insertions(+), 2 deletions(-)

Index: tip/kernel/time/clockevents.c
===================================================================
--- tip.orig/kernel/time/clockevents.c	2008-09-02 13:26:35.000000000 -0700
+++ tip/kernel/time/clockevents.c	2008-09-02 15:24:13.000000000 -0700
@@ -177,7 +177,7 @@ void clockevents_register_device(struct 
 /*
  * Noop handler when we shut down an event device
  */
-static void clockevents_handle_noop(struct clock_event_device *dev)
+void clockevents_handle_noop(struct clock_event_device *dev)
 {
 }
 
@@ -199,7 +199,6 @@ void clockevents_exchange_device(struct 
 	 * released list and do a notify add later.
 	 */
 	if (old) {
-		old->event_handler = clockevents_handle_noop;
 		clockevents_set_mode(old, CLOCK_EVT_MODE_UNUSED);
 		list_del(&old->list);
 		list_add(&old->list, &clockevents_released);
Index: tip/kernel/time/tick-common.c
===================================================================
--- tip.orig/kernel/time/tick-common.c	2008-09-02 13:26:35.000000000 -0700
+++ tip/kernel/time/tick-common.c	2008-09-02 15:24:13.000000000 -0700
@@ -161,6 +161,7 @@ static void tick_setup_device(struct tic
 	} else {
 		handler = td->evtdev->event_handler;
 		next_event = td->evtdev->next_event;
+		td->evtdev->event_handler = clockevents_handle_noop;
 	}
 
 	td->evtdev = newdev;
Index: tip/include/linux/clockchips.h
===================================================================
--- tip.orig/include/linux/clockchips.h	2008-09-02 13:26:34.000000000 -0700
+++ tip/include/linux/clockchips.h	2008-09-02 15:24:13.000000000 -0700
@@ -127,6 +127,8 @@ extern int clockevents_register_notifier
 extern int clockevents_program_event(struct clock_event_device *dev,
 				     ktime_t expires, ktime_t now);
 
+extern void clockevents_handle_noop(struct clock_event_device *dev);
+
 #ifdef CONFIG_GENERIC_CLOCKEVENTS
 extern void clockevents_notify(unsigned long reason, void *arg);
 #else

             reply	other threads:[~2008-09-02 23:20 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-09-02 23:20 Venki Pallipadi [this message]
2008-09-02 23:31 ` [PATCH] Prevent clockevent event_handler ending up handler_noop Thomas Gleixner
2008-09-03 16:43 ` Andreas Mohr
2008-09-03 17:15   ` Thomas Gleixner
2008-09-03 17:36     ` Andreas Mohr
2008-09-03 19:01       ` Thomas Gleixner
2008-09-04  8:15         ` Andreas Mohr
2008-09-04  8:27           ` Thomas Gleixner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080902232015.GA3180@linux-os.sc.intel.com \
    --to=venkatesh.pallipadi@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=shaohua.li@intel.com \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.