From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-qk1-f176.google.com (mail-qk1-f176.google.com [209.85.222.176]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A3B3E110D for ; Mon, 13 Jun 2022 07:19:37 +0000 (UTC) Received: by mail-qk1-f176.google.com with SMTP id 15so3434769qki.6 for ; Mon, 13 Jun 2022 00:19:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:subject:from:to:cc:date:in-reply-to:references :content-transfer-encoding:user-agent:mime-version; bh=GB+StqyTCKj2HdFboJFVKMCJDxe3eEc2lGFDv1Opjlc=; b=X68PW8XVsxQon0pHG8Sz5eBs3OS4NqSoIFXXe/2G6Nvk/Ompk2l/aeSLop0xsWcJ5E r+imyj6yRhl2ZBY5gXu6epRIquTOxi5XRXkt8chNvQ9i2e6cKF0ixckJovb4CewVtw3a u7m+l85fbDcOwBkr5wl8/iEJyWb9SDi2BND4tt8QxC/8hYJ+QSWoTwQJlI3MpdiaP0TW 6yNqcyKDvDSBqqDXyxsrnQmuDJ/Wm3MGqXQnOht4OpdpbeKyiQUmXcJUjIazEsf3nvgc erjP+lgqHr0T63epVk4DXJ3R/zBrJtNJPJZug+u1Hzggan083EJkQVJsbMGQXRTKtTRs dKKg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to :references:content-transfer-encoding:user-agent:mime-version; bh=GB+StqyTCKj2HdFboJFVKMCJDxe3eEc2lGFDv1Opjlc=; b=29rDYQU47oRK61VyFLJCziC7spsm7KBLrbGYJXEB83MTES3w8sNNec6NdpuNguOe8t k9q8bZ0mE9iGNobn5+w+d8ZXXoVY/AU0D4WS+ejgxIzGte0cGfXS510K8KVSnUU5LMMX VQHzP2W0S2h7YnwDOKuo/4tPafPqyBtKTDqW4XQfNxiMTGOinmb4M9zmcQscv2W8df/n 8r3hUKRUGZDl9bJwN+T4Gtf4OKO+PzKUetfXhbJZg+kC53nbglYbXud9KQRnxD2Z12AC WFUBoTFfirEurM+Hx8oubkDIv12V/k3eFMBEmc4WuZrNNKQLv3ZM/k/SjLfsTzsZi0hn Z8TQ== X-Gm-Message-State: AOAM5328REpY8QGQBh6PkdnPoiNtM4F+fpbiLDCtopeE28o07fIk4b3K cLGcDKcb+qiVxvo3dk6o1PA= X-Google-Smtp-Source: ABdhPJzkwQ8Lh4v8HmiyNYZX3CyfRLZco1LI0RmoLkuXuUs3JtU60bnSz/zcTWm7wmCt5lo+xJyqEQ== X-Received: by 2002:a37:4454:0:b0:69f:c339:e2dc with SMTP id r81-20020a374454000000b0069fc339e2dcmr36709053qka.771.1655104776463; Mon, 13 Jun 2022 00:19:36 -0700 (PDT) Received: from p200300f6ef062c0090c03b551078f99d.dip0.t-ipconnect.de (p200300f6ef062c0090c03b551078f99d.dip0.t-ipconnect.de. [2003:f6:ef06:2c00:90c0:3b55:1078:f99d]) by smtp.gmail.com with ESMTPSA id f8-20020a05620a408800b006a77e6df09asm4182070qko.24.2022.06.13.00.19.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 13 Jun 2022 00:19:35 -0700 (PDT) Message-ID: <5e81f73b996de80445c2e905c44ebb18c63a739b.camel@gmail.com> Subject: Re: [PATCH 20/34] iio: inkern: only relase the device node when done with it From: Nuno =?ISO-8859-1?Q?S=E1?= To: Jonathan Cameron Cc: Andy Shevchenko , Nuno =?ISO-8859-1?Q?S=E1?= , 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 Date: Mon, 13 Jun 2022 09:20:26 +0200 In-Reply-To: <20220611155902.2a5a7738@jic23-huawei> References: <20220610084545.547700-1-nuno.sa@analog.com> <20220610084545.547700-21-nuno.sa@analog.com> <20220611155902.2a5a7738@jic23-huawei> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.44.2 Precedence: bulk X-Mailing-List: chrome-platform@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 On Sat, 2022-06-11 at 15:59 +0100, Jonathan Cameron wrote: >=20 > +Cc Mark Brown for a query on ordering in device tree based SPI > setup. >=20 > On Fri, 10 Jun 2022 22:08:41 +0200 > Nuno S=C3=A1 wrote: >=20 > > 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 > > > wrote:=C2=A0=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.=C2=A0=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. > > > =C2=A0=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... >=20 > 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... >=20 > Which is not to say we shouldn't fix the IIO usage as this patch > does! >=20 Just to add something that came to my attention. In the IIO case, it does not even matter if the parent device has the OF node lifetime "linked" to it (as it actually happens for platform devices). The reason is that iio_dev only has a weak reference to it's parent and (I think) the parent can actually go away while the iio_dev is still around (eg: someone has an open fd to the iio_dev cdev). > >=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. >=20 > 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. >=20 > 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!) >=20 > This isn't a 'fix' I'd expect to rush in, or necessarily backport to > stable > but I think it's a valid fix. >=20 Should I drop the fixes tag? - Nuno S=C3=A1 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 358B0C43334 for ; Mon, 13 Jun 2022 07:19:48 +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: Date:Cc:To:From:Subject:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=wnODhUsvYXeRUAom351EjrSv+bSNg1k6qF2w5MGDvcY=; b=Fo1uUPHdYLPho+ eb3h3vKtdYDCYCbuj4bCKBB7pQonAxIkPAlyHFdcoEGHHTdoLGSHp8PR19enjiD4JH/O02ZPHS1Cs RivkMKUGPH31KZYRpJQydeHINw0Tp/shrHsfwWNFml0i3ORn3z7NTmxOi8xsOUBBkuWHz1utpbs/W jf3v8FGUtbGDQEbG2fDgqmTDEX530GPBfa4bS461V+oJqUmzLzuyKOTMOa2mQm3bfos0SwFORm5gh SWReEBs+vMu96CMwlnW65BUJfJN8+1PRhtaNXSzhCzH4+OS4yDcWPZh4pHUUrOGqCRb4VaQhe31ho v+WRjgeshFVIlUkAZgVg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1o0eMP-001wK8-7x; Mon, 13 Jun 2022 07:19:41 +0000 Received: from mail-qk1-x72a.google.com ([2607:f8b0:4864:20::72a]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1o0eMM-001wJ3-G5; Mon, 13 Jun 2022 07:19:39 +0000 Received: by mail-qk1-x72a.google.com with SMTP id b142so3449820qkg.2; Mon, 13 Jun 2022 00:19:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:subject:from:to:cc:date:in-reply-to:references :content-transfer-encoding:user-agent:mime-version; bh=GB+StqyTCKj2HdFboJFVKMCJDxe3eEc2lGFDv1Opjlc=; b=X68PW8XVsxQon0pHG8Sz5eBs3OS4NqSoIFXXe/2G6Nvk/Ompk2l/aeSLop0xsWcJ5E r+imyj6yRhl2ZBY5gXu6epRIquTOxi5XRXkt8chNvQ9i2e6cKF0ixckJovb4CewVtw3a u7m+l85fbDcOwBkr5wl8/iEJyWb9SDi2BND4tt8QxC/8hYJ+QSWoTwQJlI3MpdiaP0TW 6yNqcyKDvDSBqqDXyxsrnQmuDJ/Wm3MGqXQnOht4OpdpbeKyiQUmXcJUjIazEsf3nvgc erjP+lgqHr0T63epVk4DXJ3R/zBrJtNJPJZug+u1Hzggan083EJkQVJsbMGQXRTKtTRs dKKg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to :references:content-transfer-encoding:user-agent:mime-version; bh=GB+StqyTCKj2HdFboJFVKMCJDxe3eEc2lGFDv1Opjlc=; b=lPsar95a4DpeMUTU/4b8FqNa5QR04XGSwfmADmch/RKM8mtmmIFDN8vsn4UMiOClKy mkNTF6cDGNfP/+bBf0RWXjQuzN2YR9YZdrjbr7wc/d4QTTf4g8jM4n1pXY+4doUl9014 OujaXT7QF/HrKrfRo+1BBP17HKNQoe6o9QtzP+Bhnec9sBQyzvTtlbm6cJVZz0ZAifbw typSSdClQuZzd7ks01FeSpAqvuWDBGjIEnsu4+5CgiZKvboNdT4MW+QIyEVFnKxkS2VQ cVrxcJGGDcXcX9SHIXgM86oBwE7Gqg8iNSsL+k1mjVZuaGxAgHi3FnPljozXGsZjZcrE L2bg== X-Gm-Message-State: AOAM531RU/kGmxi8ht2EZ4M27Qu3VKuQtUkO+ytSZQbd8Z280J4a0OlJ mvQXLgeqm7kaTH7W10KD62s= X-Google-Smtp-Source: ABdhPJzkwQ8Lh4v8HmiyNYZX3CyfRLZco1LI0RmoLkuXuUs3JtU60bnSz/zcTWm7wmCt5lo+xJyqEQ== X-Received: by 2002:a37:4454:0:b0:69f:c339:e2dc with SMTP id r81-20020a374454000000b0069fc339e2dcmr36709053qka.771.1655104776463; Mon, 13 Jun 2022 00:19:36 -0700 (PDT) Received: from p200300f6ef062c0090c03b551078f99d.dip0.t-ipconnect.de (p200300f6ef062c0090c03b551078f99d.dip0.t-ipconnect.de. [2003:f6:ef06:2c00:90c0:3b55:1078:f99d]) by smtp.gmail.com with ESMTPSA id f8-20020a05620a408800b006a77e6df09asm4182070qko.24.2022.06.13.00.19.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 13 Jun 2022 00:19:35 -0700 (PDT) Message-ID: <5e81f73b996de80445c2e905c44ebb18c63a739b.camel@gmail.com> Subject: Re: [PATCH 20/34] iio: inkern: only relase the device node when done with it From: Nuno =?ISO-8859-1?Q?S=E1?= To: Jonathan Cameron Cc: Andy Shevchenko , Nuno =?ISO-8859-1?Q?S=E1?= , 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 Date: Mon, 13 Jun 2022 09:20:26 +0200 In-Reply-To: <20220611155902.2a5a7738@jic23-huawei> References: <20220610084545.547700-1-nuno.sa@analog.com> <20220610084545.547700-21-nuno.sa@analog.com> <20220611155902.2a5a7738@jic23-huawei> User-Agent: Evolution 3.44.2 MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220613_001938_611769_FAC8528D X-CRM114-Status: GOOD ( 43.38 ) 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 T24gU2F0LCAyMDIyLTA2LTExIGF0IDE1OjU5ICswMTAwLCBKb25hdGhhbiBDYW1lcm9uIHdyb3Rl Ogo+IAo+ICtDYyBNYXJrIEJyb3duIGZvciBhIHF1ZXJ5IG9uIG9yZGVyaW5nIGluIGRldmljZSB0 cmVlIGJhc2VkIFNQSQo+IHNldHVwLgo+IAo+IE9uIEZyaSwgMTAgSnVuIDIwMjIgMjI6MDg6NDEg KzAyMDAKPiBOdW5vIFPDoSA8bm9uYW1lLm51bm9AZ21haWwuY29tPiB3cm90ZToKPiAKPiA+IE9u IEZyaSwgMjAyMi0wNi0xMCBhdCAxNjo1NiArMDIwMCwgQW5keSBTaGV2Y2hlbmtvIHdyb3RlOgo+ ID4gPiBPbiBGcmksIEp1biAxMCwgMjAyMiBhdCAxMDo0OCBBTSBOdW5vIFPDoSA8bnVuby5zYUBh bmFsb2cuY29tPgo+ID4gPiB3cm90ZTrCoCAKPiA+ID4gPiAKPiA+ID4gPiAnb2Zfbm9kZV9wdXQo KScgY2FuIHBvdGVudGlhbGx5IHJlbGVhc2UgdGhlIG1lbW9yeSBwb2ludGVkIHRvCj4gPiA+ID4g YnkKPiA+ID4gPiAnaWlvc3BlYy5ucCcgd2hpY2ggd291bGQgbGVhdmUgdXMgd2l0aCBhbiBpbnZh bGlkIHBvaW50ZXIgKGFuZAo+ID4gPiA+IHdlCj4gPiA+ID4gd291bGQKPiA+ID4gPiBzdGlsbCBw YXNzIGl0IGluICdvZl94bGF0ZSgpJykuIEFzIHN1Y2gsIHdlIGNhbiBvbmx5IHJlbGVhc2UKPiA+ ID4gPiB0aGUKPiA+ID4gPiBub2RlCj4gPiA+ID4gYWZ0ZXIgd2UgYXJlIGRvbmUgd2l0aCBpdC7C oCAKPiA+ID4gCj4gPiA+IFRoZSBxdWVzdGlvbiB5b3Ugc2hvdWxkIGFuc3dlciBpbiB0aGUgY29t bWl0IG1lc3NhZ2UgaXMgdGhlCj4gPiA+IGZvbGxvd2luZzoKPiA+ID4gIkNhbiBhbiBPRiBub2Rl LCBhdHRhY2hlZCB0byBhIHN0cnVjdCBkZXZpY2UsIGJlIGdvbmUgYmVmb3JlIHRoZQo+ID4gPiBk ZXZpY2UgaXRzZWxmPyIgSWYgaXQgc28sIHRoZW4gcGF0Y2ggaXMgZ29vZCwgb3RoZXJ3aXNlIHRo ZXJlIGlzCj4gPiA+IG5vCj4gPiA+IHBvaW50IGluIHRoaXMgcGF0Y2ggaW4gdGhlIGZpcnN0IHBs YWNlLgo+ID4gPiDCoCAKPiA+IAo+ID4gWWVhaCwgSSBtaWdodCBiZSB3cm9uZyBidXQgZnJvbSBh IHF1aWNrIGxvb2suLi4geWVzLCBJIHRoaW5rIHRoZQo+ID4gbm9kZQo+ID4gY2FuIGJlIGdvbmUg YmVmb3JlIHRoZSBkZXZpY2UuIFRha2UgYSBsb29rIG9uIHRoZSBzcGkgb3IgaTJjCj4gPiBvZl9u b3RpZnkKPiA+IGhhbmRsaW5nIGFuZCB5b3UgY2FuIHNlZSB0aGF0IHRoZSBub2RlcyBhcmUgZ2V0 L3B1dCBvbiB0aGUKPiA+IGFkZC9yZW1vdmUKPiA+IG5vdGlmY2F0aW9uLiBNZWFuaW5nIHRoYXQg dGhlIG5vZGUgbGlmZXNwYW4gaXMgbm90IHJlYWxseSBhdHRhY2hlZAo+ID4gdG8KPiA+IHRoZSBk ZXZpY2UgbGlmZXNwYW4uIElmIGl0IHdhcywgSSB3b3VsZCBleHBlY3QgdG8gc2VlIG9mX25vZGVf cHV0KCkKPiA+IG9uCj4gPiB0aGUgZGV2aWNlIHJlbGVhc2UoKSBmdW5jdGlvbi4uLgo+IAo+IEkg aGFkIGEgbG9vayBhdCBzcGlfb2Zfbm90aWZ5KCkgYW5kIGluZGVlZCB2aWEKPiBzcGlfdW5yZWdp c3Rlcl9kZXZpY2UoKQo+IHRoZSBub2RlIGlzIHB1dCBqdXN0IGJlZm9yZSBkZXZpY2VfZGVsKCkg c28gSSBhZ3JlZSB0aGF0IGF0IGZpcnN0Cj4gZ2xhbmNlCj4gaXQgc2VlbXMgbGlrZSB0aGVyZSBt YXkgYmUgYSByYWNlIHRoZXJlIGFnYWluc3QgdGhlIHVzZWFnZSBoZXJlLgo+IE1hcmsgKCtDQykg b3V0IG9mIGludGVyZXN0IHdoeSBhcmUgdGhlIG5vZGUgZ2V0cyBiZWZvcmUgdGhlCj4gZGV2aWNl X2FkZCgpCj4gaW4gc3BpX2FkZF9kZXZpY2UoKSBjYWxsZWQgZnJvbSBvZl9yZWdpc3Rlcl9zcGlf ZGV2aWNlKCkgYnV0IHRoZQo+IG1hdGNoaW5nCj4gbm9kZSBwdXRzIGJlZm9yZSB0aGUgZGV2aWNl X2RlbCgpIGluIHNwaV91bnJlZ2lzdGVyX2RldmljZSgpPwo+IFNlZW1zIGxpa2UgaW5jb25zaXN0 ZW50IG9yZGVyaW5nLi4uCj4gCj4gV2hpY2ggaXMgbm90IHRvIHNheSB3ZSBzaG91bGRuJ3QgZml4 IHRoZSBJSU8gdXNhZ2UgYXMgdGhpcyBwYXRjaAo+IGRvZXMhCj4gCgpKdXN0IHRvIGFkZCBzb21l dGhpbmcgdGhhdCBjYW1lIHRvIG15IGF0dGVudGlvbi4gSW4gdGhlIElJTyBjYXNlLCBpdApkb2Vz IG5vdCBldmVuIG1hdHRlciBpZiB0aGUgcGFyZW50IGRldmljZSBoYXMgdGhlIE9GIG5vZGUgbGlm ZXRpbWUKImxpbmtlZCIgdG8gaXQgKGFzIGl0IGFjdHVhbGx5IGhhcHBlbnMgZm9yIHBsYXRmb3Jt IGRldmljZXMpLiBUaGUKcmVhc29uIGlzIHRoYXQgaWlvX2RldiBvbmx5IGhhcyBhIHdlYWsgcmVm ZXJlbmNlIHRvIGl0J3MgcGFyZW50IGFuZCAoSQp0aGluaykgdGhlIHBhcmVudCBjYW4gYWN0dWFs bHkgZ28gYXdheSB3aGlsZSB0aGUgaWlvX2RldiBpcyBzdGlsbAphcm91bmQgKGVnOiBzb21lb25l IGhhcyBhbiBvcGVuIGZkIHRvIHRoZSBpaW9fZGV2IGNkZXYpLgoKPiA+IAo+ID4gQWdhaW4sIEkg bWlnaHQgYmUgd3JvbmcgYW5kIEkgYWRtaXQgSSB3YXMgbm90IHN1cmUgYWJvdXQgaW5jbHVkaW5n Cj4gPiB0aGlzCj4gPiBwYXRjaCBiZWNhdXNlIGl0J3MgYSB2ZXJ5IHVubGlrZWx5IHNjZW5hcmlv IGV2ZW4gdGhvdWdoIEkgdGhpbmssIGluCj4gPiB0aGVvcnksIGEgcG9zc2libGUgb25lLgo+IAo+ IFRoZSBwYXRjaCBpcyBjdXJyZW50bHkgdmFsaWQgZXZlbiBpZiBpdCdzIG5vdCBhICdyZWFsJyBi dWcuCj4gR2l2ZW4gd2UgYXJlIGRvaW5nIGEgcHV0IG9uIHRoYXQgZGV2aWNlX25vZGUsIGl0IG1h a2VzIHNlbnNlIGZvciB0aGF0Cj4gdG8gb2NjdXIgYWZ0ZXIgdGhlIGxvY2FsIHVzZSBoYXMgZmlu aXNoZWQgLSB3ZSBzaG91bGRuJ3QgYmUgcmVseWluZwo+IG9uCj4gd2hhdCBoYXBwZW5zIHRvIGJl IHRoZSBjYXNlIGZvciBsaWZldGltZXMgdG9kYXkuCj4gCj4gTm93LCBJIGRpZCB3b25kZXIgaWYg YW55IGRyaXZlcnMgYWN0dWFsbHkgdXNlIGl0IGluIHRoZWlyIHhsYXRlCj4gY2FsbGJhY2tzLgo+ IE9uZSBkb2VzIGZvciBhbiBlcnJvciBwcmludCwgc28gdGhpcyBpcyBwb3RlbnRpYWxseSByZWFs IChpZiB2ZXJ5Cj4gdW5saWtlbHkhKQo+IAo+IFRoaXMgaXNuJ3QgYSAnZml4JyBJJ2QgZXhwZWN0 IHRvIHJ1c2ggaW4sIG9yIG5lY2Vzc2FyaWx5IGJhY2twb3J0IHRvCj4gc3RhYmxlCj4gYnV0IEkg dGhpbmsgaXQncyBhIHZhbGlkIGZpeC4KPiAKClNob3VsZCBJIGRyb3AgdGhlIGZpeGVzIHRhZz8K Ci0gTnVubyBTw6EKCgoKCgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fXwpMaW51eC1tZWRpYXRlayBtYWlsaW5nIGxpc3QKTGludXgtbWVkaWF0ZWtAbGlzdHMu aW5mcmFkZWFkLm9yZwpodHRwOi8vbGlzdHMuaW5mcmFkZWFkLm9yZy9tYWlsbWFuL2xpc3RpbmZv L2xpbnV4LW1lZGlhdGVrCg== 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 37A7BC433EF for ; Thu, 16 Jun 2022 02:42:33 +0000 (UTC) Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4LNmfv4v6sz3dvl for ; Thu, 16 Jun 2022 12:42:31 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20210112 header.b=X68PW8XV; dkim-atps=neutral Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gmail.com (client-ip=2607:f8b0:4864:20::734; helo=mail-qk1-x734.google.com; envelope-from=noname.nuno@gmail.com; receiver=) Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20210112 header.b=X68PW8XV; dkim-atps=neutral Received: from mail-qk1-x734.google.com (mail-qk1-x734.google.com [IPv6:2607:f8b0:4864:20::734]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4LM2y50Dptz2x9G for ; Mon, 13 Jun 2022 17:19:40 +1000 (AEST) Received: by mail-qk1-x734.google.com with SMTP id d23so3475028qke.0 for ; Mon, 13 Jun 2022 00:19:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:subject:from:to:cc:date:in-reply-to:references :content-transfer-encoding:user-agent:mime-version; bh=GB+StqyTCKj2HdFboJFVKMCJDxe3eEc2lGFDv1Opjlc=; b=X68PW8XVsxQon0pHG8Sz5eBs3OS4NqSoIFXXe/2G6Nvk/Ompk2l/aeSLop0xsWcJ5E r+imyj6yRhl2ZBY5gXu6epRIquTOxi5XRXkt8chNvQ9i2e6cKF0ixckJovb4CewVtw3a u7m+l85fbDcOwBkr5wl8/iEJyWb9SDi2BND4tt8QxC/8hYJ+QSWoTwQJlI3MpdiaP0TW 6yNqcyKDvDSBqqDXyxsrnQmuDJ/Wm3MGqXQnOht4OpdpbeKyiQUmXcJUjIazEsf3nvgc erjP+lgqHr0T63epVk4DXJ3R/zBrJtNJPJZug+u1Hzggan083EJkQVJsbMGQXRTKtTRs dKKg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to :references:content-transfer-encoding:user-agent:mime-version; bh=GB+StqyTCKj2HdFboJFVKMCJDxe3eEc2lGFDv1Opjlc=; b=YRGNR4wgaNLtKMX0k7Vk2fSjc4FVXgW5BnbL6HRHtPCkIhB+DHjy1j9bprNHPg8GSs m/WD8GfOasmlJwWhIHeETJyUA8o2RypQdKBHcZeqkTx6dZa9cyJRqSM0yzSKp7MKzxqa P62d19chkbpoiFx0obSolyd6JjUEFnKLGQwnIDG9WTQ+99ltvxyKg4/IMtG+usz5NLMt cKF1wEbXNDOq/B7TXjcaHLezB2D5btc3NwWnfbxk9uXCAegP6WsrDV7jl3yITTTZ2gjL V1a72OUPf23Qrj7g5ZO+VgfyT+NmsU6EPwhGdE+r6cgmoVBYmBnQh2JgRb4yMc/HJSop QgnQ== X-Gm-Message-State: AOAM533Yi7nWsEq9m8AgXlstXbIiXgUwfaL+9DiEufTIWXZATZ6t6pK3 Taz9Rc3uCTZGP9233psW7fg= X-Google-Smtp-Source: ABdhPJzkwQ8Lh4v8HmiyNYZX3CyfRLZco1LI0RmoLkuXuUs3JtU60bnSz/zcTWm7wmCt5lo+xJyqEQ== X-Received: by 2002:a37:4454:0:b0:69f:c339:e2dc with SMTP id r81-20020a374454000000b0069fc339e2dcmr36709053qka.771.1655104776463; Mon, 13 Jun 2022 00:19:36 -0700 (PDT) Received: from p200300f6ef062c0090c03b551078f99d.dip0.t-ipconnect.de (p200300f6ef062c0090c03b551078f99d.dip0.t-ipconnect.de. [2003:f6:ef06:2c00:90c0:3b55:1078:f99d]) by smtp.gmail.com with ESMTPSA id f8-20020a05620a408800b006a77e6df09asm4182070qko.24.2022.06.13.00.19.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 13 Jun 2022 00:19:35 -0700 (PDT) Message-ID: <5e81f73b996de80445c2e905c44ebb18c63a739b.camel@gmail.com> Subject: Re: [PATCH 20/34] iio: inkern: only relase the device node when done with it From: Nuno =?ISO-8859-1?Q?S=E1?= To: Jonathan Cameron Date: Mon, 13 Jun 2022 09:20:26 +0200 In-Reply-To: <20220611155902.2a5a7738@jic23-huawei> References: <20220610084545.547700-1-nuno.sa@analog.com> <20220610084545.547700-21-nuno.sa@analog.com> <20220611155902.2a5a7738@jic23-huawei> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.44.2 MIME-Version: 1.0 X-Mailman-Approved-At: Thu, 16 Jun 2022 12:05:36 +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 =?ISO-8859-1?Q?S=E1?= , 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" On Sat, 2022-06-11 at 15:59 +0100, Jonathan Cameron wrote: >=20 > +Cc Mark Brown for a query on ordering in device tree based SPI > setup. >=20 > On Fri, 10 Jun 2022 22:08:41 +0200 > Nuno S=C3=A1 wrote: >=20 > > 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 > > > wrote:=C2=A0=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.=C2=A0=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. > > > =C2=A0=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... >=20 > 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... >=20 > Which is not to say we shouldn't fix the IIO usage as this patch > does! >=20 Just to add something that came to my attention. In the IIO case, it does not even matter if the parent device has the OF node lifetime "linked" to it (as it actually happens for platform devices). The reason is that iio_dev only has a weak reference to it's parent and (I think) the parent can actually go away while the iio_dev is still around (eg: someone has an open fd to the iio_dev cdev). > >=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. >=20 > 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. >=20 > 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!) >=20 > This isn't a 'fix' I'd expect to rush in, or necessarily backport to > stable > but I think it's a valid fix. >=20 Should I drop the fixes tag? - Nuno S=C3=A1