On 09/07/2014 05:02 PM, Rob Clark wrote: > On Fri, Sep 5, 2014 at 8:25 AM, Daniel Vetter <daniel@xxxxxxxx> wrote: >> On Fri, Sep 05, 2014 at 07:59:45AM -0400, Rob Clark wrote: >>> While in real life, we could never fail to grab the newly created mutex, >>> ww_mutex fault injection has no way to know this. Which could result >>> that kernels built with CONFIG_DEBUG_WW_MUTEX_SLOWPATH=y might fail to >>> acquire the new crtc lock. Which results in bad things when the locks >>> are dropped. >>> >>> See: https://bugzilla.kernel.org/show_bug.cgi?id=83341 >>> >>> Signed-off-by: Rob Clark <robdclark@xxxxxxxxx> >>> --- >>> drivers/gpu/drm/drm_crtc.c | 10 +++++++++- >>> 1 file changed, 9 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c >>> index 7d7c1fd..8bb11fa 100644 >>> --- a/drivers/gpu/drm/drm_crtc.c >>> +++ b/drivers/gpu/drm/drm_crtc.c >>> @@ -682,7 +682,15 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc, >>> drm_modeset_lock_all(dev); >>> drm_modeset_lock_init(&crtc->mutex); >>> /* dropped by _unlock_all(): */ >>> - drm_modeset_lock(&crtc->mutex, config->acquire_ctx); >>> + /* NOTE: use trylock here for the benefit of ww_mutex fault >>> + * injection. We cannot actually fail to grab this lock (as >>> + * it has only just been created), but fault injection does >>> + * not know this, which can result in the this lock failing, >>> + * and hilarity when we later try to drop the locks. See: >>> + * https://bugzilla.kernel.org/show_bug.cgi?id=83341 >>> + */ >>> + ret = ww_mutex_trylock(&crtc->mutex.mutex); >>> + WARN_ON(ret); >> Hm, I've thought on our quick discussion on irc we've agreed that the >> locking here in the init path is useless anyway and best dropped? Not just >> remove the crtc locking, but the entire modeset_lock_all. > well, 0day appears to disagree with you.. I still think we should go > the trylock route for 3.17, as it is more the more conservative patch. > > I'm not against getting rid of that locking (which is in fact > overkill) once the other fallout is fixed up. But that seems more > like a merge-window thing, so probably best to wait for 3.18. > > BR, > -R > FWIW, Reviewed-by: Thomas Hellstrom <thellstrom@xxxxxxxxxx> >> -Daniel >> >>> ret = drm_mode_object_get(dev, &crtc->base, DRM_MODE_OBJECT_CRTC); >>> if (ret) >>> -- >>> 1.9.3 >>> >> -- >> Daniel Vetter >> Software Engineer, Intel Corporation >> +41 (0) 79 365 57 48 - http://blog.ffwll.ch > _______________________________________________ > dri-devel mailing list > dri-devel@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/dri-devel _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
