I wonder why don't you use cancel_delayed_work_sync() here (and in a
few other places), like in bond_work_cancel_all() from patch 2/4?
In bond_close(), you can't use cancel_delayed_work_sync() because you can't
unlock RTNL in it. (So, on current implementation, it becomes work cancel is
not ensured on bond_close().)
In sysfs, I think you are right, thanks. So, I will modify the patch as below
(other patches won't be affected).

---

 drivers/net/bonding/bond_main.c  |   46 ++++++++++++++++++++------------------
 drivers/net/bonding/bond_sysfs.c |   34 ++++++++--------------------
 drivers/net/bonding/bonding.h    |    3 +-
 3 files changed, 37 insertions(+), 46 deletions(-)

--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2692,7 +2692,7 @@ out:
 void bond_loadbalance_arp_mon(struct work_struct *work)
 {
        struct bonding *bond = container_of(work, struct bonding,
-                                           arp_work.work);
+                                           lb_arp_work.work);
        struct slave *slave, *oldcurrent;
        int do_failover = 0;
        int delta_in_ticks;
@@ -2803,7 +2803,7 @@ void bond_loadbalance_arp_mon(struct wor

 re_arm:
        if (bond->params.arp_interval)
-               queue_delayed_work(bond->wq, &bond->arp_work, delta_in_ticks);
+               queue_delayed_work(bond->wq, &bond->lb_arp_work, 
delta_in_ticks);
 out:
        read_unlock(&bond->lock);
 }
@@ -2826,7 +2826,7 @@ out:
 void bond_activebackup_arp_mon(struct work_struct *work)
 {
        struct bonding *bond = container_of(work, struct bonding,
-                                           arp_work.work);
+                                           ab_arp_work.work);
        struct slave *slave;
        int delta_in_ticks;
        int i;
@@ -3060,7 +3060,7 @@ void bond_activebackup_arp_mon(struct wo

 re_arm:
        if (bond->params.arp_interval) {
-               queue_delayed_work(bond->wq, &bond->arp_work, delta_in_ticks);
+               queue_delayed_work(bond->wq, &bond->ab_arp_work, 
delta_in_ticks);
        }
 out:
        read_unlock(&bond->lock);
@@ -3679,30 +3679,23 @@ static int bond_open(struct net_device *
                        return -1;
                }

-               INIT_DELAYED_WORK(&bond->alb_work, bond_alb_monitor);
                queue_delayed_work(bond->wq, &bond->alb_work, 0);
        }

        if (bond->params.miimon) {  /* link check interval, in milliseconds. */
-               INIT_DELAYED_WORK(&bond->mii_work, bond_mii_monitor);
                queue_delayed_work(bond->wq, &bond->mii_work, 0);
        }

        if (bond->params.arp_interval) {  /* arp interval, in milliseconds. */
                if (bond->params.mode == BOND_MODE_ACTIVEBACKUP)
-                       INIT_DELAYED_WORK(&bond->arp_work,
-                                         bond_activebackup_arp_mon);
+                       queue_delayed_work(bond->wq, &bond->ab_arp_work, 0);
                else
-                       INIT_DELAYED_WORK(&bond->arp_work,
-                                         bond_loadbalance_arp_mon);
-
-               queue_delayed_work(bond->wq, &bond->arp_work, 0);
+                       queue_delayed_work(bond->wq, &bond->lb_arp_work, 0);
                if (bond->params.arp_validate)
                        bond_register_arp(bond);
        }

        if (bond->params.mode == BOND_MODE_8023AD) {
-               INIT_DELAYED_WORK(&bond->ad_work, 
bond_3ad_state_machine_handler);
                queue_delayed_work(bond->wq, &bond->ad_work, 0);
                /* register to receive LACPDUs */
                bond_register_lacpdu(bond);
@@ -3736,7 +3729,10 @@ static int bond_close(struct net_device
        }

        if (bond->params.arp_interval) {  /* arp interval, in milliseconds. */
-               cancel_delayed_work(&bond->arp_work);
+               if (bond->params.mode == BOND_MODE_ACTIVEBACKUP)
+                       cancel_delayed_work(&bond->ab_arp_work);
+               else
+                       cancel_delayed_work(&bond->lb_arp_work);
        }

        switch (bond->params.mode) {
@@ -4416,6 +4412,12 @@ static int bond_init(struct net_device *
        if (!bond->wq)
                return -ENOMEM;

+       INIT_DELAYED_WORK(&bond->alb_work, bond_alb_monitor);
+       INIT_DELAYED_WORK(&bond->mii_work, bond_mii_monitor);
+       INIT_DELAYED_WORK(&bond->ab_arp_work, bond_activebackup_arp_mon);
+       INIT_DELAYED_WORK(&bond->lb_arp_work, bond_loadbalance_arp_mon);
+       INIT_DELAYED_WORK(&bond->ad_work, bond_3ad_state_machine_handler);
+
        /* Initialize pointers */
        bond->first_slave = NULL;
        bond->curr_active_slave = NULL;
@@ -4498,18 +4500,20 @@ static void bond_work_cancel_all(struct
        bond->kill_timers = 1;
        write_unlock_bh(&bond->lock);

-       if (bond->params.miimon && delayed_work_pending(&bond->mii_work))
+       if (bond->params.miimon)
                cancel_delayed_work(&bond->mii_work);

-       if (bond->params.arp_interval && delayed_work_pending(&bond->arp_work))
-               cancel_delayed_work(&bond->arp_work);
+       if (bond->params.arp_interval) {
+               if (bond->params.mode == BOND_MODE_ACTIVEBACKUP)
+                       cancel_delayed_work(&bond->ab_arp_work);
+               else
+                       cancel_delayed_work(&bond->lb_arp_work);
+       }

-       if (bond->params.mode == BOND_MODE_ALB &&
-           delayed_work_pending(&bond->alb_work))
+       if (bond->params.mode == BOND_MODE_ALB)
                cancel_delayed_work(&bond->alb_work);

-       if (bond->params.mode == BOND_MODE_8023AD &&
-           delayed_work_pending(&bond->ad_work))
+       if (bond->params.mode == BOND_MODE_8023AD)
                cancel_delayed_work(&bond->ad_work);
 }

--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -643,10 +643,7 @@ static ssize_t bonding_store_arp_interva
                       "%s Disabling MII monitoring.\n",
                       bond->dev->name, bond->dev->name);
                bond->params.miimon = 0;
-               if (delayed_work_pending(&bond->mii_work)) {
-                       cancel_delayed_work(&bond->mii_work);
-                       flush_workqueue(bond->wq);
-               }
+               cancel_delayed_work_sync(&bond->mii_work);
        }
        if (!bond->params.arp_targets[0]) {
                printk(KERN_INFO DRV_NAME
@@ -660,16 +657,10 @@ static ssize_t bonding_store_arp_interva
                 * timer will get fired off when the open function
                 * is called.
                 */
-               if (!delayed_work_pending(&bond->arp_work)) {
-                       if (bond->params.mode == BOND_MODE_ACTIVEBACKUP)
-                               INIT_DELAYED_WORK(&bond->arp_work,
-                                                 bond_activebackup_arp_mon);
-                       else
-                               INIT_DELAYED_WORK(&bond->arp_work,
-                                                 bond_loadbalance_arp_mon);
-
-                       queue_delayed_work(bond->wq, &bond->arp_work, 0);
-               }
+               if (bond->params.mode == BOND_MODE_ACTIVEBACKUP)
+                       queue_delayed_work(bond->wq, &bond->ab_arp_work, 0);
+               else
+                       queue_delayed_work(bond->wq, &bond->lb_arp_work, 0);
        }

 out:
@@ -1022,10 +1013,10 @@ static ssize_t bonding_store_miimon(stru
                                bond->params.arp_validate =
                                        BOND_ARP_VALIDATE_NONE;
                        }
-                       if (delayed_work_pending(&bond->arp_work)) {
-                               cancel_delayed_work(&bond->arp_work);
-                               flush_workqueue(bond->wq);
-                       }
+                       if (bond->params.mode == BOND_MODE_ACTIVEBACKUP)
+                               cancel_delayed_work_sync(&bond->ab_arp_work);
+                       else
+                               cancel_delayed_work_sync(&bond->lb_arp_work);
                }

                if (bond->dev->flags & IFF_UP) {
@@ -1034,12 +1025,7 @@ static ssize_t bonding_store_miimon(stru
                         * timer will get fired off when the open function
                         * is called.
                         */
-                       if (!delayed_work_pending(&bond->mii_work)) {
-                               INIT_DELAYED_WORK(&bond->mii_work,
-                                                 bond_mii_monitor);
-                               queue_delayed_work(bond->wq,
-                                                  &bond->mii_work, 0);
-                       }
+                       queue_delayed_work(bond->wq, &bond->mii_work, 0);
                }
        }
 out:
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -206,7 +206,8 @@ struct bonding {
        struct   packet_type arp_mon_pt;
        struct   workqueue_struct *wq;
        struct   delayed_work mii_work;
-       struct   delayed_work arp_work;
+       struct   delayed_work ab_arp_work;
+       struct   delayed_work lb_arp_work;
        struct   delayed_work alb_work;
        struct   delayed_work ad_work;
 };

--
Makito SHIOKAWA
MIRACLE LINUX CORPORATION

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to