On Thu, Feb 15, 2007 at 03:45:23PM -0800, Jay Vosburgh wrote: > > For the short term, yes, I don't have any disagreement with > switching the timer based stuff over to workqueues. Basically a one for > one replacement to get the functions in a process context and tweak the > locking.
I did some testing of mine last week and my patch definitely has some issues. I'm running into a problem that is similar to the thread started last week titled "BUG] RTNL and flush_scheduled_work deadlocks" but I think I can patch around that if needed. > > I do think we're having a little confusion over details of > terminology; if I'm not mistaken, you're thinking that workqueue means > single threaded: even though each individual "monitor thingie" is a > separate piece of work, they still can't collide. > > That's true, but (unless I've missed a call somewhere) there > isn't a "wq_pause_for_a_bit" type of call (that, e.g., waits for > anything running to stop, then doesn't run any further work until we > later tell it to), so suspending all of the periodic things running for > the bond is more hassle than if there's just one schedulable work thing, > which internally calls the right functions to do the various things. > This is also single threaded, but easier to stop and start. It seems to > be simpler to have multiple link monitors running in such a system as > well (without having them thrashing the link state as would happen now). > I see by looking at your patch that you keep a list of timers and only schedule work for the event that will happen next. I've seen timer implementations like this before and feel its reasonable. It would be good to account for skew, but other than that it seems like a reasonable solution -- though it is too bad that workqueues and their behavior seem like somewhat of a mystery to most and cause people to code around them (I don't blame you one bit). I also plan to start testing your patch later this week and will let you know what I find. -andy - 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
