Re: [PATCH v5a 5/5] treewide: Convert del_timer*() to timer_shutdown*()

[Steven Rostedt]

On Sun, 6 Nov 2022 12:51:46 -0800
Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:

> Thanks, this looks reasonable.
> 
> On Sat, Nov 5, 2022 at 10:46 PM Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
> >
> > The coccinelle script:
> >
> > @@
> > expression E;
> > identifier ptr, timer, rfield, slab;  
> 
> I think Julia suggested making 'ptr' be an expression too, and I think

And I forgot to add Julia to the Cc :-/

> she's right. Probably 'slab' should be too - there's no reason to
> limit it to just one identifier.
> 
> >   ... when strict
> >       when != ptr->timer.function = E;  
> 
> I suspect any "ptr->timer" access anywhere between the
> del_timer_sync() and the freeing should disable things.
> 
> Although hopefully there aren't any other odd cases than that one
> "clear timer function by hand" one.

OK, I did the following with this new script:

$ cat timer.cocci
@@
expression E,ptr,slab;
identifier timer, rfield;
@@
(
-       del_timer(&ptr->timer);
+       timer_shutdown(&ptr->timer);
|
-       del_timer_sync(&ptr->timer);
+       timer_shutdown_sync(&ptr->timer);
)
  ... when strict
      when != ptr->timer.function = E;
(
        kfree_rcu(ptr, rfield);
|
        kmem_cache_free(slab, ptr);
|
        kfree(ptr);
)


$ git show | patch -p1 -R
$ spatch --dir timer.cocci . > /tmp/t.patch
$ patch -p1 < /tmp/t.patch

And it caught some more:

$ git diff
diff --git a/drivers/atm/idt77252.c b/drivers/atm/idt77252.c
index 99cae174d558..eec0cc2144e0 100644
--- a/drivers/atm/idt77252.c
+++ b/drivers/atm/idt77252.c
@@ -2530,7 +2530,7 @@ idt77252_close(struct atm_vcc *vcc)
                vc->tx_vcc = NULL;
 
                if (vc->estimator) {
-                       del_timer(&vc->estimator->timer);
+                       timer_shutdown(&vc->estimator->timer);
                        kfree(vc->estimator);
                        vc->estimator = NULL;
                }
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/btcoex.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/btcoex.c
index f9f18ff451ea..7ea2631b8069 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/btcoex.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/btcoex.c
@@ -394,7 +394,7 @@ void brcmf_btcoex_detach(struct brcmf_cfg80211_info *cfg)
 
        if (cfg->btcoex->timer_on) {
                cfg->btcoex->timer_on = false;
-               del_timer_sync(&cfg->btcoex->timer);
+               timer_shutdown_sync(&cfg->btcoex->timer);
        }
 
        cancel_work_sync(&cfg->btcoex->work);
diff --git a/net/netfilter/xt_IDLETIMER.c b/net/netfilter/xt_IDLETIMER.c
index 0f8bb0bf558f..8d36303f3935 100644
--- a/net/netfilter/xt_IDLETIMER.c
+++ b/net/netfilter/xt_IDLETIMER.c
@@ -413,7 +413,7 @@ static void idletimer_tg_destroy(const struct xt_tgdtor_param *par)
                pr_debug("deleting timer %s\n", info->label);
 
                list_del(&info->timer->entry);
-               del_timer_sync(&info->timer->timer);
+               timer_shutdown_sync(&info->timer->timer);
                cancel_work_sync(&info->timer->work);
                sysfs_remove_file(idletimer_tg_kobj, &info->timer->attr.attr);
                kfree(info->timer->attr.attr.name);
@@ -441,7 +441,7 @@ static void idletimer_tg_destroy_v1(const struct xt_tgdtor_param *par)
                if (info->timer->timer_type & XT_IDLETIMER_ALARM) {
                        alarm_cancel(&info->timer->alarm);
                } else {
-                       del_timer_sync(&info->timer->timer);
+                       timer_shutdown_sync(&info->timer->timer);
                }
                cancel_work_sync(&info->timer->work);
                sysfs_remove_file(idletimer_tg_kobj, &info->timer->attr.attr);


Which all look legit.

I'll post a v6a using the new script.

-- Steve