diff mbox series

[V2,09/17] timers: Rename del_timer_sync() to timer_delete_sync()

Message ID 20221122173648.619071341@linutronix.de (mailing list archive)
State Superseded
Headers show
Series timers: Provide timer_shutdown[_sync]() | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch

Commit Message

Thomas Gleixner Nov. 22, 2022, 5:44 p.m. UTC 
The timer related functions do not have a strict timer_ prefixed namespace
which is really annoying.

Rename del_timer_sync() to timer_delete_sync() and provide del_timer_sync()
as a wrapper. Document that del_timer_sync() is not for new code.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Tested-by: Guenter Roeck <linux@roeck-us.net>
---
 include/linux/timer.h |   15 ++++++++++++++-
 kernel/time/timer.c   |   18 +++++++++---------
 2 files changed, 23 insertions(+), 10 deletions(-)

Comments

David Laight Nov. 22, 2022, 10:23 p.m. UTC | #1 
From: Thomas Gleixner
> Sent: 22 November 2022 17:45
> 
> The timer related functions do not have a strict timer_ prefixed namespace
> which is really annoying.
> 
> Rename del_timer_sync() to timer_delete_sync() and provide del_timer_sync()
> as a wrapper. Document that del_timer_sync() is not for new code.

To change the colo[u]r of the bikeshed, would it be better to
name the functions timer_start() and timer_stop[_sync]().

And, as I found out for a local driver, adding items to work queues
from timer callbacks really isn't a good idea at all!
The delayed_work functions handle it a lot better.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
Steven Rostedt Nov. 22, 2022, 10:45 p.m. UTC | #2 
On Tue, 22 Nov 2022 22:23:11 +0000
David Laight <David.Laight@ACULAB.COM> wrote:

> > Rename del_timer_sync() to timer_delete_sync() and provide del_timer_sync()
> > as a wrapper. Document that del_timer_sync() is not for new code.  
> 
> To change the colo[u]r of the bikeshed, would it be better to
> name the functions timer_start() and timer_stop[_sync]().

I kinda like this color. ;-)

-- Steve
Thomas Gleixner Nov. 23, 2022, 12:08 a.m. UTC | #3 
On Tue, Nov 22 2022 at 17:45, Steven Rostedt wrote:
> On Tue, 22 Nov 2022 22:23:11 +0000
> David Laight <David.Laight@ACULAB.COM> wrote:
>
>> > Rename del_timer_sync() to timer_delete_sync() and provide del_timer_sync()
>> > as a wrapper. Document that del_timer_sync() is not for new code.  
>> 
>> To change the colo[u]r of the bikeshed, would it be better to
>> name the functions timer_start() and timer_stop[_sync]().
>
> I kinda like this color. ;-)

Feel free to repaint the series with this new color. My spare cycles are
exhausted by now.

Thanks,

        tglx
Steven Rostedt Nov. 23, 2022, 12:28 a.m. UTC | #4 
On Wed, 23 Nov 2022 01:08:53 +0100
Thomas Gleixner <tglx@linutronix.de> wrote:

> >> To change the colo[u]r of the bikeshed, would it be better to
> >> name the functions timer_start() and timer_stop[_sync]().  
> >
> > I kinda like this color. ;-)  
> 
> Feel free to repaint the series with this new color. My spare cycles are
> exhausted by now.

Yep, it can always be repainted. Let's just get this in now.

-- Steve
diff mbox series

Patch

--- a/include/linux/timer.h
+++ b/include/linux/timer.h
@@ -183,7 +183,20 @@  extern int timer_reduce(struct timer_lis
 extern void add_timer(struct timer_list *timer);
 
 extern int try_to_del_timer_sync(struct timer_list *timer);
-extern int del_timer_sync(struct timer_list *timer);
+extern int timer_delete_sync(struct timer_list *timer);
+
+/**
+ * del_timer_sync - Delete a pending timer and wait for a running callback
+ * @timer:	The timer to be deleted
+ *
+ * See timer_delete_sync() for detailed explanation.
+ *
+ * Do not use in new code. Use timer_delete_sync() instead.
+ */
+static inline int del_timer_sync(struct timer_list *timer)
+{
+	return timer_delete_sync(timer);
+}
 
 extern void init_timers(void);
 struct hrtimer;
--- a/kernel/time/timer.c
+++ b/kernel/time/timer.c
@@ -1083,7 +1083,7 @@  static inline int
 		/*
 		 * We are trying to schedule the timer on the new base.
 		 * However we can't change timer's base while it is running,
-		 * otherwise del_timer_sync() can't detect that the timer's
+		 * otherwise timer_delete_sync() can't detect that the timer's
 		 * handler yet has not finished. This also guarantees that the
 		 * timer is serialized wrt itself.
 		 */
@@ -1257,7 +1257,7 @@  EXPORT_SYMBOL_GPL(add_timer_on);
  * @timer:	The timer to be deactivated
  *
  * The function only deactivates a pending timer, but contrary to
- * del_timer_sync() it does not take into account whether the timers
+ * timer_delete_sync() it does not take into account whether the timers
  * callback function is concurrently executed on a different CPU or not.
  * It neither prevents rearming of the timer.  If @timer can be rearmed
  * concurrently then the return value of this function is meaningless.
@@ -1393,7 +1393,7 @@  static inline void del_timer_wait_runnin
 #endif
 
 /**
- * del_timer_sync - Deactivate a timer and wait for the handler to finish.
+ * 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,
@@ -1415,10 +1415,10 @@  static inline void del_timer_wait_runnin
  *    spin_lock_irq(somelock);
  *                                     <IRQ>
  *                                        spin_lock(somelock);
- *    del_timer_sync(mytimer);
+ *    timer_delete_sync(mytimer);
  *    while (base->running_timer == mytimer);
  *
- * Now del_timer_sync() will never return and never release somelock.
+ * 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.
  *
@@ -1431,7 +1431,7 @@  static inline void del_timer_wait_runnin
  * * %0	- The timer was not pending
  * * %1	- The timer was pending and deactivated
  */
-int del_timer_sync(struct timer_list *timer)
+int timer_delete_sync(struct timer_list *timer)
 {
 	int ret;
 
@@ -1471,7 +1471,7 @@  int del_timer_sync(struct timer_list *ti
 
 	return ret;
 }
-EXPORT_SYMBOL(del_timer_sync);
+EXPORT_SYMBOL(timer_delete_sync);
 
 static void call_timer_fn(struct timer_list *timer,
 			  void (*fn)(struct timer_list *),
@@ -1493,8 +1493,8 @@  static void call_timer_fn(struct timer_l
 #endif
 	/*
 	 * Couple the lock chain with the lock chain at
-	 * del_timer_sync() by acquiring the lock_map around the fn()
-	 * call here and in del_timer_sync().
+	 * timer_delete_sync() by acquiring the lock_map around the fn()
+	 * call here and in timer_delete_sync().
 	 */
 	lock_map_acquire(&lockdep_map);