Re: [PATCH] timers: fix synchronization rules in comments of del_timer_sync

From: duoming
Date: Mon Aug 08 2022 - 21:02:46 EST


Hello,

On Mon, 08 Aug 2022 16:01:57 +0200 Thomas Gleixner wrote:

> On Fri, Jul 01 2022 at 16:55, Duoming Zhou wrote:
> > - * 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
> > - * handler. 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.
> > + * Synchronization rules: Callers must prevent restarting of the timer in
> > + * other places except for the timer's handler, 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 handler. The timer's handler must
> > + * not call add_timer_on().
>
> If we are making this correct: What's so special about add_timer_on()?

The del_timer_sync() could also stop add_timer_on(), there is nothing special
about add_timer_on(). I think change the annotation to the following is better.

diff --git a/kernel/time/timer.c b/kernel/time/timer.c
index 717fcb9fb14..dd623018dbc 100644
--- a/kernel/time/timer.c
+++ b/kernel/time/timer.c
@@ -1375,11 +1375,11 @@ static inline void del_timer_wait_running(struct timer_list *timer) { }
* CPUs.
*
* 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
- * handler. 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.
+ * otherwise this function is meaningless. It could also stop periodic timer.
+ * 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 handler. Upon exit the timer is not queued and the handler is not
+ * running on any CPU.

Best regards,
Duoming Zhou