On Tue, 2009-10-13 at 17:25 -0700, Joe Eykholt wrote: > I see. What about the lock checking code, though? It might still > print a warning about that. Maybe there's a way of quieting it. >
Good point, even if these could be suppressed it should be avoided as this helps in finding real issues. > >> That's a real problem since the timeout handler may be trying to > get > >> the lock and we would wait for the handler to finish. > >> > > > > As long as upper blocks of EM doesn't tries to acquire their lock on > > EX_CLOSED event we are okay and current code seems to not doing > this > > anymore. > > > >> Another way might be to use a separate work queue for exchange > timeouts, > >> and flush all work on that queue synchronously before freeing an > >> exchange manager. > >> > > > > I tried that fix before and posted here for RFC > > http://www.open-fcoe.org/pipermail/devel/2009-August/003743.html > > > > Yeah this should also work fine. I think my test code had issue in > > testing above RFC patch as I was skipping cancel_delayed_work() in > my > > test while expecting flush_workqueue to clean up delayed scheduled > work > > still sitting in timer queue. Let me know if above fix works for > you. I > > also done with changes for fixing using _sync. > > > > I think fix 1 is better in above RFC patch since other fix with > _sync > > adds more constraint on upper block to not acquire lock again and > > required dropping _bh in acquiring exch lock in fc_exch_reset since > > _sync might sleep. > > Not only that, but the ex_lock *is* taken by fc_exch_timeout() and > is held while we're calling cancel_delayed_work_sync(), so that's > an issue. > Yeah that would be bigger issue, again good point. > > Which one do you think is better fix ? > > I agree the RFC fix is better. Sounds good, I'll provide this fix with current tip after some more testing. Thanks Vasu _______________________________________________ devel mailing list [email protected] http://www.open-fcoe.org/mailman/listinfo/devel
