From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [Intel-gfx] [PATCH] drm: Assert correct locking for drm_send_vblank_event Date: Fri, 12 Sep 2014 19:25:31 +0200 Message-ID: <20140912172531.GI4740@phenom.ffwll.local> References: <1410529256-8602-1-git-send-email-daniel.vetter@ffwll.ch> <20140912152329.GI16043@nuc-i3427.alporthouse.com> <20140912153456.GH4740@phenom.ffwll.local> <20140912160442.GA14793@nuc-i3427.alporthouse.com> <54132777.5080804@hurleysoftware.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-wi0-f172.google.com (mail-wi0-f172.google.com [209.85.212.172]) by gabe.freedesktop.org (Postfix) with ESMTP id 2CF2E6E751 for ; Fri, 12 Sep 2014 10:25:10 -0700 (PDT) Received: by mail-wi0-f172.google.com with SMTP id e4so1036954wiv.17 for ; Fri, 12 Sep 2014 10:25:09 -0700 (PDT) Content-Disposition: inline In-Reply-To: <54132777.5080804@hurleysoftware.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Peter Hurley Cc: Daniel Vetter , Intel Graphics Development , dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org On Fri, Sep 12, 2014 at 01:03:51PM -0400, Peter Hurley wrote: > On 09/12/2014 12:04 PM, Chris Wilson wrote: > > On Fri, Sep 12, 2014 at 05:34:56PM +0200, Daniel Vetter wrote: > >> On Fri, Sep 12, 2014 at 04:23:29PM +0100, Chris Wilson wrote: > >>> On Fri, Sep 12, 2014 at 03:40:56PM +0200, Daniel Vetter wrote: > >>>> The comment says that the caller must hold the dev->event_lock > >>>> spinlock, so let's enforce this. > >>>> > >>>> A quick audit over all driver shows that except for the one place in > >>>> i915 which motivated this all callers fullfill this requirement > >>>> already. > >>> > >>> Replace the rogue WARN_ON_SMP(!spin_is_locked(&dev->event_lock)) in > >>> send_vblank_event() as well then. > >> > >> Meh, I've missed that one, that's actually better I think. I'll drop my > >> patch here. > > > > I thought assert_spin_lock was the preferred form? > > Actually, lockdep_assert_held() is the preferred form. > > See https://lkml.org/lkml/2014/9/3/171 Which unfortunately doesn't warn for all the normal users which are not insane enough to enable lockdep and so is totally useless to validate a driver that runs on metric piles of different chips (with a resulting combinatorial explosion of code-paths because hw designers are creative). And we rely a lot on random drive-by testers to report such issues. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch