On Tue, Sep 9, 2014 at 3:07 AM, Tomi Valkeinen <tomi.valkeinen@xxxxxx> wrote: > >>>> lockdep won't complain though since you essentially open-code a >>>> workqueue_flush, and lockdep also doesn't complain about all possible >>>> deadlocks (due to some design issues with lockdep). >>> >>> What do you mean "open-code a workqueue_flush"?. I use flush_workqueue >>> there. We have two things to wait for: work on the workqueue and work >>> which is triggered by the vsync irq. So we loop and test for both of >>> those, until there's no more work. >> >> Oops, missed that. Ordering looks wrong though since if the irq can latch >> the workqueue you need to wait for irqs to happen first before flushing. >> And obviously queue the work before signalling the completion of the >> interrupt. But since this seems to lack locking anyway and is only used >> for unload it doesn't really matter. from a quick look at omap_crtc_flush(), you probably also race w/ apply worker list manipulation (like moving from queued_applies to pending_applies) > Yeah, well, the workqueue can create work for the irq also. I don't know > if it does, currently, but I think it's safer to presume that both > workqueue and the irq can create work to the other. > > But that's why I have a loop there. So we flush, then check if there is > work for the irq. If yes, sleep a bit and go back to start. So if the > irq work created new work for the wq, we flush that. And if that work > created new work for the irq, we check that again. Etc. I think adding an internal per-crtc apply_lock would solve a lot of problems. I was originally shying away from recommending that, mostly because I didn't want to think about it and I though there was an easier solution. But with an apply_lock we could at least add some locking to _flush() and make it not racy as well.. Although, I wonder if some waitqueue scheme would be a bit more sane.. ie. end of apply_worker, if there is nothing more queued up, signal the event that _flush() is waiting on. BR, -R _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
