Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756167Ab0BBOUL (ORCPT ); Tue, 2 Feb 2010 09:20:11 -0500 Received: from casper.infradead.org ([85.118.1.10]:60570 "EHLO casper.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755792Ab0BBOUI (ORCPT ); Tue, 2 Feb 2010 09:20:08 -0500 Subject: [PATCH] hrtimer, softirq: Fix hrtimer->softirq trampoline From: Peter Zijlstra To: Yury Polyanskiy Cc: Herbert Xu , Wei Yongjun , "netdev@vger.kernel.org" , "David S. Miller" , polyanskiy@gmail.com, Thomas Gleixner , lkml In-Reply-To: <20100202085117.7a5c3530@penta.localdomain> References: <4B66A670.70503@cn.fujitsu.com> <20100202074914.GD11081@gondor.apana.org.au> <20100202085117.7a5c3530@penta.localdomain> Content-Type: text/plain; charset="UTF-8" Date: Tue, 02 Feb 2010 15:20:01 +0100 Message-ID: <1265120401.24455.306.camel@laptop> Mime-Version: 1.0 X-Mailer: Evolution 2.28.1 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3661 Lines: 102 On Tue, 2010-02-02 at 08:51 -0500, Yury Polyanskiy wrote: > If hrtimer_tasklet interface functions properly, the > xfrm_timer_handler should be called in softirq context (and thus is > never in parallel with xfrm_input()). The deadlock isn't possible then. > > In this case it seems that for some reason xfrm_timer_handler() is > called in the hardirq context. The relevant code in hrtimer_tasklet: > > static enum hrtimer_restart __hrtimer_tasklet_trampoline(struct hrtimer *timer) > { > struct tasklet_hrtimer *ttimer = > container_of(timer, struct tasklet_hrtimer, timer); > > if (hrtimer_is_hres_active(timer)) { > tasklet_hi_schedule(&ttimer->tasklet); > return HRTIMER_NORESTART; > } > return ttimer->function(timer); > } > > I am copying Peter on this. Peter, how is it possible that > ttimer->function() is called in hardirq? > > Could it be that switch from hres_active happened after the call to > trampoline and before the if() above? The original email had more information: > {IN-HARDIRQ-W} state was registered at: > [] __lock_acquire+0xa9c/0x1890 > [] lock_acquire+0x7f/0xf0 > [] _raw_spin_lock+0x38/0x50 > [] xfrm_timer_handler+0x3a/0x260 > [] __hrtimer_tasklet_trampoline+0xd/0x10 > [] hrtimer_run_queues+0x15e/0x2a0 > [] run_local_timers+0xd/0x20 > [] update_process_times+0x34/0x70 > [] tick_periodic+0x2a/0x80 > [] tick_handle_periodic+0x1e/0x90 > [] smp_apic_timer_interrupt+0x57/0x8b > [] apic_timer_interrupt+0x2f/0x34 > [] cpu_idle+0x4b/0x80 > [] rest_init+0x67/0x70 > [] start_kernel+0x30e/0x314 > [] i386_start_kernel+0x9e/0xa5 Which indicates we were called from hardirq context, it appears that that hrtimer_is_hres_active() case is indeed faulty. Not sure if I made a mistake when I wrote that or if we changed hrtimer behaviour afterwards, but the hrtimer fallback is still from hardirq context. Which would seem to suggest the following patch: --- Subject: hrtimer, softirq: Fix hrtimer->softirq trampoline hrtimers callbacks are always done from hardirq context, either the jiffy tick interrupt or the hrtimer device interrupt. Signed-off-by: Peter Zijlstra --- kernel/softirq.c | 13 +++---------- 1 files changed, 3 insertions(+), 10 deletions(-) diff --git a/kernel/softirq.c b/kernel/softirq.c index a09502e..c1983b7 100644 --- a/kernel/softirq.c +++ b/kernel/softirq.c @@ -500,22 +500,15 @@ EXPORT_SYMBOL(tasklet_kill); */ /* - * The trampoline is called when the hrtimer expires. If this is - * called from the hrtimer interrupt then we schedule the tasklet as - * the timer callback function expects to run in softirq context. If - * it's called in softirq context anyway (i.e. high resolution timers - * disabled) then the hrtimer callback is called right away. + * The trampoline is called when the hrtimer expires. */ static enum hrtimer_restart __hrtimer_tasklet_trampoline(struct hrtimer *timer) { struct tasklet_hrtimer *ttimer = container_of(timer, struct tasklet_hrtimer, timer); - if (hrtimer_is_hres_active(timer)) { - tasklet_hi_schedule(&ttimer->tasklet); - return HRTIMER_NORESTART; - } - return ttimer->function(timer); + tasklet_hi_schedule(&ttimer->tasklet); + return HRTIMER_NORESTART; } /* -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/