2010-12-12 15:53:13

by Tejun Heo

[permalink] [raw]
Subject: [PATCHSET] net-dev: don't use flush_scheduled_work()

Hello,

This patchset removes the use of flush_scheduled_work(), which is
being deprecated, from various network drivers. It contains the
following nine patches.

0001-drivers-net-remove-unnecessary-flush_scheduled_work-.patch
0002-drivers-net-don-t-use-flush_scheduled_work.patch
0003-ehea-kill-unused-ehea_rereg_mr_task.patch
0004-ehea-don-t-use-flush_scheduled_work.patch
0005-iseries_veth-don-t-use-flush_scheduled_work.patch
0006-igb-v-ixgbe-don-t-use-flush_scheduled_work.patch
0007-sungem-update-gp-reset_task-flushing.patch
0008-i2400m-drop-i2400m_schedule_work.patch
0009-hostap-don-t-use-flush_scheduled_work.patch

0001 and 0002 are straight forward conversions across multiple network
drivers. The rest are a bit more involved per-driver or driver family
conversions.

This patchset is on top of the current net-next-2.6.git#master
(ad1184c6cf067a13e8cb2a4e7ccc407f947027d0) and available in the
following git branch.

git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git net-dev-kill-flush_scheduled_work

diffstat follows. Thanks.

drivers/net/8139too.c | 3 -
drivers/net/atlx/atl2.c | 4 -
drivers/net/bcm63xx_enet.c | 2
drivers/net/bnx2.c | 4 -
drivers/net/can/janz-ican3.c | 9 ---
drivers/net/cassini.c | 4 -
drivers/net/cxgb3/cxgb3_main.c | 3 -
drivers/net/e1000e/netdev.c | 6 +-
drivers/net/ehea/ehea.h | 2
drivers/net/ehea/ehea_main.c | 14 +---
drivers/net/enic/enic_main.c | 2
drivers/net/ibm_newemac/core.c | 2
drivers/net/igb/igb_main.c | 9 ++-
drivers/net/igbvf/netdev.c | 7 +-
drivers/net/irda/mcs7780.c | 2
drivers/net/iseries_veth.c | 18 +-----
drivers/net/ixgb/ixgb_main.c | 2
drivers/net/ixgbe/ixgbe_main.c | 11 ++-
drivers/net/ixgbevf/ixgbevf_main.c | 3 -
drivers/net/mv643xx_eth.c | 2
drivers/net/myri10ge/myri10ge.c | 2
drivers/net/niu.c | 2
drivers/net/pxa168_eth.c | 2
drivers/net/r8169.c | 2
drivers/net/s2io.c | 6 +-
drivers/net/sh_eth.c | 1
drivers/net/sh_eth.h | 1
drivers/net/sis190.c | 3 -
drivers/net/skge.c | 2
drivers/net/sungem.c | 12 +---
drivers/net/tg3.c | 4 -
drivers/net/usb/sierra_net.c | 5 -
drivers/net/usb/usbnet.c | 3 -
drivers/net/vmxnet3/vmxnet3_drv.c | 2
drivers/net/vxge/vxge-main.c | 2
drivers/net/wimax/i2400m/driver.c | 96 +++++---------------------------
drivers/net/wimax/i2400m/i2400m.h | 19 +-----
drivers/net/wimax/i2400m/sdio.c | 1
drivers/net/wimax/i2400m/usb.c | 1
drivers/net/wireless/hostap/hostap_ap.c | 3 +
drivers/net/wireless/hostap/hostap_hw.c | 8 ++
41 files changed, 97 insertions(+), 189 deletions(-)

--
tejun


2010-12-12 15:53:36

by Tejun Heo

[permalink] [raw]
Subject: [PATCH 3/9] ehea: kill unused ehea_rereg_mr_task

ehea_rereg_mr_task is not used. Remove it and drop @work parameter
from ehea_rereg_mrs().

Signed-off-by: Tejun Heo <[email protected]>
Cc: Breno Leitao <[email protected]>
Cc: [email protected]
---
drivers/net/ehea/ehea.h | 2 --
drivers/net/ehea/ehea_main.c | 9 +++------
2 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ehea/ehea.h b/drivers/net/ehea/ehea.h
index 8e745e7..1f2a675 100644
--- a/drivers/net/ehea/ehea.h
+++ b/drivers/net/ehea/ehea.h
@@ -515,6 +515,4 @@ void ehea_set_ethtool_ops(struct net_device *netdev);
int ehea_sense_port_attr(struct ehea_port *port);
int ehea_set_portspeed(struct ehea_port *port, u32 port_speed);

-extern struct work_struct ehea_rereg_mr_task;
-
#endif /* __EHEA_H__ */
diff --git a/drivers/net/ehea/ehea_main.c b/drivers/net/ehea/ehea_main.c
index 69f6152..d51def1 100644
--- a/drivers/net/ehea/ehea_main.c
+++ b/drivers/net/ehea/ehea_main.c
@@ -101,7 +101,6 @@ MODULE_PARM_DESC(use_lro, " Large Receive Offload, 1: enable, 0: disable, "
static int port_name_cnt;
static LIST_HEAD(adapter_list);
static unsigned long ehea_driver_flags;
-struct work_struct ehea_rereg_mr_task;
static DEFINE_MUTEX(dlpar_mem_lock);
struct ehea_fw_handle_array ehea_fw_handles;
struct ehea_bcmc_reg_array ehea_bcmc_regs;
@@ -2984,7 +2983,7 @@ out:
mutex_unlock(&dlpar_mem_lock);
}

-static void ehea_rereg_mrs(struct work_struct *work)
+static void ehea_rereg_mrs(void)
{
int ret, i;
struct ehea_adapter *adapter;
@@ -3659,14 +3658,14 @@ static int ehea_mem_notifier(struct notifier_block *nb,
set_bit(__EHEA_STOP_XFER, &ehea_driver_flags);
if (ehea_add_sect_bmap(arg->start_pfn, arg->nr_pages))
goto out_unlock;
- ehea_rereg_mrs(NULL);
+ ehea_rereg_mrs();
break;
case MEM_GOING_OFFLINE:
ehea_info("memory is going offline");
set_bit(__EHEA_STOP_XFER, &ehea_driver_flags);
if (ehea_rem_sect_bmap(arg->start_pfn, arg->nr_pages))
goto out_unlock;
- ehea_rereg_mrs(NULL);
+ ehea_rereg_mrs();
break;
default:
break;
@@ -3742,8 +3741,6 @@ int __init ehea_module_init(void)
printk(KERN_INFO "IBM eHEA ethernet device driver (Release %s)\n",
DRV_VERSION);

-
- INIT_WORK(&ehea_rereg_mr_task, ehea_rereg_mrs);
memset(&ehea_fw_handles, 0, sizeof(ehea_fw_handles));
memset(&ehea_bcmc_regs, 0, sizeof(ehea_bcmc_regs));

--
1.7.1

2010-12-12 15:53:29

by Tejun Heo

[permalink] [raw]
Subject: [PATCH 8/9] i2400m: drop i2400m_schedule_work()

i2400m implements dynamic work allocation and queueing mechanism in
i2400_schedule_work(); however, this is only used for reset and
recovery which can be served equally well with preallocated per device
works.

Replace i2400m_schedule_work() with two work structs in struct i2400m.
These works are explicitly canceled when the device is released making
calls to flush_scheduled_work(), which is being deprecated,
unnecessary.

Signed-off-by: Tejun Heo <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Inaky Perez-Gonzalez <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
drivers/net/wimax/i2400m/driver.c | 96 ++++++------------------------------
drivers/net/wimax/i2400m/i2400m.h | 19 ++-----
drivers/net/wimax/i2400m/sdio.c | 1 -
drivers/net/wimax/i2400m/usb.c | 1 -
4 files changed, 21 insertions(+), 96 deletions(-)

diff --git a/drivers/net/wimax/i2400m/driver.c b/drivers/net/wimax/i2400m/driver.c
index cdedab4..f060332 100644
--- a/drivers/net/wimax/i2400m/driver.c
+++ b/drivers/net/wimax/i2400m/driver.c
@@ -92,54 +92,6 @@ MODULE_PARM_DESC(barkers,
"signal; values are appended to a list--setting one value "
"as zero cleans the existing list and starts a new one.");

-static
-struct i2400m_work *__i2400m_work_setup(
- struct i2400m *i2400m, void (*fn)(struct work_struct *),
- gfp_t gfp_flags, const void *pl, size_t pl_size)
-{
- struct i2400m_work *iw;
-
- iw = kzalloc(sizeof(*iw) + pl_size, gfp_flags);
- if (iw == NULL)
- return NULL;
- iw->i2400m = i2400m_get(i2400m);
- iw->pl_size = pl_size;
- memcpy(iw->pl, pl, pl_size);
- INIT_WORK(&iw->ws, fn);
- return iw;
-}
-
-
-/*
- * Schedule i2400m's specific work on the system's queue.
- *
- * Used for a few cases where we really need it; otherwise, identical
- * to i2400m_queue_work().
- *
- * Returns < 0 errno code on error, 1 if ok.
- *
- * If it returns zero, something really bad happened, as it means the
- * works struct was already queued, but we have just allocated it, so
- * it should not happen.
- */
-static int i2400m_schedule_work(struct i2400m *i2400m,
- void (*fn)(struct work_struct *), gfp_t gfp_flags,
- const void *pl, size_t pl_size)
-{
- int result;
- struct i2400m_work *iw;
-
- result = -ENOMEM;
- iw = __i2400m_work_setup(i2400m, fn, gfp_flags, pl, pl_size);
- if (iw != NULL) {
- result = schedule_work(&iw->ws);
- if (WARN_ON(result == 0))
- result = -ENXIO;
- }
- return result;
-}
-
-
/*
* WiMAX stack operation: relay a message from user space
*
@@ -648,17 +600,11 @@ EXPORT_SYMBOL_GPL(i2400m_post_reset);
static
void __i2400m_dev_reset_handle(struct work_struct *ws)
{
- int result;
- struct i2400m_work *iw = container_of(ws, struct i2400m_work, ws);
- const char *reason;
- struct i2400m *i2400m = iw->i2400m;
+ struct i2400m *i2400m = container_of(ws, struct i2400m, reset_ws);
+ const char *reason = i2400m->reset_reason;
struct device *dev = i2400m_dev(i2400m);
struct i2400m_reset_ctx *ctx = i2400m->reset_ctx;
-
- if (WARN_ON(iw->pl_size != sizeof(reason)))
- reason = "SW BUG: reason n/a";
- else
- memcpy(&reason, iw->pl, sizeof(reason));
+ int result;

d_fnstart(3, dev, "(ws %p i2400m %p reason %s)\n", ws, i2400m, reason);

@@ -733,8 +679,6 @@ void __i2400m_dev_reset_handle(struct work_struct *ws)
}
}
out:
- i2400m_put(i2400m);
- kfree(iw);
d_fnend(3, dev, "(ws %p i2400m %p reason %s) = void\n",
ws, i2400m, reason);
}
@@ -754,8 +698,8 @@ out:
*/
int i2400m_dev_reset_handle(struct i2400m *i2400m, const char *reason)
{
- return i2400m_schedule_work(i2400m, __i2400m_dev_reset_handle,
- GFP_ATOMIC, &reason, sizeof(reason));
+ i2400m->reset_reason = reason;
+ return schedule_work(&i2400m->reset_ws);
}
EXPORT_SYMBOL_GPL(i2400m_dev_reset_handle);

@@ -768,14 +712,9 @@ EXPORT_SYMBOL_GPL(i2400m_dev_reset_handle);
static
void __i2400m_error_recovery(struct work_struct *ws)
{
- struct i2400m_work *iw = container_of(ws, struct i2400m_work, ws);
- struct i2400m *i2400m = iw->i2400m;
+ struct i2400m *i2400m = container_of(ws, struct i2400m, recovery_ws);

i2400m_reset(i2400m, I2400M_RT_BUS);
-
- i2400m_put(i2400m);
- kfree(iw);
- return;
}

/*
@@ -805,18 +744,10 @@ void __i2400m_error_recovery(struct work_struct *ws)
*/
void i2400m_error_recovery(struct i2400m *i2400m)
{
- struct device *dev = i2400m_dev(i2400m);
-
- if (atomic_add_return(1, &i2400m->error_recovery) == 1) {
- if (i2400m_schedule_work(i2400m, __i2400m_error_recovery,
- GFP_ATOMIC, NULL, 0) < 0) {
- dev_err(dev, "run out of memory for "
- "scheduling an error recovery ?\n");
- atomic_dec(&i2400m->error_recovery);
- }
- } else
+ if (atomic_add_return(1, &i2400m->error_recovery) == 1)
+ schedule_work(&i2400m->recovery_ws);
+ else
atomic_dec(&i2400m->error_recovery);
- return;
}
EXPORT_SYMBOL_GPL(i2400m_error_recovery);

@@ -886,6 +817,10 @@ void i2400m_init(struct i2400m *i2400m)

mutex_init(&i2400m->init_mutex);
/* wake_tx_ws is initialized in i2400m_tx_setup() */
+
+ INIT_WORK(&i2400m->reset_ws, __i2400m_dev_reset_handle);
+ INIT_WORK(&i2400m->recovery_ws, __i2400m_error_recovery);
+
atomic_set(&i2400m->bus_reset_retries, 0);

i2400m->alive = 0;
@@ -1040,6 +975,9 @@ void i2400m_release(struct i2400m *i2400m)

i2400m_dev_stop(i2400m);

+ cancel_work_sync(&i2400m->reset_ws);
+ cancel_work_sync(&i2400m->recovery_ws);
+
i2400m_debugfs_rm(i2400m);
sysfs_remove_group(&i2400m->wimax_dev.net_dev->dev.kobj,
&i2400m_dev_attr_group);
@@ -1083,8 +1021,6 @@ module_init(i2400m_driver_init);
static
void __exit i2400m_driver_exit(void)
{
- /* for scheds i2400m_dev_reset_handle() */
- flush_scheduled_work();
i2400m_barker_db_exit();
}
module_exit(i2400m_driver_exit);
diff --git a/drivers/net/wimax/i2400m/i2400m.h b/drivers/net/wimax/i2400m/i2400m.h
index 59ac770..17ecaa4 100644
--- a/drivers/net/wimax/i2400m/i2400m.h
+++ b/drivers/net/wimax/i2400m/i2400m.h
@@ -632,6 +632,11 @@ struct i2400m {
struct work_struct wake_tx_ws;
struct sk_buff *wake_tx_skb;

+ struct work_struct reset_ws;
+ const char *reset_reason;
+
+ struct work_struct recovery_ws;
+
struct dentry *debugfs_dentry;
const char *fw_name; /* name of the current firmware image */
unsigned long fw_version; /* version of the firmware interface */
@@ -896,20 +901,6 @@ struct device *i2400m_dev(struct i2400m *i2400m)
return i2400m->wimax_dev.net_dev->dev.parent;
}

-/*
- * Helper for scheduling simple work functions
- *
- * This struct can get any kind of payload attached (normally in the
- * form of a struct where you pack the stuff you want to pass to the
- * _work function).
- */
-struct i2400m_work {
- struct work_struct ws;
- struct i2400m *i2400m;
- size_t pl_size;
- u8 pl[0];
-};
-
extern int i2400m_msg_check_status(const struct i2400m_l3l4_hdr *,
char *, size_t);
extern int i2400m_msg_size_check(struct i2400m *,
diff --git a/drivers/net/wimax/i2400m/sdio.c b/drivers/net/wimax/i2400m/sdio.c
index 9bfc26e..be428ca 100644
--- a/drivers/net/wimax/i2400m/sdio.c
+++ b/drivers/net/wimax/i2400m/sdio.c
@@ -590,7 +590,6 @@ module_init(i2400ms_driver_init);
static
void __exit i2400ms_driver_exit(void)
{
- flush_scheduled_work(); /* for the stuff we schedule */
sdio_unregister_driver(&i2400m_sdio_driver);
}
module_exit(i2400ms_driver_exit);
diff --git a/drivers/net/wimax/i2400m/usb.c b/drivers/net/wimax/i2400m/usb.c
index d3365ac..10e3ab3 100644
--- a/drivers/net/wimax/i2400m/usb.c
+++ b/drivers/net/wimax/i2400m/usb.c
@@ -780,7 +780,6 @@ module_init(i2400mu_driver_init);
static
void __exit i2400mu_driver_exit(void)
{
- flush_scheduled_work(); /* for the stuff we schedule from sysfs.c */
usb_deregister(&i2400mu_driver);
}
module_exit(i2400mu_driver_exit);
--
1.7.1

2010-12-12 15:53:31

by Tejun Heo

[permalink] [raw]
Subject: [PATCH 9/9] hostap: don't use flush_scheduled_work()

flush_scheduled_work() is on its way out. Drop flush_scheduled_work()
from prism2_free_local_data() and replace it with explicit flushing of
work items on the respective free functions. Work items in ap_data
are flushed from hostap_free_data() and the ones in local_info from
prism2_free_local_data().

Flush is used instead of cancel as some process and free items from
queue.

Signed-off-by: Tejun Heo <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Jes Sorensen <[email protected]>
Cc: [email protected]
---
drivers/net/wireless/hostap/hostap_ap.c | 3 +++
drivers/net/wireless/hostap/hostap_hw.c | 8 +++++++-
2 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/hostap/hostap_ap.c b/drivers/net/wireless/hostap/hostap_ap.c
index dbb9869..18d63f5 100644
--- a/drivers/net/wireless/hostap/hostap_ap.c
+++ b/drivers/net/wireless/hostap/hostap_ap.c
@@ -858,7 +858,10 @@ void hostap_free_data(struct ap_data *ap)
return;
}

+ flush_work_sync(&ap->add_sta_proc_queue);
+
#ifndef PRISM2_NO_KERNEL_IEEE80211_MGMT
+ flush_work_sync(&ap->wds_oper_queue);
if (ap->crypt)
ap->crypt->deinit(ap->crypt_priv);
ap->crypt = ap->crypt_priv = NULL;
diff --git a/drivers/net/wireless/hostap/hostap_hw.c b/drivers/net/wireless/hostap/hostap_hw.c
index b7cb165..a8bddd8 100644
--- a/drivers/net/wireless/hostap/hostap_hw.c
+++ b/drivers/net/wireless/hostap/hostap_hw.c
@@ -3317,7 +3317,13 @@ static void prism2_free_local_data(struct net_device *dev)

unregister_netdev(local->dev);

- flush_scheduled_work();
+ flush_work_sync(&local->reset_queue);
+ flush_work_sync(&local->set_multicast_list_queue);
+ flush_work_sync(&local->set_tim_queue);
+#ifndef PRISM2_NO_STATION_MODES
+ flush_work_sync(&local->info_queue);
+#endif
+ flush_work_sync(&local->comms_qual_update);

lib80211_crypt_info_free(&local->crypt_info);

--
1.7.1

2010-12-12 15:53:34

by Tejun Heo

[permalink] [raw]
Subject: [PATCH 4/9] ehea: don't use flush_scheduled_work()

Directly cancel port->reset_task from ehea_shutdown_single_port()
instead. As this cancels the work for each port on driver detach,
flushing system_wq from ehea_remove() or ehea_module_exit() is no
longer necessary.

Signed-off-by: Tejun Heo <[email protected]>
Cc: Breno Leitao <[email protected]>
Cc: [email protected]
---
drivers/net/ehea/ehea_main.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ehea/ehea_main.c b/drivers/net/ehea/ehea_main.c
index d51def1..81e5b7b 100644
--- a/drivers/net/ehea/ehea_main.c
+++ b/drivers/net/ehea/ehea_main.c
@@ -3318,6 +3318,8 @@ out_err:
static void ehea_shutdown_single_port(struct ehea_port *port)
{
struct ehea_adapter *adapter = port->adapter;
+
+ cancel_work_sync(&port->reset_task);
unregister_netdev(port->netdev);
ehea_unregister_port(port);
kfree(port->mc_list);
@@ -3607,8 +3609,6 @@ static int __devexit ehea_remove(struct platform_device *dev)

ehea_remove_device_sysfs(dev);

- flush_scheduled_work();
-
ibmebus_free_irq(adapter->neq->attr.ist1, adapter);
tasklet_kill(&adapter->neq_tasklet);

@@ -3797,7 +3797,6 @@ static void __exit ehea_module_exit(void)
{
int ret;

- flush_scheduled_work();
driver_remove_file(&ehea_driver.driver, &driver_attr_capabilities);
ibmebus_unregister_driver(&ehea_driver);
unregister_reboot_notifier(&ehea_reboot_nb);
--
1.7.1

2010-12-12 15:54:27

by Tejun Heo

[permalink] [raw]
Subject: [PATCH 5/9] iseries_veth: don't use flush_scheduled_work()

flush_scheduled_work() is on its way out. Remove its usage from
iseries_veth.

* Cancelling a delayed work, queueing it for immediate execution if
cancelled and then waiting for completion can be done by simply
calling flush_delayed_work_sync().

* Explicitly cancel cnx->statemachine_wq on module unload.

Signed-off-by: Tejun Heo <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Santiago Leon <[email protected]>
Cc: [email protected]
---
drivers/net/iseries_veth.c | 18 ++++--------------
1 files changed, 4 insertions(+), 14 deletions(-)

diff --git a/drivers/net/iseries_veth.c b/drivers/net/iseries_veth.c
index 63ac531..9ece1fd 100644
--- a/drivers/net/iseries_veth.c
+++ b/drivers/net/iseries_veth.c
@@ -885,17 +885,8 @@ static void veth_stop_connection(struct veth_lpar_connection *cnx)
veth_kick_statemachine(cnx);
spin_unlock_irq(&cnx->lock);

- /* There's a slim chance the reset code has just queued the
- * statemachine to run in five seconds. If so we need to cancel
- * that and requeue the work to run now. */
- if (cancel_delayed_work(&cnx->statemachine_wq)) {
- spin_lock_irq(&cnx->lock);
- veth_kick_statemachine(cnx);
- spin_unlock_irq(&cnx->lock);
- }
-
- /* Wait for the state machine to run. */
- flush_scheduled_work();
+ /* ensure the statemachine runs now and waits for its completion */
+ flush_delayed_work_sync(&cnx->statemachine_wq);
}

static void veth_destroy_connection(struct veth_lpar_connection *cnx)
@@ -1653,15 +1644,14 @@ static void __exit veth_module_cleanup(void)
/* Disconnect our "irq" to stop events coming from the Hypervisor. */
HvLpEvent_unregisterHandler(HvLpEvent_Type_VirtualLan);

- /* Make sure any work queued from Hypervisor callbacks is finished. */
- flush_scheduled_work();
-
for (i = 0; i < HVMAXARCHITECTEDLPS; ++i) {
cnx = veth_cnx[i];

if (!cnx)
continue;

+ /* Cancel work queued from Hypervisor callbacks */
+ cancel_delayed_work_sync(&cnx->statemachine_wq);
/* Remove the connection from sysfs */
kobject_del(&cnx->kobject);
/* Drop the driver's reference to the connection */
--
1.7.1

2010-12-12 15:53:27

by Tejun Heo

[permalink] [raw]
Subject: [PATCH 7/9] sungem: update gp->reset_task flushing

gp->reset_task_pending is always set right before reset_task is
scheduled and as there is no synchronization between the setting and
scheduling, busy looping on reset_task_pending before flushing
reset_task doesn't really buy anything.

Directly flush gp->reset_task on suspend and cancel on detach.

Signed-off-by: Tejun Heo <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: [email protected]
---
drivers/net/sungem.c | 12 ++++--------
1 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/net/sungem.c b/drivers/net/sungem.c
index 4ceb3cf..9e992ca 100644
--- a/drivers/net/sungem.c
+++ b/drivers/net/sungem.c
@@ -2380,10 +2380,8 @@ static int gem_suspend(struct pci_dev *pdev, pm_message_t state)
*/
mutex_unlock(&gp->pm_mutex);

- /* Wait for a pending reset task to complete */
- while (gp->reset_task_pending)
- yield();
- flush_scheduled_work();
+ /* Wait for the pending reset task to complete */
+ flush_work_sync(&gp->reset_task);

/* Shut the PHY down eventually and setup WOL */
gem_stop_phy(gp, gp->asleep_wol);
@@ -2928,10 +2926,8 @@ static void gem_remove_one(struct pci_dev *pdev)
/* We shouldn't need any locking here */
gem_get_cell(gp);

- /* Wait for a pending reset task to complete */
- while (gp->reset_task_pending)
- yield();
- flush_scheduled_work();
+ /* Cancel reset task */
+ cancel_work_sync(&gp->reset_task);

/* Shut the PHY down */
gem_stop_phy(gp, 0);
--
1.7.1

2010-12-12 15:54:58

by Tejun Heo

[permalink] [raw]
Subject: [PATCH 1/9] drivers/net: remove unnecessary flush_scheduled_work() calls

janz-ican3, sh_eth, skge and vxge don't use workqueue at all and there
is no reason to flush the system_wq. Drop flush_scheduled_work()
calls and references to workqueue.

Signed-off-by: Tejun Heo <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Wolfgang Grandegger <[email protected]>
Cc: Stephen Hemminger <[email protected]>
Cc: Ramkrishna Vepa <[email protected]>
Cc: Sivakumar Subramani <[email protected]>
Cc: Sreenivasa Honnur <[email protected]>
Cc: Jon Mason <[email protected]>
Cc: [email protected]
---
drivers/net/can/janz-ican3.c | 9 ---------
drivers/net/sh_eth.c | 1 -
drivers/net/sh_eth.h | 1 -
drivers/net/skge.c | 2 --
drivers/net/vxge/vxge-main.c | 2 --
5 files changed, 0 insertions(+), 15 deletions(-)

diff --git a/drivers/net/can/janz-ican3.c b/drivers/net/can/janz-ican3.c
index 6e533dc..b9a6d7a 100644
--- a/drivers/net/can/janz-ican3.c
+++ b/drivers/net/can/janz-ican3.c
@@ -1114,11 +1114,6 @@ static bool ican3_txok(struct ican3_dev *mod)
/*
* Recieve one CAN frame from the hardware
*
- * This works like the core of a NAPI function, but is intended to be called
- * from workqueue context instead. This driver already needs a workqueue to
- * process control messages, so we use the workqueue instead of using NAPI.
- * This was done to simplify locking.
- *
* CONTEXT: must be called from user context
*/
static int ican3_recv_skb(struct ican3_dev *mod)
@@ -1251,7 +1246,6 @@ static irqreturn_t ican3_irq(int irq, void *dev_id)
* Reset an ICAN module to its power-on state
*
* CONTEXT: no network device registered
- * LOCKING: work function disabled
*/
static int ican3_reset_module(struct ican3_dev *mod)
{
@@ -1262,9 +1256,6 @@ static int ican3_reset_module(struct ican3_dev *mod)
/* disable interrupts so no more work is scheduled */
iowrite8(1 << mod->num, &mod->ctrl->int_disable);

- /* flush any pending work */
- flush_scheduled_work();
-
/* the first unallocated page in the DPM is #9 */
mod->free_page = DPM_FREE_START;

diff --git a/drivers/net/sh_eth.c b/drivers/net/sh_eth.c
index b12660d..819c175 100644
--- a/drivers/net/sh_eth.c
+++ b/drivers/net/sh_eth.c
@@ -1552,7 +1552,6 @@ static int sh_eth_drv_remove(struct platform_device *pdev)

sh_mdio_release(ndev);
unregister_netdev(ndev);
- flush_scheduled_work();
pm_runtime_disable(&pdev->dev);
free_netdev(ndev);
platform_set_drvdata(pdev, NULL);
diff --git a/drivers/net/sh_eth.h b/drivers/net/sh_eth.h
index 8b47763..efa6422 100644
--- a/drivers/net/sh_eth.h
+++ b/drivers/net/sh_eth.h
@@ -26,7 +26,6 @@
#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/spinlock.h>
-#include <linux/workqueue.h>
#include <linux/netdevice.h>
#include <linux/phy.h>

diff --git a/drivers/net/skge.c b/drivers/net/skge.c
index 220e039..8c1404b 100644
--- a/drivers/net/skge.c
+++ b/drivers/net/skge.c
@@ -4012,8 +4012,6 @@ static void __devexit skge_remove(struct pci_dev *pdev)
if (!hw)
return;

- flush_scheduled_work();
-
dev1 = hw->dev[1];
if (dev1)
unregister_netdev(dev1);
diff --git a/drivers/net/vxge/vxge-main.c b/drivers/net/vxge/vxge-main.c
index b771e4b..537ad87 100644
--- a/drivers/net/vxge/vxge-main.c
+++ b/drivers/net/vxge/vxge-main.c
@@ -3439,8 +3439,6 @@ static void vxge_device_unregister(struct __vxge_hw_device *hldev)

strncpy(buf, dev->name, IFNAMSIZ);

- flush_scheduled_work();
-
/* in 2.6 will call stop() if device is up */
unregister_netdev(dev);

--
1.7.1

2010-12-12 15:55:15

by Tejun Heo

[permalink] [raw]
Subject: [PATCH 2/9] drivers/net: don't use flush_scheduled_work()

flush_scheduled_work() is on its way out. This patch contains simple
conversions to replace flush_scheduled_work() usage with direct
cancels and flushes.

Directly cancel the used works on driver detach and flush them in
other cases.

The conversions are mostly straight forward and the only dangers are,

* Forgetting to cancel/flush one or more used works.

* Cancelling when a work should be flushed (ie. the work must be
executed once scheduled whether the driver is detaching or not).

I've gone over the changes multiple times but it would be much
appreciated if you can review with the above points in mind.

Signed-off-by: Tejun Heo <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Jay Cliburn <[email protected]>
Cc: Michael Chan <[email protected]>
Cc: Divy Le Ray <[email protected]>
Cc: [email protected]
Cc: Vasanthy Kolluri <[email protected]>
Cc: Samuel Ortiz <[email protected]>
Cc: Lennert Buytenhek <[email protected]>
Cc: Andrew Gallatin <[email protected]>
Cc: Francois Romieu <[email protected]>
Cc: Ramkrishna Vepa <[email protected]>
Cc: Matt Carlson <[email protected]>
Cc: David Brownell <[email protected]>
Cc: Shreyas Bhatewara <[email protected]>
Cc: [email protected]
---
drivers/net/8139too.c | 3 ++-
drivers/net/atlx/atl2.c | 4 ++--
drivers/net/bcm63xx_enet.c | 2 +-
drivers/net/bnx2.c | 4 ++--
drivers/net/cassini.c | 4 ++--
drivers/net/cxgb3/cxgb3_main.c | 3 ++-
drivers/net/e1000e/netdev.c | 6 +++---
drivers/net/enic/enic_main.c | 2 +-
drivers/net/ibm_newemac/core.c | 2 +-
drivers/net/irda/mcs7780.c | 2 +-
drivers/net/ixgb/ixgb_main.c | 2 +-
drivers/net/ixgbevf/ixgbevf_main.c | 3 +--
drivers/net/mv643xx_eth.c | 2 +-
drivers/net/myri10ge/myri10ge.c | 2 +-
drivers/net/niu.c | 2 +-
drivers/net/pxa168_eth.c | 2 +-
drivers/net/r8169.c | 2 +-
drivers/net/s2io.c | 6 ++++--
drivers/net/sis190.c | 3 ++-
drivers/net/tg3.c | 4 ++--
drivers/net/usb/sierra_net.c | 5 ++---
drivers/net/usb/usbnet.c | 3 +--
drivers/net/vmxnet3/vmxnet3_drv.c | 2 +-
23 files changed, 36 insertions(+), 34 deletions(-)

diff --git a/drivers/net/8139too.c b/drivers/net/8139too.c
index f5166dc..98517a3 100644
--- a/drivers/net/8139too.c
+++ b/drivers/net/8139too.c
@@ -1092,10 +1092,11 @@ err_out:
static void __devexit rtl8139_remove_one (struct pci_dev *pdev)
{
struct net_device *dev = pci_get_drvdata (pdev);
+ struct rtl8139_private *tp = netdev_priv(dev);

assert (dev != NULL);

- flush_scheduled_work();
+ cancel_delayed_work_sync(&tp->thread);

unregister_netdev (dev);

diff --git a/drivers/net/atlx/atl2.c b/drivers/net/atlx/atl2.c
index 35b14be..4e6f4e9 100644
--- a/drivers/net/atlx/atl2.c
+++ b/drivers/net/atlx/atl2.c
@@ -1504,8 +1504,8 @@ static void __devexit atl2_remove(struct pci_dev *pdev)

del_timer_sync(&adapter->watchdog_timer);
del_timer_sync(&adapter->phy_config_timer);
-
- flush_scheduled_work();
+ cancel_work_sync(&adapter->reset_task);
+ cancel_work_sync(&adapter->link_chg_task);

unregister_netdev(netdev);

diff --git a/drivers/net/bcm63xx_enet.c b/drivers/net/bcm63xx_enet.c
index ecfef24..e94a966 100644
--- a/drivers/net/bcm63xx_enet.c
+++ b/drivers/net/bcm63xx_enet.c
@@ -1097,7 +1097,7 @@ static int bcm_enet_stop(struct net_device *dev)
enet_dma_writel(priv, 0, ENETDMA_IRMASK_REG(priv->tx_chan));

/* make sure no mib update is scheduled */
- flush_scheduled_work();
+ cancel_work_sync(&priv->mib_update_task);

/* disable dma & mac */
bcm_enet_disable_dma(priv, priv->tx_chan);
diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
index 03209a3..5c811f3 100644
--- a/drivers/net/bnx2.c
+++ b/drivers/net/bnx2.c
@@ -8393,7 +8393,7 @@ bnx2_remove_one(struct pci_dev *pdev)
struct net_device *dev = pci_get_drvdata(pdev);
struct bnx2 *bp = netdev_priv(dev);

- flush_scheduled_work();
+ cancel_work_sync(&bp->reset_task);

unregister_netdev(dev);

@@ -8431,7 +8431,7 @@ bnx2_suspend(struct pci_dev *pdev, pm_message_t state)
if (!netif_running(dev))
return 0;

- flush_scheduled_work();
+ cancel_work_sync(&bp->reset_task);
bnx2_netif_stop(bp, true);
netif_device_detach(dev);
del_timer_sync(&bp->timer);
diff --git a/drivers/net/cassini.c b/drivers/net/cassini.c
index d6b6d6a..a8a32bc 100644
--- a/drivers/net/cassini.c
+++ b/drivers/net/cassini.c
@@ -3880,7 +3880,7 @@ static int cas_change_mtu(struct net_device *dev, int new_mtu)
schedule_work(&cp->reset_task);
#endif

- flush_scheduled_work();
+ flush_work_sync(&cp->reset_task);
return 0;
}

@@ -5177,7 +5177,7 @@ static void __devexit cas_remove_one(struct pci_dev *pdev)
vfree(cp->fw_data);

mutex_lock(&cp->pm_mutex);
- flush_scheduled_work();
+ cancel_work_sync(&cp->reset_task);
if (cp->hw_running)
cas_shutdown(cp);
mutex_unlock(&cp->pm_mutex);
diff --git a/drivers/net/cxgb3/cxgb3_main.c b/drivers/net/cxgb3/cxgb3_main.c
index 3864617..4d538a4 100644
--- a/drivers/net/cxgb3/cxgb3_main.c
+++ b/drivers/net/cxgb3/cxgb3_main.c
@@ -1359,6 +1359,7 @@ out:
static int offload_close(struct t3cdev *tdev)
{
struct adapter *adapter = tdev2adap(tdev);
+ struct t3c_data *td = T3C_DATA(tdev);

if (!test_bit(OFFLOAD_DEVMAP_BIT, &adapter->open_device_map))
return 0;
@@ -1369,7 +1370,7 @@ static int offload_close(struct t3cdev *tdev)
sysfs_remove_group(&tdev->lldev->dev.kobj, &offload_attr_group);

/* Flush work scheduled while releasing TIDs */
- flush_scheduled_work();
+ flush_work_sync(&td->tid_release_task);

tdev->lldev = NULL;
cxgb3_set_dummy_ops(tdev);
diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
index 5530d0b..02d093d 100644
--- a/drivers/net/e1000e/netdev.c
+++ b/drivers/net/e1000e/netdev.c
@@ -6028,8 +6028,8 @@ static void __devexit e1000_remove(struct pci_dev *pdev)
bool down = test_bit(__E1000_DOWN, &adapter->state);

/*
- * flush_scheduled work may reschedule our watchdog task, so
- * explicitly disable watchdog tasks from being rescheduled
+ * The timers may be rescheduled, so explicitly disable them
+ * from being rescheduled.
*/
if (!down)
set_bit(__E1000_DOWN, &adapter->state);
@@ -6040,8 +6040,8 @@ static void __devexit e1000_remove(struct pci_dev *pdev)
cancel_work_sync(&adapter->watchdog_task);
cancel_work_sync(&adapter->downshift_task);
cancel_work_sync(&adapter->update_phy_task);
+ cancel_work_sync(&adapter->led_blink_task);
cancel_work_sync(&adapter->print_hang_task);
- flush_scheduled_work();

if (!(netdev->flags & IFF_UP))
e1000_power_down_phy(adapter);
diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
index 9befd54..77d9138 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -2834,7 +2834,7 @@ static void __devexit enic_remove(struct pci_dev *pdev)
if (netdev) {
struct enic *enic = netdev_priv(netdev);

- flush_scheduled_work();
+ cancel_work_sync(&enic->reset);
unregister_netdev(netdev);
enic_dev_deinit(enic);
vnic_dev_close(enic->vdev);
diff --git a/drivers/net/ibm_newemac/core.c b/drivers/net/ibm_newemac/core.c
index 06bb9b7..8f11d29 100644
--- a/drivers/net/ibm_newemac/core.c
+++ b/drivers/net/ibm_newemac/core.c
@@ -2950,7 +2950,7 @@ static int __devexit emac_remove(struct platform_device *ofdev)

unregister_netdev(dev->ndev);

- flush_scheduled_work();
+ cancel_work_sync(&dev->reset_work);

if (emac_has_feature(dev, EMAC_FTR_HAS_TAH))
tah_detach(dev->tah_dev, dev->tah_port);
diff --git a/drivers/net/irda/mcs7780.c b/drivers/net/irda/mcs7780.c
index 74b20f1..cc821de 100644
--- a/drivers/net/irda/mcs7780.c
+++ b/drivers/net/irda/mcs7780.c
@@ -959,7 +959,7 @@ static void mcs_disconnect(struct usb_interface *intf)
if (!mcs)
return;

- flush_scheduled_work();
+ cancel_work_sync(&mcs->work);

unregister_netdev(mcs->netdev);
free_netdev(mcs->netdev);
diff --git a/drivers/net/ixgb/ixgb_main.c b/drivers/net/ixgb/ixgb_main.c
index 2e98506..b021798 100644
--- a/drivers/net/ixgb/ixgb_main.c
+++ b/drivers/net/ixgb/ixgb_main.c
@@ -527,7 +527,7 @@ ixgb_remove(struct pci_dev *pdev)
struct net_device *netdev = pci_get_drvdata(pdev);
struct ixgb_adapter *adapter = netdev_priv(netdev);

- flush_scheduled_work();
+ cancel_work_sync(&adapter->tx_timeout_task);

unregister_netdev(netdev);

diff --git a/drivers/net/ixgbevf/ixgbevf_main.c b/drivers/net/ixgbevf/ixgbevf_main.c
index 2216a3c..809e38c 100644
--- a/drivers/net/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ixgbevf/ixgbevf_main.c
@@ -3484,10 +3484,9 @@ static void __devexit ixgbevf_remove(struct pci_dev *pdev)

del_timer_sync(&adapter->watchdog_timer);

+ cancel_work_sync(&adapter->reset_task);
cancel_work_sync(&adapter->watchdog_task);

- flush_scheduled_work();
-
if (adapter->netdev_registered) {
unregister_netdev(netdev);
adapter->netdev_registered = false;
diff --git a/drivers/net/mv643xx_eth.c b/drivers/net/mv643xx_eth.c
index ce31e74..02076e1 100644
--- a/drivers/net/mv643xx_eth.c
+++ b/drivers/net/mv643xx_eth.c
@@ -2978,7 +2978,7 @@ static int mv643xx_eth_remove(struct platform_device *pdev)
unregister_netdev(mp->dev);
if (mp->phy != NULL)
phy_detach(mp->phy);
- flush_scheduled_work();
+ cancel_work_sync(&mp->tx_timeout_task);
free_netdev(mp->dev);

platform_set_drvdata(pdev, NULL);
diff --git a/drivers/net/myri10ge/myri10ge.c b/drivers/net/myri10ge/myri10ge.c
index 8524cc4..1ce0207 100644
--- a/drivers/net/myri10ge/myri10ge.c
+++ b/drivers/net/myri10ge/myri10ge.c
@@ -4067,7 +4067,7 @@ static void myri10ge_remove(struct pci_dev *pdev)
if (mgp == NULL)
return;

- flush_scheduled_work();
+ cancel_work_sync(&mgp->watchdog_work);
netdev = mgp->dev;
unregister_netdev(netdev);

diff --git a/drivers/net/niu.c b/drivers/net/niu.c
index 781e368..f64c424 100644
--- a/drivers/net/niu.c
+++ b/drivers/net/niu.c
@@ -9917,7 +9917,7 @@ static int niu_suspend(struct pci_dev *pdev, pm_message_t state)
if (!netif_running(dev))
return 0;

- flush_scheduled_work();
+ flush_work_sync(&np->reset_task);
niu_netif_stop(np);

del_timer_sync(&np->timer);
diff --git a/drivers/net/pxa168_eth.c b/drivers/net/pxa168_eth.c
index 04ed27d..1b63c8a 100644
--- a/drivers/net/pxa168_eth.c
+++ b/drivers/net/pxa168_eth.c
@@ -1602,7 +1602,7 @@ static int pxa168_eth_remove(struct platform_device *pdev)
mdiobus_unregister(pep->smi_bus);
mdiobus_free(pep->smi_bus);
unregister_netdev(dev);
- flush_scheduled_work();
+ cancel_work_sync(&pep->tx_timeout_task);
free_netdev(dev);
platform_set_drvdata(pdev, NULL);
return 0;
diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index 7d33ef4..98d792c 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -3240,7 +3240,7 @@ static void __devexit rtl8169_remove_one(struct pci_dev *pdev)
struct net_device *dev = pci_get_drvdata(pdev);
struct rtl8169_private *tp = netdev_priv(dev);

- flush_scheduled_work();
+ cancel_delayed_work_sync(&tp->task);

unregister_netdev(dev);

diff --git a/drivers/net/s2io.c b/drivers/net/s2io.c
index 9a1e32f..39c17ce 100644
--- a/drivers/net/s2io.c
+++ b/drivers/net/s2io.c
@@ -8341,9 +8341,11 @@ static void __devexit s2io_rem_nic(struct pci_dev *pdev)
return;
}

- flush_scheduled_work();
-
sp = netdev_priv(dev);
+
+ cancel_work_sync(&sp->rst_timer_task);
+ cancel_work_sync(&sp->set_link_task);
+
unregister_netdev(dev);

free_shared_mem(sp);
diff --git a/drivers/net/sis190.c b/drivers/net/sis190.c
index a5d6a6b..3406ed8 100644
--- a/drivers/net/sis190.c
+++ b/drivers/net/sis190.c
@@ -1915,9 +1915,10 @@ err_release_board:
static void __devexit sis190_remove_one(struct pci_dev *pdev)
{
struct net_device *dev = pci_get_drvdata(pdev);
+ struct sis190_private *tp = netdev_priv(dev);

sis190_mii_remove(dev);
- flush_scheduled_work();
+ cancel_work_sync(&tp->phy_task);
unregister_netdev(dev);
sis190_release_board(pdev);
pci_set_drvdata(pdev, NULL);
diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index 5faa87d..57e19fb 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -15034,7 +15034,7 @@ static void __devexit tg3_remove_one(struct pci_dev *pdev)
if (tp->fw)
release_firmware(tp->fw);

- flush_scheduled_work();
+ cancel_work_sync(&tp->reset_task);

if (tp->tg3_flags3 & TG3_FLG3_USE_PHYLIB) {
tg3_phy_fini(tp);
@@ -15073,7 +15073,7 @@ static int tg3_suspend(struct pci_dev *pdev, pm_message_t state)
if (!netif_running(dev))
return 0;

- flush_scheduled_work();
+ flush_work_sync(&tp->reset_task);
tg3_phy_stop(tp);
tg3_netif_stop(tp);

diff --git a/drivers/net/usb/sierra_net.c b/drivers/net/usb/sierra_net.c
index d1ac15c..ed1b432 100644
--- a/drivers/net/usb/sierra_net.c
+++ b/drivers/net/usb/sierra_net.c
@@ -802,10 +802,9 @@ static void sierra_net_unbind(struct usbnet *dev, struct usb_interface *intf)

dev_dbg(&dev->udev->dev, "%s", __func__);

- /* Kill the timer then flush the work queue */
+ /* kill the timer and work */
del_timer_sync(&priv->sync_timer);
-
- flush_scheduled_work();
+ cancel_work_sync(&priv->sierra_net_kevent);

/* tell modem we are going away */
status = sierra_net_send_cmd(dev, priv->shdwn_msg,
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index cff74b8..ed9a416 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1248,8 +1248,7 @@ void usbnet_disconnect (struct usb_interface *intf)
net = dev->net;
unregister_netdev (net);

- /* we don't hold rtnl here ... */
- flush_scheduled_work ();
+ cancel_work_sync(&dev->kevent);

if (dev->driver_info->unbind)
dev->driver_info->unbind (dev, intf);
diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
index 65860a9..0169be7 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -3069,7 +3069,7 @@ vmxnet3_remove_device(struct pci_dev *pdev)
#endif
num_rx_queues = 1;

- flush_scheduled_work();
+ cancel_work_sync(&adapter->work);

unregister_netdev(netdev);

--
1.7.1

2010-12-12 15:54:43

by Tejun Heo

[permalink] [raw]
Subject: [PATCH 6/9] igb[v],ixgbe: don't use flush_scheduled_work()

All three drivers use flush_scheduled_work() similarly during driver
detach. Replace it with explicit cancels.

Signed-off-by: Tejun Heo <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
drivers/net/igb/igb_main.c | 9 ++++++---
drivers/net/igbvf/netdev.c | 7 ++++---
drivers/net/ixgbe/ixgbe_main.c | 11 +++++++----
3 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/drivers/net/igb/igb_main.c b/drivers/net/igb/igb_main.c
index 041f8e6..62348fc 100644
--- a/drivers/net/igb/igb_main.c
+++ b/drivers/net/igb/igb_main.c
@@ -2050,13 +2050,16 @@ static void __devexit igb_remove(struct pci_dev *pdev)
struct igb_adapter *adapter = netdev_priv(netdev);
struct e1000_hw *hw = &adapter->hw;

- /* flush_scheduled work may reschedule our watchdog task, so
- * explicitly disable watchdog tasks from being rescheduled */
+ /*
+ * The watchdog timer may be rescheduled, so explicitly
+ * disable watchdog from being rescheduled.
+ */
set_bit(__IGB_DOWN, &adapter->state);
del_timer_sync(&adapter->watchdog_timer);
del_timer_sync(&adapter->phy_info_timer);

- flush_scheduled_work();
+ cancel_work_sync(&adapter->reset_task);
+ cancel_work_sync(&adapter->watchdog_task);

#ifdef CONFIG_IGB_DCA
if (adapter->flags & IGB_FLAG_DCA_ENABLED) {
diff --git a/drivers/net/igbvf/netdev.c b/drivers/net/igbvf/netdev.c
index 8dbde23..4fb023b 100644
--- a/drivers/net/igbvf/netdev.c
+++ b/drivers/net/igbvf/netdev.c
@@ -2825,13 +2825,14 @@ static void __devexit igbvf_remove(struct pci_dev *pdev)
struct e1000_hw *hw = &adapter->hw;

/*
- * flush_scheduled work may reschedule our watchdog task, so
- * explicitly disable watchdog tasks from being rescheduled
+ * The watchdog timer may be rescheduled, so explicitly
+ * disable it from being rescheduled.
*/
set_bit(__IGBVF_DOWN, &adapter->state);
del_timer_sync(&adapter->watchdog_timer);

- flush_scheduled_work();
+ cancel_work_sync(&adapter->reset_task);
+ cancel_work_sync(&adapter->watchdog_task);

unregister_netdev(netdev);

diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
index 8af0fc0..ca9036d 100644
--- a/drivers/net/ixgbe/ixgbe_main.c
+++ b/drivers/net/ixgbe/ixgbe_main.c
@@ -7373,13 +7373,15 @@ static void __devexit ixgbe_remove(struct pci_dev *pdev)
struct net_device *netdev = adapter->netdev;

set_bit(__IXGBE_DOWN, &adapter->state);
- /* clear the module not found bit to make sure the worker won't
- * reschedule
+
+ /*
+ * The timers may be rescheduled, so explicitly disable them
+ * from being rescheduled.
*/
clear_bit(__IXGBE_SFP_MODULE_NOT_FOUND, &adapter->state);
del_timer_sync(&adapter->watchdog_timer);
-
del_timer_sync(&adapter->sfp_timer);
+
cancel_work_sync(&adapter->watchdog_task);
cancel_work_sync(&adapter->sfp_task);
cancel_work_sync(&adapter->multispeed_fiber_task);
@@ -7387,7 +7389,8 @@ static void __devexit ixgbe_remove(struct pci_dev *pdev)
if (adapter->flags & IXGBE_FLAG_FDIR_HASH_CAPABLE ||
adapter->flags & IXGBE_FLAG_FDIR_PERFECT_CAPABLE)
cancel_work_sync(&adapter->fdir_reinit_task);
- flush_scheduled_work();
+ if (adapter->flags2 & IXGBE_FLAG2_TEMP_SENSOR_CAPABLE)
+ cancel_work_sync(&adapter->check_overtemp_task);

#ifdef CONFIG_IXGBE_DCA
if (adapter->flags & IXGBE_FLAG_DCA_ENABLED) {
--
1.7.1

2010-12-12 22:37:58

by David Miller

[permalink] [raw]
Subject: Re: [PATCHSET] net-dev: don't use flush_scheduled_work()

From: Tejun Heo <[email protected]>
Date: Sun, 12 Dec 2010 16:52:56 +0100

> This patchset removes the use of flush_scheduled_work(), which is
> being deprecated, from various network drivers. It contains the
> following nine patches.
>
> 0001-drivers-net-remove-unnecessary-flush_scheduled_work-.patch
> 0002-drivers-net-don-t-use-flush_scheduled_work.patch
> 0003-ehea-kill-unused-ehea_rereg_mr_task.patch
> 0004-ehea-don-t-use-flush_scheduled_work.patch
> 0005-iseries_veth-don-t-use-flush_scheduled_work.patch
> 0006-igb-v-ixgbe-don-t-use-flush_scheduled_work.patch
> 0007-sungem-update-gp-reset_task-flushing.patch
> 0008-i2400m-drop-i2400m_schedule_work.patch
> 0009-hostap-don-t-use-flush_scheduled_work.patch
>
> 0001 and 0002 are straight forward conversions across multiple network
> drivers. The rest are a bit more involved per-driver or driver family
> conversions.
>
> This patchset is on top of the current net-next-2.6.git#master
> (ad1184c6cf067a13e8cb2a4e7ccc407f947027d0) and available in the
> following git branch.
>
> git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git net-dev-kill-flush_scheduled_work
>
> diffstat follows. Thanks.

I spent the afternoon going over these and they look good to me.
Pulled, thanks a lot.

If any follow-up bug fixes show up just send me relative patches,
thanks again!

2010-12-13 11:22:53

by Lennert Buytenhek

[permalink] [raw]
Subject: Re: [PATCH 2/9] drivers/net: don't use flush_scheduled_work()

On Sun, Dec 12, 2010 at 04:52:58PM +0100, Tejun Heo wrote:

> flush_scheduled_work() is on its way out. This patch contains simple
> conversions to replace flush_scheduled_work() usage with direct
> cancels and flushes.
>
> Directly cancel the used works on driver detach and flush them in
> other cases.
>
> The conversions are mostly straight forward and the only dangers are,
>
> * Forgetting to cancel/flush one or more used works.
>
> * Cancelling when a work should be flushed (ie. the work must be
> executed once scheduled whether the driver is detaching or not).
>
> I've gone over the changes multiple times but it would be much
> appreciated if you can review with the above points in mind.
>
> Signed-off-by: Tejun Heo <[email protected]>
> Cc: "David S. Miller" <[email protected]>
> Cc: Jay Cliburn <[email protected]>
> Cc: Michael Chan <[email protected]>
> Cc: Divy Le Ray <[email protected]>
> Cc: [email protected]
> Cc: Vasanthy Kolluri <[email protected]>
> Cc: Samuel Ortiz <[email protected]>
> Cc: Lennert Buytenhek <[email protected]>
> Cc: Andrew Gallatin <[email protected]>
> Cc: Francois Romieu <[email protected]>
> Cc: Ramkrishna Vepa <[email protected]>
> Cc: Matt Carlson <[email protected]>
> Cc: David Brownell <[email protected]>
> Cc: Shreyas Bhatewara <[email protected]>
> Cc: [email protected]

For the mv643xx_eth part:

Acked-by: Lennert Buytenhek <[email protected]>

2010-12-13 17:13:28

by Michael Chan

[permalink] [raw]
Subject: Re: [PATCH 2/9] drivers/net: don't use flush_scheduled_work()


On Sun, 2010-12-12 at 07:52 -0800, Tejun Heo wrote:
> flush_scheduled_work() is on its way out. This patch contains simple
> conversions to replace flush_scheduled_work() usage with direct
> cancels and flushes.
>
> Directly cancel the used works on driver detach and flush them in
> other cases.
>
> The conversions are mostly straight forward and the only dangers are,
>
> * Forgetting to cancel/flush one or more used works.
>
> * Cancelling when a work should be flushed (ie. the work must be
> executed once scheduled whether the driver is detaching or not).
>
> I've gone over the changes multiple times but it would be much
> appreciated if you can review with the above points in mind.
>
> Signed-off-by: Tejun Heo <[email protected]>
> Cc: "David S. Miller" <[email protected]>
> Cc: Jay Cliburn <[email protected]>
> Cc: Michael Chan <[email protected]>
> Cc: Divy Le Ray <[email protected]>
> Cc: [email protected]
> Cc: Vasanthy Kolluri <[email protected]>
> Cc: Samuel Ortiz <[email protected]>
> Cc: Lennert Buytenhek <[email protected]>
> Cc: Andrew Gallatin <[email protected]>
> Cc: Francois Romieu <[email protected]>
> Cc: Ramkrishna Vepa <[email protected]>
> Cc: Matt Carlson <[email protected]>
> Cc: David Brownell <[email protected]>
> Cc: Shreyas Bhatewara <[email protected]>
> Cc: [email protected]
> ---

> diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
> index 03209a3..5c811f3 100644
> --- a/drivers/net/bnx2.c
> +++ b/drivers/net/bnx2.c
> @@ -8393,7 +8393,7 @@ bnx2_remove_one(struct pci_dev *pdev)
> struct net_device *dev = pci_get_drvdata(pdev);
> struct bnx2 *bp = netdev_priv(dev);
>
> - flush_scheduled_work();
> + cancel_work_sync(&bp->reset_task);

I think we can just remove flush_scheduled_work() altogether in
bnx2_remove_one(). The work has already been canceled in bnx2_close(),
so there is no possibility of it to be scheduled at this point.

>
> unregister_netdev(dev);
>
> @@ -8431,7 +8431,7 @@ bnx2_suspend(struct pci_dev *pdev, pm_message_t
> state)
> if (!netif_running(dev))
> return 0;
>
> - flush_scheduled_work();
> + cancel_work_sync(&bp->reset_task);

This part ok. Thanks.

Acked-by: Michael Chan <[email protected]>

> bnx2_netif_stop(bp, true);
> netif_device_detach(dev);
> del_timer_sync(&bp->timer);

2010-12-13 21:27:53

by Andrew Gallatin

[permalink] [raw]
Subject: Re: [PATCH 2/9] drivers/net: don't use flush_scheduled_work()

On 12/12/10 10:52, Tejun Heo wrote:
> flush_scheduled_work() is on its way out. This patch contains simple
> conversions to replace flush_scheduled_work() usage with direct
> cancels and flushes.
>
> Directly cancel the used works on driver detach and flush them in
> other cases.
>
> The conversions are mostly straight forward and the only dangers are,
>
> * Forgetting to cancel/flush one or more used works.
>
> * Cancelling when a work should be flushed (ie. the work must be
> executed once scheduled whether the driver is detaching or not).
>
> I've gone over the changes multiple times but it would be much
> appreciated if you can review with the above points in mind.
>
> Signed-off-by: Tejun Heo<[email protected]>
> Cc: "David S. Miller"<[email protected]>
> Cc: Jay Cliburn<[email protected]>
> Cc: Michael Chan<[email protected]>
> Cc: Divy Le Ray<[email protected]>
> Cc: [email protected]
> Cc: Vasanthy Kolluri<[email protected]>
> Cc: Samuel Ortiz<[email protected]>
> Cc: Lennert Buytenhek<[email protected]>
> Cc: Andrew Gallatin<[email protected]>
> Cc: Francois Romieu<[email protected]>
> Cc: Ramkrishna Vepa<[email protected]>
> Cc: Matt Carlson<[email protected]>
> Cc: David Brownell<[email protected]>
> Cc: Shreyas Bhatewara<[email protected]>
> Cc: [email protected]
> ---


For the myri10ge part:

Acked-by: Andrew Gallatin <[email protected]>

2010-12-15 00:54:42

by Jon Mason

[permalink] [raw]
Subject: Re: [PATCH 1/9] drivers/net: remove unnecessary flush_scheduled_work() calls

On Sun, Dec 12, 2010 at 07:52:57AM -0800, Tejun Heo wrote:
> janz-ican3, sh_eth, skge and vxge don't use workqueue at all and there
> is no reason to flush the system_wq. Drop flush_scheduled_work()
> calls and references to workqueue.

I believe you were referencing an older version of the vxge driver.
These is now a reset task that runs on the system workqueue. Based on
your other patches, the flush_scheduled_work should be replaced with a
cancel_delayed_work_sync(&vdev->reset_task)

Thanks,
Jon

>
> Signed-off-by: Tejun Heo <[email protected]>
> Cc: "David S. Miller" <[email protected]>
> Cc: Wolfgang Grandegger <[email protected]>
> Cc: Stephen Hemminger <[email protected]>
> Cc: Ramkrishna Vepa <[email protected]>
> Cc: Sivakumar Subramani <[email protected]>
> Cc: Sreenivasa Honnur <[email protected]>
> Cc: Jon Mason <[email protected]>
> Cc: [email protected]
> ---
> drivers/net/can/janz-ican3.c | 9 ---------
> drivers/net/sh_eth.c | 1 -
> drivers/net/sh_eth.h | 1 -
> drivers/net/skge.c | 2 --
> drivers/net/vxge/vxge-main.c | 2 --
> 5 files changed, 0 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/net/can/janz-ican3.c b/drivers/net/can/janz-ican3.c
> index 6e533dc..b9a6d7a 100644
> --- a/drivers/net/can/janz-ican3.c
> +++ b/drivers/net/can/janz-ican3.c
> @@ -1114,11 +1114,6 @@ static bool ican3_txok(struct ican3_dev *mod)
> /*
> * Recieve one CAN frame from the hardware
> *
> - * This works like the core of a NAPI function, but is intended to be called
> - * from workqueue context instead. This driver already needs a workqueue to
> - * process control messages, so we use the workqueue instead of using NAPI.
> - * This was done to simplify locking.
> - *
> * CONTEXT: must be called from user context
> */
> static int ican3_recv_skb(struct ican3_dev *mod)
> @@ -1251,7 +1246,6 @@ static irqreturn_t ican3_irq(int irq, void *dev_id)
> * Reset an ICAN module to its power-on state
> *
> * CONTEXT: no network device registered
> - * LOCKING: work function disabled
> */
> static int ican3_reset_module(struct ican3_dev *mod)
> {
> @@ -1262,9 +1256,6 @@ static int ican3_reset_module(struct ican3_dev *mod)
> /* disable interrupts so no more work is scheduled */
> iowrite8(1 << mod->num, &mod->ctrl->int_disable);
>
> - /* flush any pending work */
> - flush_scheduled_work();
> -
> /* the first unallocated page in the DPM is #9 */
> mod->free_page = DPM_FREE_START;
>
> diff --git a/drivers/net/sh_eth.c b/drivers/net/sh_eth.c
> index b12660d..819c175 100644
> --- a/drivers/net/sh_eth.c
> +++ b/drivers/net/sh_eth.c
> @@ -1552,7 +1552,6 @@ static int sh_eth_drv_remove(struct platform_device *pdev)
>
> sh_mdio_release(ndev);
> unregister_netdev(ndev);
> - flush_scheduled_work();
> pm_runtime_disable(&pdev->dev);
> free_netdev(ndev);
> platform_set_drvdata(pdev, NULL);
> diff --git a/drivers/net/sh_eth.h b/drivers/net/sh_eth.h
> index 8b47763..efa6422 100644
> --- a/drivers/net/sh_eth.h
> +++ b/drivers/net/sh_eth.h
> @@ -26,7 +26,6 @@
> #include <linux/module.h>
> #include <linux/kernel.h>
> #include <linux/spinlock.h>
> -#include <linux/workqueue.h>
> #include <linux/netdevice.h>
> #include <linux/phy.h>
>
> diff --git a/drivers/net/skge.c b/drivers/net/skge.c
> index 220e039..8c1404b 100644
> --- a/drivers/net/skge.c
> +++ b/drivers/net/skge.c
> @@ -4012,8 +4012,6 @@ static void __devexit skge_remove(struct pci_dev *pdev)
> if (!hw)
> return;
>
> - flush_scheduled_work();
> -
> dev1 = hw->dev[1];
> if (dev1)
> unregister_netdev(dev1);
> diff --git a/drivers/net/vxge/vxge-main.c b/drivers/net/vxge/vxge-main.c
> index b771e4b..537ad87 100644
> --- a/drivers/net/vxge/vxge-main.c
> +++ b/drivers/net/vxge/vxge-main.c
> @@ -3439,8 +3439,6 @@ static void vxge_device_unregister(struct __vxge_hw_device *hldev)
>
> strncpy(buf, dev->name, IFNAMSIZ);
>
> - flush_scheduled_work();
> -
> /* in 2.6 will call stop() if device is up */
> unregister_netdev(dev);
>
> --
> 1.7.1
>

The information and any attached documents contained in this message
may be confidential and/or legally privileged. The message is
intended solely for the addressee(s). If you are not the intended
recipient, you are hereby notified that any use, dissemination, or
reproduction is strictly prohibited and may be unlawful. If you are
not the intended recipient, please contact the sender immediately by
return e-mail and destroy all copies of the original message.