Re: [RFC] [PATCH] Performance of del_timer_sync

[Andrew Morton]

Geoff Gustafson <geoff@xxxxxxxxxxxxxxxxxx> wrote:
>
> 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.

Looks good.  Your email client performs bizarre whitespace transformations, so I had to
fix the patch up by hand.

Here's an incremental patch:



diff -puN include/linux/timer.h~del_singleshot_timer_sync-tweaks include/linux/timer.h
--- 25/include/linux/timer.h~del_singleshot_timer_sync-tweaks	Tue May 11 15:59:57 2004
+++ 25-akpm/include/linux/timer.h	Tue May 11 15:59:57 2004
@@ -87,8 +87,8 @@ static inline void add_timer(struct time
 }
 
 #ifdef CONFIG_SMP
-  extern int del_timer_sync(struct timer_list * timer);
-  extern int del_singleshot_timer_sync(struct timer_list * timer);
+  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)
diff -puN kernel/timer.c~del_singleshot_timer_sync-tweaks kernel/timer.c
--- 25/kernel/timer.c~del_singleshot_timer_sync-tweaks	Tue May 11 15:59:57 2004
+++ 25-akpm/kernel/timer.c	Tue May 11 15:59:57 2004
@@ -317,10 +317,16 @@ EXPORT_SYMBOL(del_timer);
  *
  * 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.
+ * interrupt contexts. 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.
  *
  * The function returns whether it has deactivated a pending timer or not.
+ *
+ * del_timer_sync() is slow and complicated because it copes with timer
+ * handlers which re-arm the timer (periodic timers).  If the timer handler
+ * is known to not do this (a single shot timer) then use
+ * del_singleshot_timer_sync() instead.
  */
 int del_timer_sync(struct timer_list *timer)
 {
@@ -348,7 +354,6 @@ del_again:
 
 	return ret;
 }
-
 EXPORT_SYMBOL(del_timer_sync);
 
 /***
@@ -361,8 +366,9 @@ EXPORT_SYMBOL(del_timer_sync);
  *
  * 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.
+ * interrupt contexts. 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.
  *
  * The function returns whether it has deactivated a pending timer or not.
  */
@@ -377,7 +383,6 @@ int del_singleshot_timer_sync(struct tim
 
 	return ret;
 }
-
 EXPORT_SYMBOL(del_singleshot_timer_sync);
 #endif
 

_

-
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/