From patchwork Tue Nov 15 20:28:51 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thomas Gleixner X-Patchwork-Id: 624930 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id B739FC433FE for ; Tue, 15 Nov 2022 20:30:36 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232489AbiKOUae (ORCPT ); Tue, 15 Nov 2022 15:30:34 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38496 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232516AbiKOU3w (ORCPT ); Tue, 15 Nov 2022 15:29:52 -0500 Received: from galois.linutronix.de (Galois.linutronix.de [IPv6:2a0a:51c0:0:12e:550::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 576232E688; Tue, 15 Nov 2022 12:28:53 -0800 (PST) Message-ID: <20221115202117.618987487@linutronix.de> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1668544132; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: references:references; bh=0ZlvY9ool6Ve+kUwOJkRD108DUOqW8Zib/pBYB0+vgk=; b=xFaX+k2VM8PXVE37HnWdfEYAyM5Zri33jJRXYDZ6OQ/qJKRiDh+mRrWz26q5mXkkCQy0C9 OgyufN/x9BczOdWYGaRyr/kyNbXdANSnerf/n21I2C6DAQXabjD23YNkVijJWOXj3sV4k1 bGfh8aJaNobtgpNOkcoROzI8uIPRSAtkjKY4lUOzTsqrQP2MHukX63mV7KsMPgPYZgibwn Lag0cjdUdjsYUpcsnUit0fFu6YSH5GAKFWb8PT6R0yXSq+++FwPbRMwk2eRFrMi4+5zzg8 VN+GKeddbDuUFUxtENU1B/V4Hb4QDCCTJsb36PKC1Qb4vnERxpf7xpg1PYZsSQ== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1668544132; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: references:references; bh=0ZlvY9ool6Ve+kUwOJkRD108DUOqW8Zib/pBYB0+vgk=; b=NJgs5/ebD7dQ0piSM2TFYFDuzA+DPBpImSZ+DQaXcZW/iHBwo6EdCY5vbnTTcULFYo+z2Y njB7NocJThPzE1BQ== From: Thomas Gleixner To: LKML Cc: Linus Torvalds , Steven Rostedt , Anna-Maria Behnsen , Peter Zijlstra , Stephen Boyd , Guenter Roeck , Andrew Morton , Julia Lawall , Arnd Bergmann , Viresh Kumar , Marc Zyngier , Marcel Holtmann , Johan Hedberg , Luiz Augusto von Dentz , linux-bluetooth@vger.kernel.org, "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , netdev@vger.kernel.org Subject: [patch 11/15] timers: Split [try_to_]del_timer[_sync]() to prepare for shutdown mode References: <20221115195802.415956561@linutronix.de> MIME-Version: 1.0 Date: Tue, 15 Nov 2022 21:28:51 +0100 (CET) Precedence: bulk List-ID: X-Mailing-List: linux-bluetooth@vger.kernel.org Tearing down timers which have circular dependencies to other functionality, e.g. workqueues, where the timer can schedule work and work can arm timers is not trivial. In those cases it is desired to shutdown the timer in a way which prevents rearming of the timer. The mechanism to do so it to set timer->function to NULL and use this as an indicator for the timer arming functions to ignore the (re)arm request. Split the inner workings of try_do_del_timer_sync(), del_timer_sync() and del_timer() into helper functions to prepare for implementing the shutdown functionality. No functional change. Co-developed-by: Steven Rostedt Signed-off-by: Steven Rostedt Signed-off-by: Thomas Gleixner Link: https://lore.kernel.org/all/20220407161745.7d6754b3@gandalf.local.home Link: https://lore.kernel.org/all/20221110064101.429013735@goodmis.org --- kernel/time/timer.c | 136 ++++++++++++++++++++++++++++++++++------------------ 1 file changed, 89 insertions(+), 47 deletions(-) --- a/kernel/time/timer.c +++ b/kernel/time/timer.c @@ -1293,19 +1293,14 @@ void add_timer_on(struct timer_list *tim EXPORT_SYMBOL_GPL(add_timer_on); /** - * timer_delete - Deactivate a timer. + * __timer_delete - Internal function: Deactivate a timer. * @timer: The timer to be deactivated * - * Contrary to timer_delete_sync() this function does not wait for an - * eventually running timer callback on a different CPU and it neither - * prevents rearming of the timer. If @timer can be rearmed concurrently - * then the return value of this function is meaningless. - * * Return: * * %0 - The timer was not pending * * %1 - The timer was pending and deactivated */ -int timer_delete(struct timer_list *timer) +static int __timer_delete(struct timer_list *timer) { struct timer_base *base; unsigned long flags; @@ -1321,22 +1316,36 @@ int timer_delete(struct timer_list *time return ret; } + +/** + * timer_delete - Deactivate a timer. + * @timer: The timer to be deactivated + * + * Contrary to timer_delete_sync() this function does not wait for an + * eventually running timer callback on a different CPU and it neither + * prevents rearming of the timer. If @timer can be rearmed concurrently + * then the return value of this function is meaningless. + * + * Return: + * * %0 - The timer was not pending + * * %1 - The timer was pending and deactivated + */ +int timer_delete(struct timer_list *timer) +{ + return __timer_delete(timer); +} EXPORT_SYMBOL(timer_delete); /** - * try_to_del_timer_sync - Try to deactivate a timer + * __try_to_del_timer_sync - Internal function: Try to deactivate a timer * @timer: Timer to deactivate * - * This function cannot guarantee that the timer cannot be rearmed right - * after dropping the base lock. That needs to be prevented by the calling - * code if necessary. - * * Return: * * %0 - The timer was not pending * * %1 - The timer was pending and deactivated * * %-1 - The timer callback function is running on a different CPU */ -int try_to_del_timer_sync(struct timer_list *timer) +static int __try_to_del_timer_sync(struct timer_list *timer) { struct timer_base *base; unsigned long flags; @@ -1353,6 +1362,25 @@ int try_to_del_timer_sync(struct timer_l return ret; } + +/** + * try_to_del_timer_sync - Try to deactivate a timer + * @timer: Timer to deactivate + * + * This function cannot guarantee that the timer cannot be rearmed + * right after dropping the base lock. That needs to be prevented + * by the calling code if necessary. If the timer can be rearmed + * concurrently then the return value is meaningless. + * + * Return: + * * %0 - The timer was not pending + * * %1 - The timer was pending and deactivated + * * %-1 - The timer callback function is running on a different CPU + */ +int try_to_del_timer_sync(struct timer_list *timer) +{ + return __try_to_del_timer_sync(timer); +} EXPORT_SYMBOL(try_to_del_timer_sync); #ifdef CONFIG_PREEMPT_RT @@ -1429,45 +1457,15 @@ static inline void del_timer_wait_runnin #endif /** - * timer_delete_sync - Deactivate a timer and wait for the handler to finish. + * __timer_delete_sync - Internal function: Deactivate a timer and wait + * for the handler to finish. * @timer: The timer to be deactivated * - * Synchronization rules: Callers must prevent restarting of the timer, - * otherwise this function is meaningless. It must not be called from - * interrupt contexts unless the timer is an irqsafe one. The caller must - * not hold locks which would prevent completion of the timer's callback - * function. The timer's handler must not call add_timer_on(). Upon exit - * the timer is not queued and the handler is not running on any CPU. - * - * For !irqsafe timers, the caller must not hold locks that are held in - * interrupt context. Even if the lock has nothing to do with the timer in - * question. Here's why:: - * - * CPU0 CPU1 - * ---- ---- - * - * call_timer_fn(); - * base->running_timer = mytimer; - * spin_lock_irq(somelock); - * - * spin_lock(somelock); - * timer_delete_sync(mytimer); - * while (base->running_timer == mytimer); - * - * Now timer_delete_sync() will never return and never release somelock. - * The interrupt on the other CPU is waiting to grab somelock but it has - * interrupted the softirq that CPU0 is waiting to finish. - * - * This function cannot guarantee that the timer is not rearmed again by - * some concurrent or preempting code, right after it dropped the base - * lock. If there is the possibility of a concurrent rearm then the return - * value of the function is meaningless. - * * Return: * * %0 - The timer was not pending * * %1 - The timer was pending and deactivated */ -int timer_delete_sync(struct timer_list *timer) +static int __timer_delete_sync(struct timer_list *timer) { int ret; @@ -1497,7 +1495,7 @@ int timer_delete_sync(struct timer_list lockdep_assert_preemption_enabled(); do { - ret = try_to_del_timer_sync(timer); + ret = __try_to_del_timer_sync(timer); if (unlikely(ret < 0)) { del_timer_wait_running(timer); @@ -1507,6 +1505,50 @@ int timer_delete_sync(struct timer_list return ret; } + +/** + * timer_delete_sync - Deactivate a timer and wait for the handler to finish. + * @timer: The timer to be deactivated + * + * Synchronization rules: Callers must prevent restarting of the timer, + * otherwise this function is meaningless. It must not be called from + * interrupt contexts unless the timer is an irqsafe one. The caller must + * not hold locks which would prevent completion of the timer's callback + * function. The timer's handler must not call add_timer_on(). Upon exit + * the timer is not queued and the handler is not running on any CPU. + * + * For !irqsafe timers, the caller must not hold locks that are held in + * interrupt context. Even if the lock has nothing to do with the timer in + * question. Here's why:: + * + * CPU0 CPU1 + * ---- ---- + * + * call_timer_fn(); + * base->running_timer = mytimer; + * spin_lock_irq(somelock); + * + * spin_lock(somelock); + * timer_delete_sync(mytimer); + * while (base->running_timer == mytimer); + * + * Now timer_delete_sync() will never return and never release somelock. + * The interrupt on the other CPU is waiting to grab somelock but it has + * interrupted the softirq that CPU0 is waiting to finish. + * + * This function cannot guarantee that the timer is not rearmed again by + * some concurrent or preempting code, right after it dropped the base + * lock. If there is the possibility of a concurrent rearm then the return + * value of the function is meaningless. + * + * Return: + * * %0 - The timer was not pending + * * %1 - The timer was pending and deactivated + */ +int timer_delete_sync(struct timer_list *timer) +{ + return __timer_delete_sync(timer); +} EXPORT_SYMBOL(timer_delete_sync); static void call_timer_fn(struct timer_list *timer,