All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: LKML <linux-kernel@vger.kernel.org>
Cc: Linus Torvalds <torvalds@linuxfoundation.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Anna-Maria Behnsen <anna-maria@linutronix.de>,
	Peter Zijlstra <peterz@infradead.org>,
	Stephen Boyd <sboyd@kernel.org>,
	Guenter Roeck <linux@roeck-us.net>,
	Andrew Morton <akpm@linux-foundation.org>,
	Julia Lawall <Julia.Lawall@inria.fr>,
	Arnd Bergmann <arnd@arndb.de>,
	Viresh Kumar <viresh.kumar@linaro.org>,
	Marc Zyngier <maz@kernel.org>,
	Marcel Holtmann <marcel@holtmann.org>,
	Johan Hedberg <johan.hedberg@gmail.com>,
	Luiz Augusto von Dentz <luiz.dentz@gmail.com>,
	linux-bluetooth@vger.kernel.org,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	netdev@vger.kernel.org
Subject: [patch 00/15] timers: Provide timer_shutdown[_sync]()
Date: Tue, 15 Nov 2022 21:28:32 +0100 (CET)	[thread overview]
Message-ID: <20221115195802.415956561@linutronix.de> (raw)

Tearing down timers can be tedious when there are circular dependencies to
other things which need to be torn down. A prime example is timer and
workqueue where the timer schedules work and the work arms the timer.

Steven and the Google Chromebook team ran into such an issue in the
Bluetooth HCI code.

Steven suggested to create a new function del_timer_free() which marks the
timer as shutdown. Rearm attempts of shutdown timers are discarded and he
wanted to emit a warning for that case:

   https://lore.kernel.org/all/20220407161745.7d6754b3@gandalf.local.home

This resulted in a lengthy discussion and suggestions how this should be
implemented. The patch series went through several iterations and during
the review of the last version it turned out that this approach is
suboptimal:

   https://lore.kernel.org/all/20221110064101.429013735@goodmis.org

The warning is not really helpful because it's entirely unclear how it
should be acted upon. The only way to address such a case is to add 'if
(in_shutdown)' conditionals all over the place. This is error prone and in
most cases of teardown like the HCI one which started this discussion not
required all.

What needs to prevented is that pending work which is drained via
destroy_workqueue() does not rearm the previously shutdown timer. Nothing
in that shutdown sequence relies on the timer being functional.

The conclusion was that the semantics of timer_shutdown_sync() should be:

    - timer is not enqueued
    - timer callback is not running
    - timer cannot be rearmed

Preventing the rearming of shutdown timers is done by discarding rearm
attempts silently.

As Steven is short of cycles, I made some spare cycles available and
reworked the patch series to follow the new semantics and plugged the races
which were discovered during review.

The patches have been split up into small pieces to make review easier and
I took the liberty to throw a bunch of overdue cleanups into the picture
instead of proliferating the existing state further.

The last patch in the series addresses the HCI teardown issue for real.

The series is also available from git:

   git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git timers

Thanks,

	tglx
---
 Documentation/RCU/Design/Requirements/Requirements.rst |    2 
 Documentation/core-api/local_ops.rst                   |    2 
 Documentation/kernel-hacking/locking.rst               |   13 
 arch/arm/mach-spear/time.c                             |    8 
 drivers/bluetooth/hci_qca.c                            |   10 
 drivers/char/tpm/tpm-dev-common.c                      |    4 
 drivers/clocksource/arm_arch_timer.c                   |   12 
 drivers/clocksource/timer-sp804.c                      |    6 
 drivers/staging/wlan-ng/hfa384x_usb.c                  |    4 
 drivers/staging/wlan-ng/prism2usb.c                    |    6 
 include/linux/timer.h                                  |   35 +
 kernel/time/timer.c                                    |  409 +++++++++++++----
 net/sunrpc/xprt.c                                      |    2 
 13 files changed, 383 insertions(+), 130 deletions(-)

             reply	other threads:[~2022-11-15 20:28 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-15 20:28 Thomas Gleixner [this message]
2022-11-15 20:28 ` [patch 01/15] ARM: spear: Do not use timer namespace for timer_shutdown() function Thomas Gleixner
2022-11-15 21:09   ` timers: Provide timer_shutdown[_sync]() bluez.test.bot
2022-11-18  4:19   ` bluez.test.bot
2022-11-18  4:38   ` bluez.test.bot
2022-11-15 20:28 ` [patch 02/15] clocksource/drivers/arm_arch_timer: Do not use timer namespace for timer_shutdown() function Thomas Gleixner
2022-11-15 20:28 ` [patch 03/15] clocksource/drivers/sp804: " Thomas Gleixner
2022-11-15 20:28 ` [patch 04/15] timers: Get rid of del_singleshot_timer_sync() Thomas Gleixner
2022-11-21 20:08   ` Steven Rostedt
2022-11-15 20:28 ` [patch 05/15] timers: Replace BUG_ON()s Thomas Gleixner
2022-11-21 20:18   ` Steven Rostedt
2022-11-15 20:28 ` [patch 06/15] timers: Update kernel-doc for various functions Thomas Gleixner
2022-11-21 20:43   ` Steven Rostedt
2022-11-22  0:09     ` Randy Dunlap
2022-11-22  0:21       ` Steven Rostedt
2022-11-22 15:18     ` Thomas Gleixner
2022-11-22 15:38       ` Steven Rostedt
2022-11-22 15:41       ` Steven Rostedt
2022-11-22 16:42         ` Thomas Gleixner
2022-11-15 20:28 ` [patch 07/15] timers: Use del_timer_sync() even on UP Thomas Gleixner
2022-11-15 20:28 ` [patch 08/15] timers: Rename del_timer_sync() to timer_delete_sync() Thomas Gleixner
2022-11-21 20:52   ` Steven Rostedt
2022-11-15 20:28 ` [patch 09/15] timers: Rename del_timer() to timer_delete() Thomas Gleixner
2022-11-21 21:08   ` Steven Rostedt
2022-11-15 20:28 ` [patch 10/15] timers: Silently ignore timers with a NULL function Thomas Gleixner
2022-11-21 21:35   ` Steven Rostedt
2022-11-21 21:46     ` Thomas Gleixner
2022-11-15 20:28 ` [patch 11/15] timers: Split [try_to_]del_timer[_sync]() to prepare for shutdown mode Thomas Gleixner
2022-11-15 20:28 ` [patch 12/15] timers: Add shutdown mechanism to the internal functions Thomas Gleixner
2022-11-21 22:18   ` Steven Rostedt
2022-11-15 20:28 ` [patch 13/15] timers: Provide timer_shutdown[_sync]() Thomas Gleixner
2022-11-21 22:21   ` Steven Rostedt
2022-11-15 20:28 ` [patch 14/15] timers: Update the documentation to reflect on the new timer_shutdown() API Thomas Gleixner
2022-11-15 20:28 ` [patch 15/15] Bluetooth: hci_qca: Fix the teardown problem for real Thomas Gleixner
2022-11-15 21:29   ` Luiz Augusto von Dentz
2022-11-17 14:10 ` [patch 00/15] timers: Provide timer_shutdown[_sync]() Guenter Roeck
2022-11-21 15:15 ` Thomas Gleixner
2022-11-21 15:26   ` Steven Rostedt
2022-11-22  2:38 ` Steven Rostedt

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=20221115195802.415956561@linutronix.de \
    --to=tglx@linutronix.de \
    --cc=Julia.Lawall@inria.fr \
    --cc=akpm@linux-foundation.org \
    --cc=anna-maria@linutronix.de \
    --cc=arnd@arndb.de \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=johan.hedberg@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-bluetooth@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=luiz.dentz@gmail.com \
    --cc=marcel@holtmann.org \
    --cc=maz@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=sboyd@kernel.org \
    --cc=torvalds@linuxfoundation.org \
    --cc=viresh.kumar@linaro.org \
    /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.