On 4/21/06, Michael Chan <[EMAIL PROTECTED]> wrote: > On Fri, 2006-04-21 at 09:27 -0400, Andy Gospodarek wrote: > > > > > Isn't the only possibility for a linkwatch deadlock when the > > __LINK_STATE_LINKWATCH_PENDING but is set in dev->state? > > This device that you're about to close may be on the linkwatch list, or > other devices may also be on the linkwatch list. As long as > linkwatch_event is in front of your driver's task on the same workqueue, > it will deadlock.
My thought was that you would only try and service the linkwatch queue if the device you are closing has the __LINK_STATE_LINKWATCH_PENDING bit set in that device's dev->state. I agree there still could be problems with ordering of events on the workqueue, so it may make sense to use a separate workqueue for linkwatch events if we can't come up with a more consistent way to call flush_scheduled_work across all drivers. I just hate to see extra resources used to solve problems that good coding can solve (not that my suggestion is necessarily a 'good' one), so I was trying to think of a way to resolve this without explicitly adding another workqueue. > > > > > Off the top of my head... > > > > Would it be interesting to change the calls for flush_scheduled_work() > > to a new function net_flush_scheduled_work() with the intent on > > eventually creating new a new work queue but temporarily just checking > > to make sure there are no linkwatch events pending and if there are > > allowing them to run first before calling flush_scheduled_work()? > > > > Using the same workqueue and holding the rtnl_lock, I'm not sure how you > can let linkwatch_event run first without deadlocking. > > You wouldn't call linkwatch_event, you would call linkwatch_run_queue (with special care taken to make sure you take the rtnl_lock if needed). See a similar example for calling linkwatch_run_queue in netdev_wait_allrefs. - 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
