From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Subject: Re: [PATCH 00/18] dev->struct_mutex crusade Date: Mon, 10 Aug 2015 13:56:56 +0200 Message-ID: <20150810115654.GW1262@ulmo.nvidia.com> References: <1436477570-4936-1-git-send-email-daniel.vetter@ffwll.ch> <20150810113556.GT1262@ulmo.nvidia.com> <20150810115323.GE11040@nuc-i3427.alporthouse.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0268478211==" Return-path: In-Reply-To: <20150810115323.GE11040@nuc-i3427.alporthouse.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Chris Wilson , Daniel Vetter , Intel Graphics Development , DRI Development List-Id: dri-devel@lists.freedesktop.org --===============0268478211== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="E5HUUwS3R9LcK7+r" Content-Disposition: inline --E5HUUwS3R9LcK7+r Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Aug 10, 2015 at 12:53:23PM +0100, Chris Wilson wrote: > On Mon, Aug 10, 2015 at 01:35:58PM +0200, Thierry Reding wrote: > > On Thu, Jul 09, 2015 at 11:32:32PM +0200, Daniel Vetter wrote: > > > Hi all, > > >=20 > > > I wanted to take another look at struct_mutex usage in modern (gem) d= rivers and > > > noticed that for a fair lot we're very to be completely struct_mutex = free. > > >=20 > > > This pile here is the the simple part, which mostly just removes code= and > > > mutex_lock/unlock calls. All the patches here are independent and can= be merged > > > in any order whatsoever. My plan is to send out a pull request for al= l those not > > > picked up by driver maintainers in 2-3 weeks or so, assuming no one c= omplains. > > >=20 > > > Of course review & comments still very much welcome. > > >=20 > > > The more tricky 2nd part of this (and that one's not yet done) is to = rework the > > > gem mmap handler to use the same kref_get_unless_zero trick as ttm. W= ith that > > > there's no core requirement to hold struct_mutex over the final unref= , which > > > means we can make that one lockless. I plan to add a gem_object_free_= unlocked > > > for all the drivers which don't have any need for this lock. > > >=20 > > > Also there's a few more drivers which can be made struct_mutex free e= asily, I'll > > > propably stitch together poc patches for those. > >=20 > > There's a concurrency bug in Tegra DRM currently because we don't lock > > accesses to drm_mm (I guess this demonstrates how badly we need better > > testing...) and it seems like this is typically protected by the very > > same struct_mutex that you're on a crusade to remove. If your goal is > > to get rid of it for good, should we simply add a separate lock just > > for the drm_mm? We don't have another one that would fit. >=20 > Actually that is one of the first targets for more fine-grained locking. > I would not add a new lock to drm_mm as at least for i915, we want to use > a similar per-vm lock (of which the drm_mm is just one part). Sorry if I was being unclear. I wasn't suggesting adding the lock to struct drm_mm, but rather add a driver-specific one specifically to serialize accesses to the drm_mm. I agree that it's better to do this in a driver-specific way because other structures may need to be protected by the same lock. Thierry --E5HUUwS3R9LcK7+r Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAABCAAGBQJVyJGGAAoJEN0jrNd/PrOhwTEP/RNK7cd9uwhrLvLZtzfuIkLn E6gmrwGDqbKmxGWmMea5pFPNPGFJvLxPhPgQqtVdR1MBUxVLCaCYpqu3sR+TXM+V LGfjIHprHcLNva7QPleRnFLrGBiyktNcpeJYrfJFbnk10A0NhTmxptsQWWa3o4M4 v98q1BlZ/pn0z0B19GnoLLRccD5cw8622XeeKFLLFHEIJ7+56WdUY2sZRPRi2ZUY qbtNF6EY/wtlWHn3rx2uy5RNImdBTUmg9okdMWpzq0zue3116nlxn+KuBswrmw72 jUISDq8irJN21YxmRId9l0IgbvLVORSNrOJTbn28q8ONenPkSladaeRz+mqYdSQS XoI+NSA6kmt9NlrDd5i187xVA2JmAytLaxv04eA8ZmaR3gItqTdQZ0r8WMp7PdAZ bcOROQX3MW5JMUWmM/FUszPY0lzWcjZaixd7GacYooqjK2StELTiIAcQAEeXM8ov Nxhg0zUGo8szme5yDKVhfTgVzAI8i6IyP+VxorkYiQwjge9XkL3RXb3Ii9DRBq0w Y9c/zzQ111a2z5vF6bYUtLHkLDJ17SUw9q8giRyNgRroVSNmKyY5vexkBKGUcuYu /1298S6+Kk4RRcu+k/ze5ikfAl/7XRsWor3xenvoKNlSB2724Ru3jfhrAokRnZHh dV+L7OBNdIWA26z50Nx2 =FjFo -----END PGP SIGNATURE----- --E5HUUwS3R9LcK7+r-- --===============0268478211== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0 cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK --===============0268478211==--