Re: [patch 00/16] timers: Plug debugobject leaks and use del_timer_sync() in exit/teardown

From: Julia Lawall
Date: Sun Mar 23 2014 - 18:34:25 EST


As far as I could tell, (part of) the issue is that any kind of exit or
close function should use del_timer_sync, because they could be called
from a different CPU than the one that started up the timer?

Here is a semantic patch that takes care of the case of simple module_exit
functions:

@r@
declarer name module_exit;
identifier ex;
@@

module_exit(ex);

@@
identifier r.ex;
@@

ex(...) {
<...
- del_timer
+ del_timer_sync
(...)
...>
}

The transformations it makes are below. I haven't had a chance to check
which results overlap with what Thomas has already sent, but I could look
into it if this is the right idea. I guess other kinds of close/exit
functions would have to be identified manually, to make more rules.

In what circumstance can one be sure that two instructions are executed on
the same CPU?

julia

diff -u -p a/arch/mips/lasat/picvue_proc.c b/arch/mips/lasat/picvue_proc.c
--- a/arch/mips/lasat/picvue_proc.c
+++ b/arch/mips/lasat/picvue_proc.c
@@ -175,7 +175,7 @@ static void pvc_proc_cleanup(void)
remove_proc_entry("scroll", pvc_display_dir);
remove_proc_entry(DISPLAY_DIR_NAME, NULL);

- del_timer(&timer);
+ del_timer_sync(&timer);
}

static int __init pvc_proc_init(void)
diff -u -p a/drivers/tty/serial/mux.c b/drivers/tty/serial/mux.c
--- a/drivers/tty/serial/mux.c
+++ b/drivers/tty/serial/mux.c
@@ -613,7 +613,7 @@ static void __exit mux_exit(void)
{
/* Delete the Mux timer. */
if(port_cnt > 0) {
- del_timer(&mux_timer);
+ del_timer_sync(&mux_timer);
#ifdef CONFIG_SERIAL_MUX_CONSOLE
unregister_console(&mux_console);
#endif
diff -u -p a/drivers/input/serio/hp_sdc.c b/drivers/input/serio/hp_sdc.c
--- a/drivers/input/serio/hp_sdc.c
+++ b/drivers/input/serio/hp_sdc.c
@@ -984,7 +984,7 @@ static void hp_sdc_exit(void)
free_irq(hp_sdc.irq, &hp_sdc);
write_unlock_irq(&hp_sdc.lock);

- del_timer(&hp_sdc.kicker);
+ del_timer_sync(&hp_sdc.kicker);

tasklet_kill(&hp_sdc.task);

diff -u -p a/drivers/isdn/sc/init.c b/drivers/isdn/sc/init.c
--- a/drivers/isdn/sc/init.c
+++ b/drivers/isdn/sc/init.c
@@ -390,8 +390,8 @@ static void __exit sc_exit(void)
/*
* kill the timers
*/
- del_timer(&(sc_adapter[i]->reset_timer));
- del_timer(&(sc_adapter[i]->stat_timer));
+ del_timer_sync(&(sc_adapter[i]->reset_timer));
+ del_timer_sync(&(sc_adapter[i]->stat_timer));

/*
* Tell I4L we're toast
diff -u -p a/drivers/isdn/i4l/isdn_common.c b/drivers/isdn/i4l/isdn_common.c
--- a/drivers/isdn/i4l/isdn_common.c
+++ b/drivers/isdn/i4l/isdn_common.c
@@ -2381,7 +2381,7 @@ static void __exit isdn_exit(void)
}
isdn_tty_exit();
unregister_chrdev(ISDN_MAJOR, "isdn");
- del_timer(&dev->timer);
+ del_timer_sync(&dev->timer);
/* call vfree with interrupts enabled, else it will hang */
vfree(dev);
printk(KERN_NOTICE "ISDN-subsystem unloaded\n");
diff -u -p a/drivers/isdn/hardware/mISDN/hfcpci.c b/drivers/isdn/hardware/mISDN/hfcpci.c
--- a/drivers/isdn/hardware/mISDN/hfcpci.c
+++ b/drivers/isdn/hardware/mISDN/hfcpci.c
@@ -2352,7 +2352,7 @@ static void __exit
HFC_cleanup(void)
{
if (timer_pending(&hfc_tl))
- del_timer(&hfc_tl);
+ del_timer_sync(&hfc_tl);

pci_unregister_driver(&hfc_driver);
}
diff -u -p a/drivers/isdn/act2000/module.c b/drivers/isdn/act2000/module.c
--- a/drivers/isdn/act2000/module.c
+++ b/drivers/isdn/act2000/module.c
@@ -796,7 +796,7 @@ static void __exit act2000_exit(void)
act2000_card *last;
while (card) {
unregister_card(card);
- del_timer(&card->ptimer);
+ del_timer_sync(&card->ptimer);
card = card->next;
}
card = cards;
diff -u -p a/drivers/net/hamradio/yam.c b/drivers/net/hamradio/yam.c
--- a/drivers/net/hamradio/yam.c
+++ b/drivers/net/hamradio/yam.c
@@ -1184,7 +1184,7 @@ static void __exit yam_cleanup_driver(vo
struct yam_mcs *p;
int i;

- del_timer(&yam_timer);
+ del_timer_sync(&yam_timer);
for (i = 0; i < NR_PORTS; i++) {
struct net_device *dev = yam_devs[i];
if (dev) {
diff -u -p a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c
--- a/drivers/staging/speakup/main.c
+++ b/drivers/staging/speakup/main.c
@@ -2218,7 +2218,7 @@ static void __exit speakup_exit(void)
unregister_keyboard_notifier(&keyboard_notifier_block);
unregister_vt_notifier(&vt_notifier_block);
speakup_unregister_devsynth();
- del_timer(&cursor_timer);
+ del_timer_sync(&cursor_timer);
kthread_stop(speakup_task);
speakup_task = NULL;
mutex_lock(&spk_mutex);
diff -u -p a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
--- a/drivers/staging/panel/panel.c
+++ b/drivers/staging/panel/panel.c
@@ -2298,7 +2298,7 @@ static void __exit panel_cleanup_module(
unregister_reboot_notifier(&panel_notifier);

if (scan_timer.function != NULL)
- del_timer(&scan_timer);
+ del_timer_sync(&scan_timer);

if (pprt != NULL) {
if (keypad_enabled) {
diff -u -p a/drivers/staging/lustre/lustre/llite/super25.c b/drivers/staging/lustre/lustre/llite/super25.c
--- a/drivers/staging/lustre/lustre/llite/super25.c
+++ b/drivers/staging/lustre/lustre/llite/super25.c
@@ -197,7 +197,7 @@ static void __exit exit_lustre_lite(void
{
ll_xattr_fini();
vvp_global_fini();
- del_timer(&ll_capa_timer);
+ del_timer_sync(&ll_capa_timer);
ll_capa_thread_stop();
LASSERTF(capa_count[CAPA_SITE_CLIENT] == 0,
"client remaining capa count %d\n",
diff -u -p a/drivers/atm/idt77105.c b/drivers/atm/idt77105.c
--- a/drivers/atm/idt77105.c
+++ b/drivers/atm/idt77105.c
@@ -369,8 +369,8 @@ EXPORT_SYMBOL(idt77105_init);
static void __exit idt77105_exit(void)
{
/* turn off timers */
- del_timer(&stats_timer);
- del_timer(&restart_timer);
+ del_timer_sync(&stats_timer);
+ del_timer_sync(&restart_timer);
}

module_exit(idt77105_exit);
diff -u -p a/drivers/atm/nicstar.c b/drivers/atm/nicstar.c
--- a/drivers/atm/nicstar.c
+++ b/drivers/atm/nicstar.c
@@ -306,7 +306,7 @@ static void __exit nicstar_cleanup(void)
{
XPRINTK("nicstar: nicstar_cleanup() called.\n");

- del_timer(&ns_timer);
+ del_timer_sync(&ns_timer);

pci_unregister_driver(&nicstar_driver);

diff -u -p a/drivers/atm/iphase.c b/drivers/atm/iphase.c
--- a/drivers/atm/iphase.c
+++ b/drivers/atm/iphase.c
@@ -3289,7 +3289,7 @@ static void __exit ia_module_exit(void)
{
pci_unregister_driver(&ia_driver);

- del_timer(&ia_timer);
+ del_timer_sync(&ia_timer);
}

module_init(ia_module_init);
diff -u -p a/net/hsr/hsr_main.c b/net/hsr/hsr_main.c
--- a/net/hsr/hsr_main.c
+++ b/net/hsr/hsr_main.c
@@ -459,7 +459,7 @@ static int __init hsr_init(void)
static void __exit hsr_exit(void)
{
unregister_netdevice_notifier(&hsr_nb);
- del_timer(&prune_timer);
+ del_timer_sync(&prune_timer);
hsr_netlink_exit();
dev_remove_pack(&hsr_pt);
}
diff -u -p a/net/atm/mpc.c b/net/atm/mpc.c
--- a/net/atm/mpc.c
+++ b/net/atm/mpc.c
@@ -1492,7 +1492,7 @@ static void __exit atm_mpoa_cleanup(void

mpc_proc_clean();

- del_timer(&mpc_timer);
+ del_timer_sync(&mpc_timer);
unregister_netdevice_notifier(&mpoa_notifier);
deregister_atm_ioctl(&atm_ioctl_ops);

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