Re: [tip:timers/core] hrtimers: Move SMP function call to thread context

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



tip-bot for Thomas Gleixner <tipbot@xxxxxxxxx> wrote:

>Commit-ID:  5ec2481b7b47a4005bb446d176e5d0257400c77d
>Gitweb:    
>http://git.kernel.org/tip/5ec2481b7b47a4005bb446d176e5d0257400c77d
>Author:     Thomas Gleixner <tglx@xxxxxxxxxxxxx>
>AuthorDate: Fri, 5 Jul 2013 12:09:18 +0200
>Committer:  Thomas Gleixner <tglx@xxxxxxxxxxxxx>
>CommitDate: Fri, 5 Jul 2013 17:25:58 +0200
>
>hrtimers: Move SMP function call to thread context
>
>smp_call_function_* must not be called from softirq context.
>
>But clock_was_set() which calls on_each_cpu() is called from softirq
>context to implement a delayed clock_was_set() for the timer interrupt
>handler. Though that almost never gets invoked. A recent change in the
>resume code uses the softirq based delayed clock_was_set to support
>Xens resume mechanism.
>
>linux-next contains a new warning which warns if smp_call_function_*
>is called from softirq context which gets triggered by that Xen
>change.
>
>Fix this by moving the delayed clock_was_set() call to a work context.
>
>Reported-and-tested-by: Artem Savkov <artem.savkov@xxxxxxxxx>
>Reported-by: Sasha Levin <sasha.levin@xxxxxxxxxx>
>Cc: David Vrabel <david.vrabel@xxxxxxxxxx>
>Cc: Ingo Molnar <mingo@xxxxxxxxxx>
>Cc: H. Peter Anvin <hpa@xxxxxxxxx>,
>Cc: Konrad Wilk <konrad.wilk@xxxxxxxxxx>
>Cc: John Stultz <john.stultz@xxxxxxxxxx>
>Cc: xen-devel@xxxxxxxxxxxxx
>Cc: stable@xxxxxxxxxxxxxxx
>Signed-off-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
>---
> kernel/hrtimer.c | 37 +++++++++++++++----------------------
> 1 file changed, 15 insertions(+), 22 deletions(-)
>
>diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
>index e86827e..b9b9420 100644
>--- a/kernel/hrtimer.c
>+++ b/kernel/hrtimer.c
>@@ -721,17 +721,20 @@ static int hrtimer_switch_to_hres(void)
> 	return 1;
> }
> 
>+static void clock_was_set_work(struct work_struct *work)
>+{
>+	clock_was_set();
>+}
>+
>+static DECLARE_WORK(hrtimer_work, clock_was_set_work);
>+
> /*
>- * Called from timekeeping code to reprogramm the hrtimer interrupt
>- * device. If called from the timer interrupt context we defer it to
>- * softirq context.
>+ * Called from timekeeping and resume code to reprogramm the hrtimer
>+ * interrupt device on all cpus.
>  */
> void clock_was_set_delayed(void)
> {
>-	struct hrtimer_cpu_base *cpu_base = &__get_cpu_var(hrtimer_bases);
>-
>-	cpu_base->clock_was_set = 1;
>-	__raise_softirq_irqoff(HRTIMER_SOFTIRQ);
>+	schedule_work(&hrtimer_work);
> }
> 
> #else
>@@ -775,12 +778,7 @@ void clock_was_set(void)
>  * During resume we might have to reprogram the high resolution timer
>  * interrupt on all online CPUs.  However, all other CPUs will be
>  * stopped with IRQs interrupts disabled so the clock_was_set() call
>- * must be deferred to the softirq.
>- *
>- * The one-shot timer has already been programmed to fire immediately
>- * (see tick_resume_oneshot()) and this interrupt will trigger the
>- * softirq to run early enough to correctly reprogram the timers on
>- * all CPUs.
>+ * must be deferred.
>  */
> void hrtimers_resume(void)
> {
>@@ -789,8 +787,10 @@ void hrtimers_resume(void)
> 	WARN_ONCE(!irqs_disabled(),
> 		  KERN_INFO "hrtimers_resume() called with IRQs enabled!");
> 
>-	cpu_base->clock_was_set = 1;
>-	__raise_softirq_irqoff(HRTIMER_SOFTIRQ);
>+	/* Retrigger on the local CPU */
>+	retrigger_next_event(NULL);
>+	/* And schedule a retrigger for all others */
>+	clock_was_set_delayed();
> }
> 
>static inline void timer_stats_hrtimer_set_start_info(struct hrtimer
>*timer)
>@@ -1441,13 +1441,6 @@ void hrtimer_peek_ahead_timers(void)
> 
> static void run_hrtimer_softirq(struct softirq_action *h)
> {
>-	struct hrtimer_cpu_base *cpu_base = &__get_cpu_var(hrtimer_bases);
>-
>-	if (cpu_base->clock_was_set) {
>-		cpu_base->clock_was_set = 0;
>-		clock_was_set();
>-	}
>-
> 	hrtimer_peek_ahead_timers();
> }
> 

Would it make sense to mention it the description the git commits (and their titles)  of the patches that caused this.  Saying 'by that Xen change'  is a bit vague.  I am surely  not going to remember this in two months and if we hit other related bugs it would be helpful to have the ids right in the commit to backtrack the development? 
-- 
Sent from my Android phone. Please excuse my brevity.
--
To unsubscribe from this list: send the line "unsubscribe linux-tip-commits" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Stable Commits]     [Linux Stable Kernel]     [Linux Kernel]     [Linux USB Devel]     [Linux Video &Media]     [Linux Audio Users]     [Yosemite News]     [Linux SCSI]

  Powered by Linux