Re: [PATCH 03/17] drm: convert crtc and mode_config to ww_mutex

[Daniel Vetter <daniel@ffwll.ch>]

On Mon, May 26, 2014 at 11:20:49AM -0400, Rob Clark wrote:
> On Mon, May 26, 2014 at 11:07 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Mon, May 26, 2014 at 5:04 PM, Rob Clark <robdclark@gmail.com> wrote:
> >> 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.
> >
> > The output poll work most definitely doesn't propagate -EINTR. Like
> > I've said, this will be painful. And imo doing this also makes the kms
> > locking into quite a mess overall.
> 
> Well, we could hold mode_config.mutex as a traditional mutex around
> atomic operations.  What you loose out would be now _NONBLOCK
> operations could conceivable call into driver paths without
> mode_config.mutex held.  This was the advantage of converting
> mode_config.mutex as well.  Granted, it is slightly theoretical
> because until we expose atomic ioctl it would only apply to page_flip
> (which was not holding mode_config.mutex).  And we also want to get
> rid of mode_config.mutex in these paths too.
> 
> Otoh, if we want to make locking more fine grained, more use of
> ww_mutex seems like the best way.  And if that means adding a return
> value to a fxn here/there and propagating errors properly, maybe we
> should just go ahead and do that.  It sounds like the right long term
> solution anyways.

Yeah, I'm starting to lean towards trying to elide mode_config.mutex
completely from the atomic paths (and modesets in general). I think the
only bits we really need is adding ww mutexes to planes _and_ to
connectors. The atomic would _only_ ever acquire ww mutexes, and we would
be able to guarante that most of them are only held short times so that we
don't need to bother with trylocking them for NONBLOCK. That should simply
the atomic logic a bit I hope.

So that, and a full subsystem audit unfortunately :(
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch