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:43:17 +0200 Message-ID: <20140417144316.GD550@ulmo> References: <1397252175-14227-1-git-send-email-daniel.vetter@ffwll.ch> <1397252175-14227-6-git-send-email-daniel.vetter@ffwll.ch> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0621651064==" Return-path: Received: from mail-ee0-f46.google.com (mail-ee0-f46.google.com [74.125.83.46]) by gabe.freedesktop.org (Postfix) with ESMTP id E39EA6E0FD for ; Thu, 17 Apr 2014 07:44:27 -0700 (PDT) Received: by mail-ee0-f46.google.com with SMTP id t10so761289eei.19 for ; Thu, 17 Apr 2014 07:44:25 -0700 (PDT) In-Reply-To: <1397252175-14227-6-git-send-email-daniel.vetter@ffwll.ch> 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 --===============0621651064== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="Xm/fll+QQv+hsKip" Content-Disposition: inline --Xm/fll+QQv+hsKip Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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(-) Looks good to me: Reviewed-by: Thierry Reding --Xm/fll+QQv+hsKip Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBAgAGBQJTT+iEAAoJEN0jrNd/PrOhN10P/2VNV1ywTw8+TSufj/b/hhIf KDKCVQqjU5is+PrPWuRVes5/7ERkD4sb88Qb1oq7h+hD7mcRGxq4Ols+pkYASvsK BZIyd29pLFILlp/tQSyVDbNEeObml2tBbzLpApqWq02mVApaM1WzS4rUMmiQsn+B dUdVy5SuZFtvIeX1lCWfBa9ZFuVTXtddOccSDhSZj3ghKepXg7K4Ze7R+MjgqpUG 1ac8Sz8uerqtMmQqpmiyTJVlmHIh4/FNBKN6jrUraLL8ZaXqHI6csxiW2auBBgt4 dpe02HQ2zXes/m09OumtUDamowRqMPxQ1dlwz0AfYFUT9A258h+MqnwkAppWLp9j CqwKWlz/72r3p+76ZTTFaXzbKBsYtounh+LiYfmNxHO5WoMJIAFYPGM+IjZKiF/Q 0j3MzIcSLr3WoDUw9M5ME2ksfQeyp2Vif7rXuUM1hXXU/yuoi/W1a4Rebw24XHPi WaqBSxZNNYIzsqmTk6Ct8mYyWEB+vOYSFgVeQESpGqWwYBWQaIhcIMX4XGbN8CUZ DCC5LwylxM8swqYiKKsLN/GckyvG0yXupTNQR+ZX1K4neA/FG29NSvBqmba3xoJw jlLMBZGmP0LqtVCJr7brHilFAQgGY2mYb2fYy7iJOR3bdtegsyB/YWDqHFp/yTN0 D8dEcRKerTDSbBDxCmTH =VcRQ -----END PGP SIGNATURE----- --Xm/fll+QQv+hsKip-- --===============0621651064== 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 --===============0621651064==--