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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id D3A41C43334 for ; Sat, 11 Jun 2022 14:50:06 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236071AbiFKOuG (ORCPT ); Sat, 11 Jun 2022 10:50:06 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51174 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233571AbiFKOuF (ORCPT ); Sat, 11 Jun 2022 10:50:05 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 10D7D5584; Sat, 11 Jun 2022 07:50:05 -0700 (PDT) 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 dfw.source.kernel.org (Postfix) with ESMTPS id A0D2D60FF7; 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) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-mips@vger.kernel.org +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