From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Subject: Re: [PATCH 05/18] drm/irq: remove cargo-culted locking from irq_install/unistall Date: Thu, 17 Apr 2014 16:44:44 +0200 Message-ID: <20140417144426.GE550@ulmo> References: <1397252175-14227-1-git-send-email-daniel.vetter@ffwll.ch> <1397252175-14227-6-git-send-email-daniel.vetter@ffwll.ch> <20140417144316.GD550@ulmo> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1199791712==" Return-path: Received: from mail-ee0-f45.google.com (mail-ee0-f45.google.com [74.125.83.45]) by gabe.freedesktop.org (Postfix) with ESMTP id 8AADA6EB50 for ; Thu, 17 Apr 2014 07:45:52 -0700 (PDT) Received: by mail-ee0-f45.google.com with SMTP id d17so766222eek.4 for ; Thu, 17 Apr 2014 07:45:51 -0700 (PDT) In-Reply-To: <20140417144316.GD550@ulmo> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Daniel Vetter Cc: DRI Development List-Id: dri-devel@lists.freedesktop.org --===============1199791712== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="mR8QP4gmHujQHb1c" Content-Disposition: inline --mR8QP4gmHujQHb1c Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Apr 17, 2014 at 04:43:17PM +0200, Thierry Reding wrote: > On Fri, Apr 11, 2014 at 11:36:02PM +0200, Daniel Vetter wrote: > > The dev->struct_mutex locking in drm_irq.c only protects > > dev->irq_enabled. Which isn't really much at all and only prevents > > especially nasty ums userspace from concurrently installing the > > interrupt handling a few times. Or at least trying. > >=20 > > There are tons of unlocked readers of dev->irqs_enabled in the vblank > > wait code (and by extension also in the pageflip code since that uses > > the same vblank timestamp engine). > >=20 > > Real modesetting drivers should ensure that nothing can go haywire > > with a sane setup teardown sequence. So we only really need this for > > the drm_control ioctl, everywhere else this will just paper over > > nastiness. > >=20 > > Note that drm/i915 is a bit specially due to the gem+ums combination. > > So there we also need to properly protect the entervt and leavevt > > ioctls. But it's definitely saner to do everything in one go than to > > drop the lock in-between. > >=20 > > Finally there's the gpu reset code in drm/i915. That one's just race > > (concurrent userspace calls to for vblank waits of pageflips could > > spuriously fail). So wrap it up in with a nice comment since fixing > > this is more involved. > >=20 > > Signed-off-by: Daniel Vetter > > --- > > drivers/gpu/drm/drm_irq.c | 30 +++++++++++++----------------- > > drivers/gpu/drm/i915/i915_drv.c | 5 +++++ > > drivers/gpu/drm/i915/i915_gem.c | 5 +++-- > > 3 files changed, 21 insertions(+), 19 deletions(-) >=20 > Looks good to me: >=20 > Reviewed-by: Thierry Reding Oh, perhaps s/unistall/uninstall/ in the commit subject. Thierry --mR8QP4gmHujQHb1c Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBAgAGBQJTT+jKAAoJEN0jrNd/PrOh0p4P+wVB93mhWTNFzuL/55Ew2vA1 mKEk43nbybtjd8Tku9N6ZefwKsQ4ZyjU4FocBuC4VpojtheyZium2b78NKN0fqgI PI2TCCqpwq3T8gEd6c+eWgQ5MgtMnMd8R8bzXrTeheZ1KebjG10Y832nwgVvOzZ2 X2EQSbFjxQdAfUZYN0Wm7RKz/HFeNY62FIEJm/HTvUTIukJed7NMvp0szeQjV9ZU vW+S5K/4yRtn4tTWkormR4UJG+Y7XcfVqgyVyMQpaF2YxmjuseX6Z2Fak1ZlvuWr kB7FAeQ6ciPD6R1XPE4xxx1LEjDxN3ZcgwdvArQFN+JT3o1fAspYH1uL0Ksfc5I5 FSdoBlYsyjfBvqHms3yWrFZRLCOTcfYK+e9GhlLuwTMRQZE3Mra3l8DsOinwSL9w g1pf2et/A+3sq0A4YbXxuTVdkGXWvEWc8jaKCmP4zBQqcSnDE3OdOE7vsWI9D7nR WVsdN3UDqN42InKtjPCwNdJdO+Us7u9z+jOX/FPq4PQpnQ25XaUK+KnySmCH+GxF uWLztcOMVnU2LRvtluBK3aNM5VUTPmZm00S6gLCiPPK/KSQdiYvPZHnaJysGfK84 AJc6N0gjqEEnTLgv3ijCmIZ5v0u9qqwV+5/+4lOmk2SATXBeXu/EX/ik1j7YIL/o GselssiChSWHn46agwET =oYVq -----END PGP SIGNATURE----- --mR8QP4gmHujQHb1c-- --===============1199791712== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel --===============1199791712==--