Note to all: please use this address when copying me regarding
upstream matters. It keeps my mailboxes better sorted...thanks!
Since I'm being called-out by name, I suppose I should add my
$0.02... :-)
On Thu, Aug 25, 2005 at 04:38:35PM -0400, Jeff Garzik wrote:
> Malli Chilakala wrote:
> >Backout watchdog task patch from John Linville
> Rejected for two reasons:
>
> 1) No explanation why this back-out is needed.
The patch that created the e1000_watchdog_task inadvertantly created
a synchronization problem. e1000_down calls del_timer_sync on the
watchdog_timer. Code later in that path executes under the presumption
that the code which had been part of the watchdog_timer handler (but
is now in e1000_watchdog_task) is not running. Since that code is now
part of e1000_watchdog_task apparently there is a window in which the
del_timer_sync can return before e1000_watchdog_task actually runs,
and in some cases (SMP?) that can lead to locking/corruption problems.
> 2) You should fix problems rather than running from them. We want as
> much slow path code as possible -- such as reset or phy code -- to be in
> process context.
Although I did consent to backing-out the patch, I am inclined to
agree with Jeff for the long term.
It seems like we tried adding a flush_workqueue after the
del_timer_sync, but that was somehow insufficient? Ganesh, do you
remember?
Ganesh, John, Malli, etc., I'll be happy to work with you guys on this
one if you'd like to figure-out how to make this work and please Jeff.
We need to do something, as there is a real problem there in the
current upstream code.
John
P.S. Why would this patch not work?
diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -352,6 +352,7 @@ e1000_down(struct e1000_adapter *adapter
#endif
del_timer_sync(&adapter->tx_fifo_stall_timer);
del_timer_sync(&adapter->watchdog_timer);
+ flush_workqueue(&adapter->watchdog_task);
del_timer_sync(&adapter->phy_info_timer);
#ifdef CONFIG_E1000_NAPI
--
John W. Linville
[EMAIL PROTECTED]
-
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
