From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5B367658 for ; Sat, 11 Jun 2022 14:50:04 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id EC62CC34116; Sat, 11 Jun 2022 14:49:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1654959004; bh=LX+rtX5P3mKqmVMa/tJgwQ4uwQTtTzZC0lJMO9c+uKY=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=NozNRnZ9eVVMlh3i/YFnXyveVGFVoLy0j6zJGUcdi8rJ4t2MDdOBT4MXOdmzJVbc1 ZIKXXfLH89Q8gM33/AvmEkMy7rRJRm4wLuFmzjkNryddMmxJ0F2puXoi1DHkqKnEe7 BMw3tDXb1LdjCJv0ZwsNznU5FKt/MnWP7qW7wg4BxhiFAu9Kgcsl40fmseyFCTpUIr irqOYi+V8FtnEYrtIDDJq5R6/OmDksOY9wgPTCNXsn8yQGMxmJq3ghGfhFOXEXv/SZ 8mQiUM2KswJhVPFAsaXxGO2zwsjGSn8/KxZl6/iv+Lc9WGRXVQ578HMjv3GoAbU3KS d8lBUX9EjQXSA== Date: Sat, 11 Jun 2022 15:59:02 +0100 From: Jonathan Cameron To: Nuno =?UTF-8?B?U8Oh?= Cc: Andy Shevchenko , Nuno =?UTF-8?B?U8Oh?= , dl-linux-imx , Linux-Renesas , "open list:BROADCOM NVRAM DRIVER" , linux-arm Mailing List , chrome-platform@lists.linux.dev, Lad Prabhakar , "moderated list:ARM/Mediatek SoC support" , linux-stm32@st-md-mailman.stormreply.com, linux-arm-msm , linux-iio , OpenBMC Maillist , Cai Huoqing , Benjamin Fair , Jishnu Prakash , Linus Walleij , Lars-Peter Clausen , Alexandre Torgue , Amit Kucheria , Andy Gross , Michael Hennerich , Haibo Chen , Benson Leung , "Rafael J. Wysocki" , Alexandre Belloni , Christophe Branchereau , Patrick Venture , Arnd Bergmann , Nancy Yuen , Sascha Hauer , Daniel Lezcano , Gwendal Grignou , Saravanan Sekar , Tali Perry , Maxime Coquelin , Paul Cercueil , Thara Gopinath , Avi Fishman , Lorenzo Bianconi , Claudiu Beznea , Pengutronix Kernel Team , Fabrice Gasnier , Matthias Brugger , Tomer Maimon , Bjorn Andersson , Nicolas Ferre , Zhang Rui , Shawn Guo , Guenter Roeck , Fabio Estevam , Olivier Moysan , Eugen Hristev , Miquel Raynal , Mark Brown Subject: Re: [PATCH 20/34] iio: inkern: only relase the device node when done with it Message-ID: <20220611155902.2a5a7738@jic23-huawei> In-Reply-To: References: <20220610084545.547700-1-nuno.sa@analog.com> <20220610084545.547700-21-nuno.sa@analog.com> X-Mailer: Claws Mail 4.1.0 (GTK 3.24.34; x86_64-pc-linux-gnu) Precedence: bulk X-Mailing-List: chrome-platform@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable +Cc Mark Brown for a query on ordering in device tree based SPI setup. On Fri, 10 Jun 2022 22:08:41 +0200 Nuno S=C3=A1 wrote: > On Fri, 2022-06-10 at 16:56 +0200, Andy Shevchenko wrote: > > On Fri, Jun 10, 2022 at 10:48 AM Nuno S=C3=A1 wrot= e: =20 > > >=20 > > > 'of_node_put()' can potentially release the memory pointed to by > > > 'iiospec.np' which would leave us with an invalid pointer (and we > > > would > > > still pass it in 'of_xlate()'). As such, we can only release the > > > node > > > after we are done with it. =20 > >=20 > > The question you should answer in the commit message is the > > following: > > "Can an OF node, attached to a struct device, be gone before the > > device itself?" If it so, then patch is good, otherwise there is no > > point in this patch in the first place. > > =20 >=20 > Yeah, I might be wrong but from a quick look... yes, I think the node > can be gone before the device. Take a look on the spi or i2c of_notify > handling and you can see that the nodes are get/put on the add/remove > notifcation. Meaning that the node lifespan is not really attached to > the device lifespan. If it was, I would expect to see of_node_put() on > the device release() function... I had a look at spi_of_notify() and indeed via spi_unregister_device() the node is put just before device_del() so I agree that at first glance it seems like there may be a race there against the useage here. Mark (+CC) out of interest why are the node gets before the device_add() in spi_add_device() called from of_register_spi_device() but the matching node puts before the device_del() in spi_unregister_device()? Seems like inconsistent ordering... Which is not to say we shouldn't fix the IIO usage as this patch does! >=20 > Again, I might be wrong and I admit I was not sure about including this > patch because it's a very unlikely scenario even though I think, in > theory, a possible one. The patch is currently valid even if it's not a 'real' bug. Given we are doing a put on that device_node, it makes sense for that to occur after the local use has finished - we shouldn't be relying on what happens to be the case for lifetimes today. Now, I did wonder if any drivers actually use it in their xlate callbacks. One does for an error print, so this is potentially real (if very unlikely!) This isn't a 'fix' I'd expect to rush in, or necessarily backport to stable but I think it's a valid fix. Perhaps add a little more detail to the patch description for v2. Thanks, Jonathan >=20 > - Nuno S=C3=A1 >=20 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id CDDC2C43334 for ; Sat, 11 Jun 2022 14:50:16 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=f4xS7TIF+XXLZJY8et9sXi5qtbsGBFNo/Q6bOOQ4ntY=; b=31CHUWRe3QR1cC N5pZDLcuE2XCthkfqqYNBfTqvQGnxlParlh0xP0yQxf1/DQeoAP3Yt1IfuDG4IkcncFga0BQuGfS6 Ih4PFkYvn3CQwJaA5JaXwyWhrzsp4sxDKCV3uzY99Fyc3rdEjnO5F72mtncoDWAEVpsVON39GpmwJ PR5LSMXHwOQkCOIjwmelxb5WA5/dVQc/812m1h8JPbRbJUeZ8zsgy702PuI83yF0kqvZP+22rOgsO 9j47mcTuHSFFMvo8zmrGeDcnca9TL8/S2JBUXMfymf5a45MyoN1eVfPqyJOTuvNiMiPRHGtuoBdHW doO/eqlQ1cKMFKH/Hhqw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1o02RG-00DPer-EC; Sat, 11 Jun 2022 14:50:10 +0000 Received: from ams.source.kernel.org ([145.40.68.75]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1o02RC-00DPdk-MP; Sat, 11 Jun 2022 14:50:08 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 4A88AB80123; Sat, 11 Jun 2022 14:50:05 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id EC62CC34116; Sat, 11 Jun 2022 14:49:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1654959004; bh=LX+rtX5P3mKqmVMa/tJgwQ4uwQTtTzZC0lJMO9c+uKY=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=NozNRnZ9eVVMlh3i/YFnXyveVGFVoLy0j6zJGUcdi8rJ4t2MDdOBT4MXOdmzJVbc1 ZIKXXfLH89Q8gM33/AvmEkMy7rRJRm4wLuFmzjkNryddMmxJ0F2puXoi1DHkqKnEe7 BMw3tDXb1LdjCJv0ZwsNznU5FKt/MnWP7qW7wg4BxhiFAu9Kgcsl40fmseyFCTpUIr irqOYi+V8FtnEYrtIDDJq5R6/OmDksOY9wgPTCNXsn8yQGMxmJq3ghGfhFOXEXv/SZ 8mQiUM2KswJhVPFAsaXxGO2zwsjGSn8/KxZl6/iv+Lc9WGRXVQ578HMjv3GoAbU3KS d8lBUX9EjQXSA== Date: Sat, 11 Jun 2022 15:59:02 +0100 From: Jonathan Cameron To: Nuno =?UTF-8?B?U8Oh?= Cc: Andy Shevchenko , Nuno =?UTF-8?B?U8Oh?= , dl-linux-imx , Linux-Renesas , "open list:BROADCOM NVRAM DRIVER" , linux-arm Mailing List , chrome-platform@lists.linux.dev, Lad Prabhakar , "moderated list:ARM/Mediatek SoC support" , linux-stm32@st-md-mailman.stormreply.com, linux-arm-msm , linux-iio , OpenBMC Maillist , Cai Huoqing , Benjamin Fair , Jishnu Prakash , Linus Walleij , Lars-Peter Clausen , Alexandre Torgue , Amit Kucheria , Andy Gross , Michael Hennerich , Haibo Chen , Benson Leung , "Rafael J. Wysocki" , Alexandre Belloni , Christophe Branchereau , Patrick Venture , Arnd Bergmann , Nancy Yuen , Sascha Hauer , Daniel Lezcano , Gwendal Grignou , Saravanan Sekar , Tali Perry , Maxime Coquelin , Paul Cercueil , Thara Gopinath , Avi Fishman , Lorenzo Bianconi , Claudiu Beznea , Pengutronix Kernel Team , Fabrice Gasnier , Matthias Brugger , Tomer Maimon , Bjorn Andersson , Nicolas Ferre , Zhang Rui , Shawn Guo , Guenter Roeck , Fabio Estevam , Olivier Moysan , Eugen Hristev , Miquel Raynal , Mark Brown Subject: Re: [PATCH 20/34] iio: inkern: only relase the device node when done with it Message-ID: <20220611155902.2a5a7738@jic23-huawei> In-Reply-To: References: <20220610084545.547700-1-nuno.sa@analog.com> <20220610084545.547700-21-nuno.sa@analog.com> X-Mailer: Claws Mail 4.1.0 (GTK 3.24.34; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220611_075007_052164_907EE197 X-CRM114-Status: GOOD ( 34.55 ) X-BeenThere: linux-mediatek@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Sender: "Linux-mediatek" Errors-To: linux-mediatek-bounces+linux-mediatek=archiver.kernel.org@lists.infradead.org CitDYyBNYXJrIEJyb3duIGZvciBhIHF1ZXJ5IG9uIG9yZGVyaW5nIGluIGRldmljZSB0cmVlIGJh c2VkIFNQSSBzZXR1cC4KCk9uIEZyaSwgMTAgSnVuIDIwMjIgMjI6MDg6NDEgKzAyMDAKTnVubyBT w6EgPG5vbmFtZS5udW5vQGdtYWlsLmNvbT4gd3JvdGU6Cgo+IE9uIEZyaSwgMjAyMi0wNi0xMCBh dCAxNjo1NiArMDIwMCwgQW5keSBTaGV2Y2hlbmtvIHdyb3RlOgo+ID4gT24gRnJpLCBKdW4gMTAs IDIwMjIgYXQgMTA6NDggQU0gTnVubyBTw6EgPG51bm8uc2FAYW5hbG9nLmNvbT4gd3JvdGU6ICAK PiA+ID4gCj4gPiA+ICdvZl9ub2RlX3B1dCgpJyBjYW4gcG90ZW50aWFsbHkgcmVsZWFzZSB0aGUg bWVtb3J5IHBvaW50ZWQgdG8gYnkKPiA+ID4gJ2lpb3NwZWMubnAnIHdoaWNoIHdvdWxkIGxlYXZl IHVzIHdpdGggYW4gaW52YWxpZCBwb2ludGVyIChhbmQgd2UKPiA+ID4gd291bGQKPiA+ID4gc3Rp bGwgcGFzcyBpdCBpbiAnb2ZfeGxhdGUoKScpLiBBcyBzdWNoLCB3ZSBjYW4gb25seSByZWxlYXNl IHRoZQo+ID4gPiBub2RlCj4gPiA+IGFmdGVyIHdlIGFyZSBkb25lIHdpdGggaXQuICAKPiA+IAo+ ID4gVGhlIHF1ZXN0aW9uIHlvdSBzaG91bGQgYW5zd2VyIGluIHRoZSBjb21taXQgbWVzc2FnZSBp cyB0aGUKPiA+IGZvbGxvd2luZzoKPiA+ICJDYW4gYW4gT0Ygbm9kZSwgYXR0YWNoZWQgdG8gYSBz dHJ1Y3QgZGV2aWNlLCBiZSBnb25lIGJlZm9yZSB0aGUKPiA+IGRldmljZSBpdHNlbGY/IiBJZiBp dCBzbywgdGhlbiBwYXRjaCBpcyBnb29kLCBvdGhlcndpc2UgdGhlcmUgaXMgbm8KPiA+IHBvaW50 IGluIHRoaXMgcGF0Y2ggaW4gdGhlIGZpcnN0IHBsYWNlLgo+ID4gICAKPiAKPiBZZWFoLCBJIG1p Z2h0IGJlIHdyb25nIGJ1dCBmcm9tIGEgcXVpY2sgbG9vay4uLiB5ZXMsIEkgdGhpbmsgdGhlIG5v ZGUKPiBjYW4gYmUgZ29uZSBiZWZvcmUgdGhlIGRldmljZS4gVGFrZSBhIGxvb2sgb24gdGhlIHNw aSBvciBpMmMgb2Zfbm90aWZ5Cj4gaGFuZGxpbmcgYW5kIHlvdSBjYW4gc2VlIHRoYXQgdGhlIG5v ZGVzIGFyZSBnZXQvcHV0IG9uIHRoZSBhZGQvcmVtb3ZlCj4gbm90aWZjYXRpb24uIE1lYW5pbmcg dGhhdCB0aGUgbm9kZSBsaWZlc3BhbiBpcyBub3QgcmVhbGx5IGF0dGFjaGVkIHRvCj4gdGhlIGRl dmljZSBsaWZlc3Bhbi4gSWYgaXQgd2FzLCBJIHdvdWxkIGV4cGVjdCB0byBzZWUgb2Zfbm9kZV9w dXQoKSBvbgo+IHRoZSBkZXZpY2UgcmVsZWFzZSgpIGZ1bmN0aW9uLi4uCgpJIGhhZCBhIGxvb2sg YXQgc3BpX29mX25vdGlmeSgpIGFuZCBpbmRlZWQgdmlhIHNwaV91bnJlZ2lzdGVyX2RldmljZSgp CnRoZSBub2RlIGlzIHB1dCBqdXN0IGJlZm9yZSBkZXZpY2VfZGVsKCkgc28gSSBhZ3JlZSB0aGF0 IGF0IGZpcnN0IGdsYW5jZQppdCBzZWVtcyBsaWtlIHRoZXJlIG1heSBiZSBhIHJhY2UgdGhlcmUg YWdhaW5zdCB0aGUgdXNlYWdlIGhlcmUuCk1hcmsgKCtDQykgb3V0IG9mIGludGVyZXN0IHdoeSBh cmUgdGhlIG5vZGUgZ2V0cyBiZWZvcmUgdGhlIGRldmljZV9hZGQoKQppbiBzcGlfYWRkX2Rldmlj ZSgpIGNhbGxlZCBmcm9tIG9mX3JlZ2lzdGVyX3NwaV9kZXZpY2UoKSBidXQgdGhlIG1hdGNoaW5n Cm5vZGUgcHV0cyBiZWZvcmUgdGhlIGRldmljZV9kZWwoKSBpbiBzcGlfdW5yZWdpc3Rlcl9kZXZp Y2UoKT8KU2VlbXMgbGlrZSBpbmNvbnNpc3RlbnQgb3JkZXJpbmcuLi4KCldoaWNoIGlzIG5vdCB0 byBzYXkgd2Ugc2hvdWxkbid0IGZpeCB0aGUgSUlPIHVzYWdlIGFzIHRoaXMgcGF0Y2ggZG9lcyEK Cj4gCj4gQWdhaW4sIEkgbWlnaHQgYmUgd3JvbmcgYW5kIEkgYWRtaXQgSSB3YXMgbm90IHN1cmUg YWJvdXQgaW5jbHVkaW5nIHRoaXMKPiBwYXRjaCBiZWNhdXNlIGl0J3MgYSB2ZXJ5IHVubGlrZWx5 IHNjZW5hcmlvIGV2ZW4gdGhvdWdoIEkgdGhpbmssIGluCj4gdGhlb3J5LCBhIHBvc3NpYmxlIG9u ZS4KClRoZSBwYXRjaCBpcyBjdXJyZW50bHkgdmFsaWQgZXZlbiBpZiBpdCdzIG5vdCBhICdyZWFs JyBidWcuCkdpdmVuIHdlIGFyZSBkb2luZyBhIHB1dCBvbiB0aGF0IGRldmljZV9ub2RlLCBpdCBt YWtlcyBzZW5zZSBmb3IgdGhhdAp0byBvY2N1ciBhZnRlciB0aGUgbG9jYWwgdXNlIGhhcyBmaW5p c2hlZCAtIHdlIHNob3VsZG4ndCBiZSByZWx5aW5nIG9uCndoYXQgaGFwcGVucyB0byBiZSB0aGUg Y2FzZSBmb3IgbGlmZXRpbWVzIHRvZGF5LgoKTm93LCBJIGRpZCB3b25kZXIgaWYgYW55IGRyaXZl cnMgYWN0dWFsbHkgdXNlIGl0IGluIHRoZWlyIHhsYXRlIGNhbGxiYWNrcy4KT25lIGRvZXMgZm9y IGFuIGVycm9yIHByaW50LCBzbyB0aGlzIGlzIHBvdGVudGlhbGx5IHJlYWwgKGlmIHZlcnkgdW5s aWtlbHkhKQoKVGhpcyBpc24ndCBhICdmaXgnIEknZCBleHBlY3QgdG8gcnVzaCBpbiwgb3IgbmVj ZXNzYXJpbHkgYmFja3BvcnQgdG8gc3RhYmxlCmJ1dCBJIHRoaW5rIGl0J3MgYSB2YWxpZCBmaXgu CgpQZXJoYXBzIGFkZCBhIGxpdHRsZSBtb3JlIGRldGFpbCB0byB0aGUgcGF0Y2ggZGVzY3JpcHRp b24gZm9yIHYyLgoKVGhhbmtzLAoKSm9uYXRoYW4KCgo+IAo+IC0gTnVubyBTw6EKPiAKCgpfX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpMaW51eC1tZWRpYXRl ayBtYWlsaW5nIGxpc3QKTGludXgtbWVkaWF0ZWtAbGlzdHMuaW5mcmFkZWFkLm9yZwpodHRwOi8v bGlzdHMuaW5mcmFkZWFkLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2xpbnV4LW1lZGlhdGVrCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 05B14C433EF for ; Thu, 16 Jun 2022 02:37:52 +0000 (UTC) Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4LNmYV4jS5z3bnj for ; Thu, 16 Jun 2022 12:37:50 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=kernel.org header.i=@kernel.org header.a=rsa-sha256 header.s=k20201202 header.b=NozNRnZ9; dkim-atps=neutral Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=kernel.org (client-ip=145.40.68.75; helo=ams.source.kernel.org; envelope-from=jic23@kernel.org; receiver=) Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=kernel.org header.i=@kernel.org header.a=rsa-sha256 header.s=k20201202 header.b=NozNRnZ9; dkim-atps=neutral Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4LL12n0zbTz3015 for ; Sun, 12 Jun 2022 00:50:09 +1000 (AEST) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 4A88AB80123; Sat, 11 Jun 2022 14:50:05 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id EC62CC34116; Sat, 11 Jun 2022 14:49:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1654959004; bh=LX+rtX5P3mKqmVMa/tJgwQ4uwQTtTzZC0lJMO9c+uKY=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=NozNRnZ9eVVMlh3i/YFnXyveVGFVoLy0j6zJGUcdi8rJ4t2MDdOBT4MXOdmzJVbc1 ZIKXXfLH89Q8gM33/AvmEkMy7rRJRm4wLuFmzjkNryddMmxJ0F2puXoi1DHkqKnEe7 BMw3tDXb1LdjCJv0ZwsNznU5FKt/MnWP7qW7wg4BxhiFAu9Kgcsl40fmseyFCTpUIr irqOYi+V8FtnEYrtIDDJq5R6/OmDksOY9wgPTCNXsn8yQGMxmJq3ghGfhFOXEXv/SZ 8mQiUM2KswJhVPFAsaXxGO2zwsjGSn8/KxZl6/iv+Lc9WGRXVQ578HMjv3GoAbU3KS d8lBUX9EjQXSA== Date: Sat, 11 Jun 2022 15:59:02 +0100 From: Jonathan Cameron To: Nuno =?UTF-8?B?U8Oh?= Subject: Re: [PATCH 20/34] iio: inkern: only relase the device node when done with it Message-ID: <20220611155902.2a5a7738@jic23-huawei> In-Reply-To: References: <20220610084545.547700-1-nuno.sa@analog.com> <20220610084545.547700-21-nuno.sa@analog.com> X-Mailer: Claws Mail 4.1.0 (GTK 3.24.34; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-Mailman-Approved-At: Thu, 16 Jun 2022 12:05:37 +1000 X-BeenThere: openbmc@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Development list for OpenBMC List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Alexandre Belloni , Daniel Lezcano , Tomer Maimon , "Rafael J. Wysocki" , linux-iio , Linus Walleij , Amit Kucheria , Alexandre Torgue , Nuno =?UTF-8?B?U8Oh?= , Paul Cercueil , Miquel Raynal , Guenter Roeck , Fabio Estevam , linux-stm32@st-md-mailman.stormreply.com, chrome-platform@lists.linux.dev, Lars-Peter Clausen , Benjamin Fair , OpenBMC Maillist , Jishnu Prakash , Haibo Chen , Andy Shevchenko , Andy Gross , dl-linux-imx , Olivier Moysan , Zhang Rui , Christophe Branchereau , Bjorn Andersson , Saravanan Sekar , Michael Hennerich , linux-arm-msm , Sascha Hauer , Nicolas Ferre , Lad Prabhakar , Fabrice Gasnier , "moderated list:ARM/Mediatek SoC support" , Eugen Hristev , Matthias Brugger , Gwendal Grignou , Tali Perry , Benson Leung , Pengutronix Kernel Team , linux-arm Mailing List , Lorenzo Bianconi , Avi Fishman , Patrick Venture , "open list:BROADCOM NVRAM DRIVER" , Thara Gopinath , Linux-Renesas , Mark Brown , Arnd Bergmann , Maxime Coquelin , Cai Huoqing , Shawn Guo , Claudiu Beznea Errors-To: openbmc-bounces+openbmc=archiver.kernel.org@lists.ozlabs.org Sender: "openbmc" +Cc Mark Brown for a query on ordering in device tree based SPI setup. On Fri, 10 Jun 2022 22:08:41 +0200 Nuno S=C3=A1 wrote: > On Fri, 2022-06-10 at 16:56 +0200, Andy Shevchenko wrote: > > On Fri, Jun 10, 2022 at 10:48 AM Nuno S=C3=A1 wrot= e: =20 > > >=20 > > > 'of_node_put()' can potentially release the memory pointed to by > > > 'iiospec.np' which would leave us with an invalid pointer (and we > > > would > > > still pass it in 'of_xlate()'). As such, we can only release the > > > node > > > after we are done with it. =20 > >=20 > > The question you should answer in the commit message is the > > following: > > "Can an OF node, attached to a struct device, be gone before the > > device itself?" If it so, then patch is good, otherwise there is no > > point in this patch in the first place. > > =20 >=20 > Yeah, I might be wrong but from a quick look... yes, I think the node > can be gone before the device. Take a look on the spi or i2c of_notify > handling and you can see that the nodes are get/put on the add/remove > notifcation. Meaning that the node lifespan is not really attached to > the device lifespan. If it was, I would expect to see of_node_put() on > the device release() function... I had a look at spi_of_notify() and indeed via spi_unregister_device() the node is put just before device_del() so I agree that at first glance it seems like there may be a race there against the useage here. Mark (+CC) out of interest why are the node gets before the device_add() in spi_add_device() called from of_register_spi_device() but the matching node puts before the device_del() in spi_unregister_device()? Seems like inconsistent ordering... Which is not to say we shouldn't fix the IIO usage as this patch does! >=20 > Again, I might be wrong and I admit I was not sure about including this > patch because it's a very unlikely scenario even though I think, in > theory, a possible one. The patch is currently valid even if it's not a 'real' bug. Given we are doing a put on that device_node, it makes sense for that to occur after the local use has finished - we shouldn't be relying on what happens to be the case for lifetimes today. Now, I did wonder if any drivers actually use it in their xlate callbacks. One does for an error print, so this is potentially real (if very unlikely!) This isn't a 'fix' I'd expect to rush in, or necessarily backport to stable but I think it's a valid fix. Perhaps add a little more detail to the patch description for v2. Thanks, Jonathan >=20 > - Nuno S=C3=A1 >=20