From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wolfram Sang Subject: Re: [PATCH] [RFC] i2c: Don't wait for device release in i2c_del_adapter Date: Tue, 13 Jan 2015 16:29:57 +0100 Message-ID: <20150113152957.GL7660@katana> References: <1421082050-10213-1-git-send-email-pantelis.antoniou@konsulko.com> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="PpocKf6TCvdC9BKE" Return-path: Content-Disposition: inline In-Reply-To: <1421082050-10213-1-git-send-email-pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> Sender: linux-i2c-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Pantelis Antoniou , Jean Delvare Cc: Guenter Roeck , Matt Porter , Greg Kroah-Hartman , Grant Likely , linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Pantelis Antoniou List-Id: linux-i2c@vger.kernel.org --PpocKf6TCvdC9BKE Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Jan 12, 2015 at 07:00:50PM +0200, Pantelis Antoniou wrote: > Waiting for the device release method to be called when > going through i2c_del_adapter is wrong and leads to deadlock > when removing an i2c mux device. Adding Jean to the list, maybe he knows more details from the past. >=20 > For instance when using the OF i2c mux unitest removal we get this. >=20 > [] (__schedule) from [] (schedule_timeout+0x18/0x198) > [] (schedule_timeout) from [] (wait_for_common+0xf8/0= x138) > [] (wait_for_common) from [] (i2c_del_adapter+0x174/0= x1cc) > [] (i2c_del_adapter) from [] (i2c_del_mux_adapter+0x4= 8/0x60) > [] (i2c_del_mux_adapter) from [] (selftest_i2c_mux_re= move+0x28/0x34) > [] (selftest_i2c_mux_remove) from [] (i2c_device_remo= ve+0x34/0x70) > [] (i2c_device_remove) from [] (__device_release_driv= er+0x7c/0xc0) > [] (__device_release_driver) from [] (driver_detach+0= x8c/0xb4) > [] (driver_detach) from [] (bus_remove_driver+0x64/0x= 8c) > [] (bus_remove_driver) from [] (of_selftest+0x1f84/0x= 20f0) > [] (of_selftest) from [] (do_one_initcall+0x104/0x1b4) > [] (do_one_initcall) from [] (kernel_init_freeable+0x= 110/0x1d8) > [] (kernel_init_freeable) from [] (kernel_init+0x8/0x= e4) > [] (kernel_init) from [] (ret_from_fork+0x14/0x3c) > 2 locks held by swapper/0/1: > #0: (&dev->mutex){......}, at: [] driver_detach+0x68/0xb4 > #1: (&dev->mutex){......}, at: [] driver_detach+0x78/0xb4 > Kernel panic - not syncing: hung_task: blocked tasks > CPU: 0 PID: 16 Comm: khungtaskd Not tainted 3.19.0-rc4-00022-g261647c #443 > Hardware name: Generic AM33XX (Flattened Device Tree) > [] (unwind_backtrace) from [] (show_stack+0x10/0x14) > [] (show_stack) from [] (dump_stack+0x70/0x8c) > [] (dump_stack) from [] (panic+0x88/0x1f0) > [] (panic) from [] (watchdog+0x2d4/0x350) > [] (watchdog) from [] (kthread+0xd8/0xec) > [] (kthread) from [] (ret_from_fork+0x14/0x3c) >=20 > The device release method is never called and we hang waiting for it. >=20 > This patch is marked as an [RFC] since the original code seems to > really want to protect against a race from sysfs accessors, which > I don't see how it could be a problem. >=20 > Signed-off-by: Pantelis Antoniou > --- > drivers/i2c/i2c-core.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) >=20 > diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c > index 39d25a8..e020a16 100644 > --- a/drivers/i2c/i2c-core.c > +++ b/drivers/i2c/i2c-core.c > @@ -1185,7 +1185,7 @@ EXPORT_SYMBOL_GPL(i2c_new_dummy); > static void i2c_adapter_dev_release(struct device *dev) > { > struct i2c_adapter *adap =3D to_i2c_adapter(dev); > - complete(&adap->dev_released); > + /* XXX complete(&adap->dev_released); */ > } > =20 > /* > @@ -1797,11 +1797,11 @@ void i2c_del_adapter(struct i2c_adapter *adap) > dev_dbg(&adap->dev, "adapter [%s] unregistered\n", adap->name); > =20 > /* clean up the sysfs representation */ > - init_completion(&adap->dev_released); > + /* XXX init_completion(&adap->dev_released); */ > device_unregister(&adap->dev); > =20 > /* wait for sysfs to drop all references */ > - wait_for_completion(&adap->dev_released); > + /* XXX wait_for_completion(&adap->dev_released); */ > =20 > /* free bus id */ > mutex_lock(&core_lock); > --=20 > 1.7.12 >=20 --PpocKf6TCvdC9BKE Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJUtTn1AAoJEBQN5MwUoCm2/7UQAIpuILWa+nu6XedFfr8pwXVc hdlAmtbM/E8d7K6qgIta7zl+LP4gfz7Zdc5FqOTbWnYzb5vZhc9KkV74fiPD3TpU CVpV97PWJDbvQ5nsF/thT1T6zlIIyL92G72B4vnjvfvMIS6DHrEZoAYYtm5pkk48 eTr4sBH4WKSb0t25rt9P2o7AZ5Bz+L5NbXiTv3NuyW1sky2VHz0e0abDKmM162Ns E5Q8uJuVGuzCywrLznBDkOd5slBiA9d/qNtCtqEclX235WtecVL6yQB+bLHt0jlX zQrAja3hORPIIhjvy//tmIvMwlVviJLUiSOprbv3CkqGMef9I8WxIA2q09ZRpeBK sAChJvkAfq15ntaSLgiY9opfPFhr9jTKuaV9zG7mxYKHId8TQGew0IDqKkHNnaqB ia3Yd/47NuI2lSvybPbupWG/eQXRme/4UvjyrQRfmMmn4zamGQs71OAjEsvWUGEH +uLlDfouzvlh4z8I3fO0kKqSCIamokEjB14gY2kI2nweXkAiPQnG+CNjq54JKXBZ Q9FTNCyAUGuFe78DBw8wEPx+GUOexEUpSIlJrtf0Q8Z7mZgzOjYNKWLmdnvVts3+ apkNqxIMu650X/Htd6tD8mFJMdFU1ovRpdmB2FgQxEMjF9JQm5POava5eCBYqcrL lkFNTcPsheDUECA2gPCT =60Ut -----END PGP SIGNATURE----- --PpocKf6TCvdC9BKE--