From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tony Lindgren Subject: Re: [PATCH v2] ARM: OMAP2+: Fix device node reference counts Date: Wed, 1 Mar 2017 10:04:39 -0800 Message-ID: <20170301180439.GI20572@atomide.com> References: <1488311640-3594-1-git-send-email-linux@roeck-us.net> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <1488311640-3594-1-git-send-email-linux@roeck-us.net> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: Guenter Roeck Cc: Rob Herring , Paul Walmsley , Russell King , linux-kernel@vger.kernel.org, Qi Hou , linux-omap@vger.kernel.org, Peter Rosin , linux-arm-kernel@lists.infradead.org List-Id: linux-omap@vger.kernel.org * Guenter Roeck [170228 11:55]: > After commit 'of: fix of_node leak caused in of_find_node_opts_by_path', > the following error may be reported when running omap images. > > OF: ERROR: Bad of_node_put() on /ocp@68000000 > CPU: 0 PID: 0 Comm: swapper Not tainted 4.10.0-rc7-next-20170210 #1 > Hardware name: Generic OMAP3-GP (Flattened Device Tree) > [] (unwind_backtrace) from [] (show_stack+0x10/0x14) > [] (show_stack) from [] (dump_stack+0x98/0xac) > [] (dump_stack) from [] (kobject_release+0x48/0x7c) > [] (kobject_release) > from [] (of_find_node_by_name+0x74/0x94) > [] (of_find_node_by_name) > from [] (omap3xxx_hwmod_is_hs_ip_block_usable+0x24/0x2c) > [] (omap3xxx_hwmod_is_hs_ip_block_usable) from > [] (omap3xxx_hwmod_init+0x180/0x274) > [] (omap3xxx_hwmod_init) > from [] (omap3_init_early+0xa0/0x11c) > [] (omap3_init_early) > from [] (omap3430_init_early+0x8/0x30) > [] (omap3430_init_early) > from [] (setup_arch+0xc04/0xc34) > [] (setup_arch) from [] (start_kernel+0x68/0x38c) > [] (start_kernel) from [<8020807c>] (0x8020807c) > > of_find_node_by_name() drops the reference to the passed device node. > Use of_get_child_by_name() instead. Also, release references to > device nodes obtained with of_find_node_by_name() and > of_get_child_by_name() after they are no longer needed. > > While at it, clean up the code and change the return type of > omap3xxx_hwmod_is_hs_ip_block_usable() to bool to match its use > and the return type of of_device_is_available(). > > Cc: Qi Hou > Cc: Peter Rosin > Cc: Rob Herring > Signed-off-by: Guenter Roeck > --- > v2: Change subject ('Grab reference to device nodes where needed' > didn't really cover all the changes made) > Use of_get_child_by_name() instead of of_find_node_by_name() > Drop references to device nodes as needed > Change return type of omap3xxx_hwmod_is_hs_ip_block_usable() > to bool OK so now we have a commit id for it and should have: Fixes: 0549bde0fcb1 ("of: fix of_node leak caused in of_find_node_opts_by_path") What about other leaky users of of_find_node_by_name() and of_get_child_by_name()? We have those in: control.c display.c omap_device.c omap_hwmod.c So probably it really should be two fixes, one for the regression causing the error. Then another one for fixing the leaky np use. Sorry for going back and forth here, I guess I did not full understand the second part of the change in your earlier version of the patch when you asked if it should be two patches. Regards, Tony From mboxrd@z Thu Jan 1 00:00:00 1970 From: tony@atomide.com (Tony Lindgren) Date: Wed, 1 Mar 2017 10:04:39 -0800 Subject: [PATCH v2] ARM: OMAP2+: Fix device node reference counts In-Reply-To: <1488311640-3594-1-git-send-email-linux@roeck-us.net> References: <1488311640-3594-1-git-send-email-linux@roeck-us.net> Message-ID: <20170301180439.GI20572@atomide.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org * Guenter Roeck [170228 11:55]: > After commit 'of: fix of_node leak caused in of_find_node_opts_by_path', > the following error may be reported when running omap images. > > OF: ERROR: Bad of_node_put() on /ocp at 68000000 > CPU: 0 PID: 0 Comm: swapper Not tainted 4.10.0-rc7-next-20170210 #1 > Hardware name: Generic OMAP3-GP (Flattened Device Tree) > [] (unwind_backtrace) from [] (show_stack+0x10/0x14) > [] (show_stack) from [] (dump_stack+0x98/0xac) > [] (dump_stack) from [] (kobject_release+0x48/0x7c) > [] (kobject_release) > from [] (of_find_node_by_name+0x74/0x94) > [] (of_find_node_by_name) > from [] (omap3xxx_hwmod_is_hs_ip_block_usable+0x24/0x2c) > [] (omap3xxx_hwmod_is_hs_ip_block_usable) from > [] (omap3xxx_hwmod_init+0x180/0x274) > [] (omap3xxx_hwmod_init) > from [] (omap3_init_early+0xa0/0x11c) > [] (omap3_init_early) > from [] (omap3430_init_early+0x8/0x30) > [] (omap3430_init_early) > from [] (setup_arch+0xc04/0xc34) > [] (setup_arch) from [] (start_kernel+0x68/0x38c) > [] (start_kernel) from [<8020807c>] (0x8020807c) > > of_find_node_by_name() drops the reference to the passed device node. > Use of_get_child_by_name() instead. Also, release references to > device nodes obtained with of_find_node_by_name() and > of_get_child_by_name() after they are no longer needed. > > While at it, clean up the code and change the return type of > omap3xxx_hwmod_is_hs_ip_block_usable() to bool to match its use > and the return type of of_device_is_available(). > > Cc: Qi Hou > Cc: Peter Rosin > Cc: Rob Herring > Signed-off-by: Guenter Roeck > --- > v2: Change subject ('Grab reference to device nodes where needed' > didn't really cover all the changes made) > Use of_get_child_by_name() instead of of_find_node_by_name() > Drop references to device nodes as needed > Change return type of omap3xxx_hwmod_is_hs_ip_block_usable() > to bool OK so now we have a commit id for it and should have: Fixes: 0549bde0fcb1 ("of: fix of_node leak caused in of_find_node_opts_by_path") What about other leaky users of of_find_node_by_name() and of_get_child_by_name()? We have those in: control.c display.c omap_device.c omap_hwmod.c So probably it really should be two fixes, one for the regression causing the error. Then another one for fixing the leaky np use. Sorry for going back and forth here, I guess I did not full understand the second part of the change in your earlier version of the patch when you asked if it should be two patches. Regards, Tony From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752934AbdCATAM (ORCPT ); Wed, 1 Mar 2017 14:00:12 -0500 Received: from muru.com ([72.249.23.125]:37114 "EHLO muru.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752001AbdCATAJ (ORCPT ); Wed, 1 Mar 2017 14:00:09 -0500 Date: Wed, 1 Mar 2017 10:04:39 -0800 From: Tony Lindgren To: Guenter Roeck Cc: Paul Walmsley , Russell King , linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Qi Hou , Peter Rosin , Rob Herring Subject: Re: [PATCH v2] ARM: OMAP2+: Fix device node reference counts Message-ID: <20170301180439.GI20572@atomide.com> References: <1488311640-3594-1-git-send-email-linux@roeck-us.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1488311640-3594-1-git-send-email-linux@roeck-us.net> User-Agent: Mutt/1.7.2 (2016-11-26) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Guenter Roeck [170228 11:55]: > After commit 'of: fix of_node leak caused in of_find_node_opts_by_path', > the following error may be reported when running omap images. > > OF: ERROR: Bad of_node_put() on /ocp@68000000 > CPU: 0 PID: 0 Comm: swapper Not tainted 4.10.0-rc7-next-20170210 #1 > Hardware name: Generic OMAP3-GP (Flattened Device Tree) > [] (unwind_backtrace) from [] (show_stack+0x10/0x14) > [] (show_stack) from [] (dump_stack+0x98/0xac) > [] (dump_stack) from [] (kobject_release+0x48/0x7c) > [] (kobject_release) > from [] (of_find_node_by_name+0x74/0x94) > [] (of_find_node_by_name) > from [] (omap3xxx_hwmod_is_hs_ip_block_usable+0x24/0x2c) > [] (omap3xxx_hwmod_is_hs_ip_block_usable) from > [] (omap3xxx_hwmod_init+0x180/0x274) > [] (omap3xxx_hwmod_init) > from [] (omap3_init_early+0xa0/0x11c) > [] (omap3_init_early) > from [] (omap3430_init_early+0x8/0x30) > [] (omap3430_init_early) > from [] (setup_arch+0xc04/0xc34) > [] (setup_arch) from [] (start_kernel+0x68/0x38c) > [] (start_kernel) from [<8020807c>] (0x8020807c) > > of_find_node_by_name() drops the reference to the passed device node. > Use of_get_child_by_name() instead. Also, release references to > device nodes obtained with of_find_node_by_name() and > of_get_child_by_name() after they are no longer needed. > > While at it, clean up the code and change the return type of > omap3xxx_hwmod_is_hs_ip_block_usable() to bool to match its use > and the return type of of_device_is_available(). > > Cc: Qi Hou > Cc: Peter Rosin > Cc: Rob Herring > Signed-off-by: Guenter Roeck > --- > v2: Change subject ('Grab reference to device nodes where needed' > didn't really cover all the changes made) > Use of_get_child_by_name() instead of of_find_node_by_name() > Drop references to device nodes as needed > Change return type of omap3xxx_hwmod_is_hs_ip_block_usable() > to bool OK so now we have a commit id for it and should have: Fixes: 0549bde0fcb1 ("of: fix of_node leak caused in of_find_node_opts_by_path") What about other leaky users of of_find_node_by_name() and of_get_child_by_name()? We have those in: control.c display.c omap_device.c omap_hwmod.c So probably it really should be two fixes, one for the regression causing the error. Then another one for fixing the leaky np use. Sorry for going back and forth here, I guess I did not full understand the second part of the change in your earlier version of the patch when you asked if it should be two patches. Regards, Tony