From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Subject: Re: [RFC PATCH v3 4/4] tests/drv_module_reload: add ipvr support Date: Mon, 24 Nov 2014 10:55:46 +0100 Message-ID: <20141124095544.GA25912@ulmo.nvidia.com> References: <1416597001-6097-1-git-send-email-yao.cheng@intel.com> <20141121202702.GA10379@mithrandir> <20141121203633.GB25711@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1069133097==" Return-path: In-Reply-To: <20141121203633.GB25711@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Daniel Vetter Cc: daniel.vetter@ffwll.ch, emil.l.velikov@gmail.com, dri-devel@lists.freedesktop.org, john.chehab@intel.com, fei.jiang@intel.com, intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org --===============1069133097== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="CE+1k2dSO48ffgeK" Content-Disposition: inline --CE+1k2dSO48ffgeK Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Nov 21, 2014 at 09:36:33PM +0100, Daniel Vetter wrote: > On Fri, Nov 21, 2014 at 09:27:04PM +0100, Thierry Reding wrote: > > On Sat, Nov 22, 2014 at 03:10:01AM +0800, Yao Cheng wrote: > > > on vlv, if ipvr is installed, it need be manually unloaded before > > > i915, otherwise user might run into use-after-free issue. > >=20 > > Huh? That doesn't sound right. What exactly is it that's going wrong? > > You should never have to do this. If you do you're almost certainly > > doing something wrong in the kernel module. >=20 > It's the hilarity called platform devices. Removing them is somewhat racy, > so doing that upfront makes the entire thing a bit safer. The use after > free is on the text, since grabbing a module refcount for the platform > device doesn't work (it would pin the module forever). I don't understand what the issue is here. I've used platform devices quite extensively on ARM and I've never encountered a situation where they were insufficient (or racy for that matter). If I understand correctly what this commit tries to achieve, then it unloads one module before another module that it depends on so that the dependency can be removed subsequently without causing a crash. That sounds really brittle to me. How are you going to document this for users so that they don't accidentally go and unload the i915 module and crash their system? Thierry --CE+1k2dSO48ffgeK Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBAgAGBQJUcwCgAAoJEN0jrNd/PrOhhbwP/iqPPccVjdgGAJdF7lUuf/uc CwjVYfpKJqBMtMG1wQGgDE6Hb2WiI2iHvuhW9fT3rZOm7b3l25C8O9V1NFDkzt1f s9l8/u5Sf3Gpq7Dt9PqNvEMhjARmYjaeNSaFGWIxtuKMvglKcgvUX9n/1OkI5KAH dM3gXtizuFHSadYH75PuZnxz6urv9+syikr0cIBSoOKD/Wn6Su0vK03RFiOHCM76 V+5sR/N+1fe6F5vNhxBahZdDCmuynenykgjrWtd64yZUVmvvslL6x9RGAaItywue RivhafjY00Hyvuj4elCCpQtu+UnL9oLaXJbQgsLH1gUaJcpfc5NaO2ngd2WWf5Uk eOoVEDOMS4l4uo6tjx7YaQFHhIKYH0lwAChM/gwNbSSJJMG5EApgfuDDRSkqHDXs jA2BBTY/CQWPjqs7aEEU1zWlWNtmGZYejAsxfKwlAvgULiHhjiVrZ1yHQPPg5oOG I9p6LzKFDWncPUkpGif0wC3lN9fNxhS1mTL7wDJDCUHgHxZpY7Ns9X/SFwROB7c/ g0B9Bbsogdk5GFLz4qseES43xuXmiSZJloBevwuOOVpNmFd4jeASWd3WyPvZEPnR JZ46WRZXwOfeoToi8nxgdH4NFtLuQqOw4bRbX+D33Z8At6s1fXnnw/uS1MRAOW7p k56QRjZq3SsXaG2GMGyt =SBD7 -----END PGP SIGNATURE----- --CE+1k2dSO48ffgeK-- --===============1069133097== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4 IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0 cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9pbnRlbC1nZngK --===============1069133097==--