Re: [RFC] [PATCH] Performance of del_timer_sync

[Geoff Gustafson]

Andrew Morton wrote:
> <anal>del_singleshot_timer_sync</anal>
>
> I vote we leave it up to Ken.  But please, not del_timer_kenneth().

Despite your markup language, I thought keeping the 'sync' sounded like
a good idea, since that's the function it's replacing.

Here's a patch. I've done a small amount of testing with a 4p machine.
I've at least seen that the average call to singleshot is down around 90
cycles, pretty similar/expected. We'll test on the original platform and
post results.

- Geoff

* I am in a transitional period w/ email clients and can't seem to use
any of them properly. Sorry for the dups, etc.

diff -Naur fresh/fs/aio.c timer/fs/aio.c
--- fresh/fs/aio.c	Sun May  9 19:32:29 2004
+++ timer/fs/aio.c	Tue May 11 15:14:47 2004
@@ -793,7 +793,7 @@

 static inline void clear_timeout(struct timeout *to)
 {
-	del_timer_sync(&to->timer);
+	del_singleshot_timer_sync(&to->timer);
 }

 static int read_events(struct kioctx *ctx,
diff -Naur fresh/include/linux/timer.h timer/include/linux/timer.h
--- fresh/include/linux/timer.h	Sun May  9 19:32:54 2004
+++ timer/include/linux/timer.h	Tue May 11 14:21:36 2004
@@ -88,8 +88,10 @@

 #ifdef CONFIG_SMP
   extern int del_timer_sync(struct timer_list * timer);
+  extern int del_singleshot_timer_sync(struct timer_list * timer);
 #else
 # define del_timer_sync(t) del_timer(t)
+# define del_singleshot_timer_sync(t) del_timer(t)
 #endif

 extern void init_timers(void);
diff -Naur fresh/kernel/timer.c timer/kernel/timer.c
--- fresh/kernel/timer.c	Sun May  9 19:33:13 2004
+++ timer/kernel/timer.c	Tue May 11 15:08:24 2004
@@ -350,6 +350,35 @@
 }

 EXPORT_SYMBOL(del_timer_sync);
+
+/***
+ * del_singleshot_timer_sync - deactivate a non-recursive timer
+ * @timer: the timer to be deactivated
+ *
+ * This function is an optimization of del_timer_sync for the case where the
+ * caller can guarantee the timer does not reschedule itself in its timer
+ * function.
+ *
+ * Synchronization rules: callers must prevent restarting of the timer,
+ * otherwise this function is meaningless. It must not be called from
+ * interrupt contexts. Upon exit the timer is not queued and the handler
+ * is not running on any CPU.
+ *
+ * The function returns whether it has deactivated a pending timer or not.
+ */
+int del_singleshot_timer_sync(struct timer_list *timer)
+{
+	int ret = del_timer(timer);
+
+	if (!ret) {
+		ret = del_timer_sync(timer);
+		BUG_ON(ret);
+	}
+
+	return ret;
+}
+
+EXPORT_SYMBOL(del_singleshot_timer_sync);
 #endif

 static int cascade(tvec_base_t *base, tvec_t *tv, int index)
@@ -1109,7 +1138,7 @@

 	add_timer(&timer);
 	schedule();
-	del_timer_sync(&timer);
+	del_singleshot_timer_sync(&timer);

 	timeout = expire - jiffies;

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/