On Mon, May 26, 2014 at 10:35 AM, Daniel Vetter daniel@ffwll.ch wrote:
On Mon, May 26, 2014 at 07:56:50AM -0400, Rob Clark wrote:
On Mon, May 26, 2014 at 4:23 AM, Daniel Vetter daniel@ffwll.ch wrote:
On Sun, May 25, 2014 at 07:16:43PM -0400, Rob Clark wrote:
On Sun, May 25, 2014 at 6:10 PM, Daniel Vetter daniel@ffwll.ch wrote:
On Sat, May 24, 2014 at 8:30 PM, Rob Clark robdclark@gmail.com wrote:
@@ -8002,7 +8002,7 @@ bool intel_get_load_detect_pipe(struct drm_connector *connector, if (encoder->crtc) { crtc = encoder->crtc;
mutex_lock(&crtc->mutex);
drm_modeset_lock(&crtc->mutex, NULL);This is pretty much the reason why I think switching the mode_config.mutex to a ww_mutex is a bad idea: This call here nests within the mode_config.mutex and so must be acquired. Wiring the acquire context through everything is going to be fairly horrible, especially since you must be able to bail out when trying to lock with an axquire context.
which is the call-path to here from mode_config.mutex? Is it possible to just move the locking to a higher level for a drm_modeset_lock_all()?
Connector probing. And the entire point of crtc locks was to _not_ block all screen updates while we poke for a new edid or do load balancing. If you want to test this you need a gen3/4 with tv-out (native, not through sdvo) or a gen2 or i915g/gm with vga.
hmm, I guess I'm still not quite seeing the issue. For non-atomic paths, we are grabbing mode_config and/or crtc mutex as bare mutexes in same spots as we did before. So if it worked before without nested_lock stuff it should still work now.
Thread A is doing output probing.
Thread B is doing atomic modesetGrabs mode_config.mutex
Grabs crtc_A->ww_mutexTries to grab crtc_A->ww_mutex, blocks since normal ww_mutex_lock
Tries to grab mode_config.mutex with ww acuiquire context, blocks since current holder hasn't acquired the mutex with a ww ticket
hmm, ok, I had thought this case, thread B would get -EDEADLK because lock was held, and not his acquire ctx. If that is not the case, then I would propose this:
All places doing things the old way, must grab mode_config.mutex first currently. And we use mode_config.mutex to protect mode_config.acquire_ctx. So all the lower spots grabbing individual crtc mutexes can safely use mode_config.acquire_ctx.
Then the only headache is propagating -EDEADLK up the call stack. If we are lucky, the all already propagate -EINTR, etc.
BR, -R
-> Deadlock.
You really can't mix lock nesting with w/w and lock nesting with a static hierarchy. It's all or nothing.
-Daniel
Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch