lkml.org 
[lkml]   [2022]   [Nov]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [patch V3 00/17] timers: Provide timer_shutdown[_sync]()
    On Wed, 23 Nov 2022, Thomas Gleixner wrote:

    > This is the third version of the timer shutdown work. The second version
    > can be found here:
    >
    > https://lore.kernel.org/all/20221122171312.191765396@linutronix.de
    >
    > 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
    >

    With fix of the last NITs:

    Reviewed-by: Anna-Maria Behnsen <anna-maria@linutronix.de>

    Thanks,
    Anna-Maria

    \
     
     \ /
      Last update: 2022-11-24 15:06    [W:2.199 / U:0.052 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site