From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paul Menzel Subject: Re: [PATCH 24/30] drm/i915: Refactor pwrite/pread to use single copy of get_user_pages Date: Fri, 15 Apr 2011 11:48:33 +0200 Message-ID: <1302860913.4203.31.camel@mattotaupa> References: <1302640318-23165-1-git-send-email-chris@chris-wilson.co.uk> <1302640318-23165-25-git-send-email-chris@chris-wilson.co.uk> <20110413192623.GG3660@viiv.ffwll.ch> <1bdc18$k77tdj@fmsmga002.fm.intel.com> <20110414232323.GA4118@lundgren.kumite> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1703912901==" Return-path: Received: from mail.gw90.de (mail.gw90.de [188.40.100.199]) by gabe.freedesktop.org (Postfix) with ESMTP id 573799E752 for ; Fri, 15 Apr 2011 02:48:37 -0700 (PDT) Received: from f053034154.adsl.alicedsl.de ([78.53.34.154] helo=[192.168.178.21]) by mail.gw90.de with esmtpsa (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.69) (envelope-from ) id 1QAfe2-0003YY-Sv for intel-gfx@lists.freedesktop.org; Fri, 15 Apr 2011 09:48:55 +0000 In-Reply-To: <20110414232323.GA4118@lundgren.kumite> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org --===============1703912901== Content-Type: multipart/signed; micalg="pgp-sha1"; protocol="application/pgp-signature"; boundary="=-SXa6HPfhtzWOQAW527OU" --=-SXa6HPfhtzWOQAW527OU Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Am Donnerstag, den 14.04.2011, 16:23 -0700 schrieb Ben Widawsky: > On Wed, Apr 13, 2011 at 08:56:26PM +0100, Chris Wilson wrote: > > On Wed, 13 Apr 2011 21:26:24 +0200, Daniel Vetter wro= te: > > > On Tue, Apr 12, 2011 at 09:31:52PM +0100, Chris Wilson wrote: > > > > Replace the three nearly identical copies of the code with a single > > > > function. And take advantage of the opportunity to do some > > > > micro-optimisation: avoid the vmalloc if at all possible and also a= void > > > > dropping the lock unless we are forced to acquire the mm semaphore. > > >=20 > > > One tiny nitpick: Perhaps put an api comment at the top of > > > gem_get_user_pages that this function drops the struct_mutex. That's = not > > > something we normally do and could cause endless amounts of fun if > > > neglected. > >=20 > > How about: > >=20 > > /** > > * Magically retrieves the pages for the user addr whilst holding the > > * dev->struct_mutex. > > * > > * Since we can not take the mm semaphore whilst holding our dev->struc= t_mutex, > > * due to the pre-existing lock dependency established by i915_gem_faul= t(), > > * we have to perform some sleight-of-hand. > > * > > * First, we try the lockless variant of gup whilst continuing to hold = the I do not know what =C2=BBgup=C2=AB means. > > * mutex. If that fails to get all the user pages, then we no choice bu= t s/then we no/then we have no/ > > * to acquire the mm semaphore (thus dropping the lock on dev->struct_m= utex > > * to do so). The dev->struct_mutex is then re-acquired before we retur= n. > > * > > * Returns: an error code *and* the number of user pages acquired. Even > > * on an error, you must iterate over the return pages and release them= . > > */ > >=20 > > ? > > -Chris >=20 > I like this patch... >=20 > Reviewed-by: Ben Widawsky Reviewed-by: Paul Menzel Thanks, Paul --=-SXa6HPfhtzWOQAW527OU Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) iEYEABECAAYFAk2oFHEACgkQPX1aK2wOHVjDlQCfUMbkVz9+trQmTyIFx30i36Qu mtMAn2A7kIhrGmDECA8GqDb/MG/c864h =kHaT -----END PGP SIGNATURE----- --=-SXa6HPfhtzWOQAW527OU-- --===============1703912901== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx --===============1703912901==--