From patchwork Thu Nov 24 08:22:36 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thomas Gleixner X-Patchwork-Id: 25370 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:f944:0:0:0:0:0 with SMTP id q4csp3265622wrr; Thu, 24 Nov 2022 00:33:54 -0800 (PST) X-Google-Smtp-Source: AA0mqf5LHRvifRLslB/urZ9mE09bHGs9LD0qgaB3qayr0MV+/WYGYsmDJKcwaxOblgovwF/XdTYl X-Received: by 2002:aa7:8f01:0:b0:56d:6450:9e48 with SMTP id x1-20020aa78f01000000b0056d64509e48mr14352301pfr.14.1669278834199; Thu, 24 Nov 2022 00:33:54 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1669278834; cv=none; d=google.com; s=arc-20160816; b=lssBHT8gSE0nBO4R43RGJaHP5739lE0AuX91OGt4ftCjZGbIS9TQzWf7UYkOl7PUlI 0Nmi6GzHJYqpvuYkUnBVcGiFxWheV+CtG9RocGrQ20JHpkee+kTAAkr0eeaL4sgYEmxk kZPAyEAogZvqhkn3yDho/2oAnQBf0TKMfkMrbtWSxDgyHFwWuXvkWp6dUyKitYG6Q95A Q6+6TcNpmfLZ7YhQpgqZTZVMmiBXVZMAxuVR/rPk1egdUVU33JA4uE08Re6GS7EznFqI jh4wCfEe+Tt2N3Eh6ivNshwCsNtMrVc8wnH16Cx74mEeX1wA/BBqPpFU9q1JDUbFxc2S K21w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:mime-version:message-id:date:references :in-reply-to:subject:cc:to:dkim-signature:dkim-signature:from; bh=s+nIC3djla2eCKjxSXpIFMphkhy9KsLIp+C+/UiDTis=; b=Xu8RqSLiRx2LG/jdsyq+kofxdOIE0QGCbuReNDfZaohAzxYvqmqyHn7/6/9K6KKkgI joR9Xu3QpG9SlhfUA+VkMJvYzJSpOAM338xu582xGL3+Q0PdPQXCYZU+6q1Blez1riYd cJUVbffcKVzcjltORktGW5+eIb3rcSoCqFVkr1wLxVFHirq1p+d4MB+iNlm66fvJJ1hK dMglbcLH7774OxCTEHn18WI0QTWU3iJY4T43cw61TePbS+3CzlFWv++fbk9POxZ7STGW 2Vwr2VgxpyvOwD+EX1brT/Z4t9/g47d1IzSFvwsJsnhQig3ZBriqL5lof8DIlWR/MVOu FIcQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linutronix.de header.s=2020 header.b=WnnDkWqe; dkim=neutral (no key) header.i=@linutronix.de; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=linutronix.de Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id mi7-20020a17090b4b4700b0020354bcfe09si673129pjb.129.2022.11.24.00.33.42; Thu, 24 Nov 2022 00:33:54 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@linutronix.de header.s=2020 header.b=WnnDkWqe; dkim=neutral (no key) header.i=@linutronix.de; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=linutronix.de Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229745AbiKXIWn (ORCPT + 99 others); Thu, 24 Nov 2022 03:22:43 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48760 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229695AbiKXIWk (ORCPT ); Thu, 24 Nov 2022 03:22:40 -0500 Received: from galois.linutronix.de (Galois.linutronix.de [193.142.43.55]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C3D26CE25; Thu, 24 Nov 2022 00:22:38 -0800 (PST) From: Thomas Gleixner DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1669278157; 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: in-reply-to:in-reply-to:references:references; bh=s+nIC3djla2eCKjxSXpIFMphkhy9KsLIp+C+/UiDTis=; b=WnnDkWqesDTvoW0XJWpjXbyMRQFAJEH0gsTD+Vx8b1Veh8xDVHTxzRy23zLdxjs62rP2q+ aproFYAw5kBxG/hlnkTjE/1pKUe6sb+MIJIp9SeTtpSQc8QSs8HrlJNYxJ0DartOx91aW6 Hm1e5Std3/+gkfZrVxCE6p+qcOXqmRMEYHIgXk1OCe+32RJnxT3njZOn8pW/TCVZ6W0JU4 EBYgElc67FYlN1woXMHsxmeS7Vq+n7FRNRallduWcfiBM4PckOrDFFJHrSVRFKdWdua9O3 mFYlNUTiMm5ZYNseMfjsylX0PIjnLbgfdXakYNH3Es3GDkNGDLkZzvty7BklwQ== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1669278157; 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: in-reply-to:in-reply-to:references:references; bh=s+nIC3djla2eCKjxSXpIFMphkhy9KsLIp+C+/UiDTis=; b=KLKAKLpPJ0MEuWXdektcZAmtHEhsjdXjfi0MPD+kxV+TF5H5O7xH61chkV/YtffDHzfx6T IrK1S6d26MYxYxAg== To: Anna-Maria Behnsen Cc: LKML , Linus Torvalds , Steven Rostedt , 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, Jacob Keller Subject: [patch V3.1 12/17] timers: Silently ignore timers with a NULL function In-Reply-To: <87zgcgdau1.ffs@tglx> References: <20221123201306.823305113@linutronix.de> <20221123201625.135055320@linutronix.de> <644695b9-f343-7fb7-ed8e-763e5fe3d158@linutronix.de> <87zgcgdau1.ffs@tglx> Date: Thu, 24 Nov 2022 09:22:36 +0100 Message-ID: <87wn7kdann.ffs@tglx> MIME-Version: 1.0 X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,SPF_HELO_NONE, SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1750319728392179595?= X-GMAIL-MSGID: =?utf-8?q?1750365723234704511?= 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 is to set timer->function to NULL and use this as an indicator for the timer arming functions to ignore the (re)arm request. In preparation for that replace the warnings in the relevant code paths with checks for timer->function == NULL. If the pointer is NULL, then discard the rearm request silently. Add debug_assert_init() instead of the WARN_ON_ONCE(!timer->function) checks so that debug objects can warn about non-initialized timers. The warning of debug objects does not warn if timer->function == NULL. It warns when timer was not initialized using timer_setup[_on_stack]() or via DEFINE_TIMER(). If developers fail to enable debug objects and then waste lots of time to figure out why their non-initialized timer is not firing, they deserve it. Same for initializing a timer with a NULL function. Co-developed-by: Steven Rostedt Signed-off-by: Steven Rostedt Signed-off-by: Thomas Gleixner Tested-by: Guenter Roeck Reviewed-by: Jacob Keller Link: https://lore.kernel.org/all/20220407161745.7d6754b3@gandalf.local.home Link: https://lore.kernel.org/all/20221110064101.429013735@goodmis.org --- V2: Use continue instead of return and amend the return value docs (Steven) V3: Changelog and comment updates (Anna-Maria) V3.1: Fix it for real --- kernel/time/timer.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 52 insertions(+), 5 deletions(-) --- a/kernel/time/timer.c +++ b/kernel/time/timer.c @@ -1017,7 +1017,7 @@ static inline int unsigned int idx = UINT_MAX; int ret = 0; - BUG_ON(!timer->function); + debug_assert_init(timer); /* * This is a common optimization triggered by the networking code - if @@ -1044,6 +1044,14 @@ static inline int * dequeue/enqueue dance. */ base = lock_timer_base(timer, &flags); + /* + * Has @timer been shutdown? This needs to be evaluated + * while holding base lock to prevent a race against the + * shutdown code. + */ + if (!timer->function) + goto out_unlock; + forward_timer_base(base); if (timer_pending(timer) && (options & MOD_TIMER_REDUCE) && @@ -1070,6 +1078,14 @@ static inline int } } else { base = lock_timer_base(timer, &flags); + /* + * Has @timer been shutdown? This needs to be evaluated + * while holding base lock to prevent a race against the + * shutdown code. + */ + if (!timer->function) + goto out_unlock; + forward_timer_base(base); } @@ -1128,8 +1144,12 @@ static inline int * mod_timer_pending() is the same for pending timers as mod_timer(), but * will not activate inactive timers. * + * If @timer->function == NULL then the start operation is silently + * discarded. + * * Return: - * * %0 - The timer was inactive and not modified + * * %0 - The timer was inactive and not modified or was in + * shutdown state and the operation was discarded * * %1 - The timer was active and requeued to expire at @expires */ int mod_timer_pending(struct timer_list *timer, unsigned long expires) @@ -1155,8 +1175,12 @@ EXPORT_SYMBOL(mod_timer_pending); * same timer, then mod_timer() is the only safe way to modify the timeout, * since add_timer() cannot modify an already running timer. * + * If @timer->function == NULL then the start operation is silently + * discarded. In this case the return value is 0 and meaningless. + * * Return: - * * %0 - The timer was inactive and started + * * %0 - The timer was inactive and started or was in shutdown + * state and the operation was discarded * * %1 - The timer was active and requeued to expire at @expires or * the timer was active and not modified because @expires did * not change the effective expiry time @@ -1176,8 +1200,12 @@ EXPORT_SYMBOL(mod_timer); * modify an enqueued timer if that would reduce the expiration time. If * @timer is not enqueued it starts the timer. * + * If @timer->function == NULL then the start operation is silently + * discarded. + * * Return: - * * %0 - The timer was inactive and started + * * %0 - The timer was inactive and started or was in shutdown + * state and the operation was discarded * * %1 - The timer was active and requeued to expire at @expires or * the timer was active and not modified because @expires * did not change the effective expiry time such that the @@ -1200,6 +1228,9 @@ EXPORT_SYMBOL(timer_reduce); * The @timer->expires and @timer->function fields must be set prior * to calling this function. * + * If @timer->function == NULL then the start operation is silently + * discarded. + * * If @timer->expires is already in the past @timer will be queued to * expire at the next timer tick. * @@ -1228,7 +1259,9 @@ void add_timer_on(struct timer_list *tim struct timer_base *new_base, *base; unsigned long flags; - if (WARN_ON_ONCE(timer_pending(timer) || !timer->function)) + debug_assert_init(timer); + + if (WARN_ON_ONCE(timer_pending(timer))) return; new_base = get_timer_cpu_base(timer->flags, cpu); @@ -1239,6 +1272,13 @@ void add_timer_on(struct timer_list *tim * wrong base locked. See lock_timer_base(). */ base = lock_timer_base(timer, &flags); + /* + * Has @timer been shutdown? This needs to be evaluated while + * holding base lock to prevent a race against the shutdown code. + */ + if (!timer->function) + goto out_unlock; + if (base != new_base) { timer->flags |= TIMER_MIGRATING; @@ -1252,6 +1292,7 @@ void add_timer_on(struct timer_list *tim debug_timer_activate(timer); internal_add_timer(base, timer); +out_unlock: raw_spin_unlock_irqrestore(&base->lock, flags); } EXPORT_SYMBOL_GPL(add_timer_on); @@ -1541,6 +1582,12 @@ static void expire_timers(struct timer_b fn = timer->function; + if (WARN_ON_ONCE(!fn)) { + /* Should never happen. Emphasis on should! */ + base->running_timer = NULL; + continue; + } + if (timer->flags & TIMER_IRQSAFE) { raw_spin_unlock(&base->lock); call_timer_fn(timer, fn, baseclk);