Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751905AbdGYRxa (ORCPT ); Tue, 25 Jul 2017 13:53:30 -0400 Received: from mail-qt0-f195.google.com ([209.85.216.195]:34245 "EHLO mail-qt0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751420AbdGYRx1 (ORCPT ); Tue, 25 Jul 2017 13:53:27 -0400 Subject: Re: [PATCH] powerpc/pseries: Fix of_node_put() underflow during pseries remove To: Michael Ellerman , Laurent Vivier , linux-kernel@vger.kernel.org Cc: David Gibson , linuxppc-dev@lists.ozlabs.org, Tyrel Datwyler , Thomas Huth References: <20170721145139.9384-1-lvivier@redhat.com> <87tw22jeud.fsf@concordia.ellerman.id.au> <87inih6s23.fsf@concordia.ellerman.id.au> From: Tyrel Datwyler Message-ID: Date: Tue, 25 Jul 2017 10:53:23 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.0 MIME-Version: 1.0 In-Reply-To: <87inih6s23.fsf@concordia.ellerman.id.au> Content-Type: text/plain; charset=utf-8 Content-Language: en-GB Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1550 Lines: 44 On 07/24/2017 09:47 PM, Michael Ellerman wrote: > Tyrel Datwyler writes: > >> On 07/24/2017 03:42 AM, Michael Ellerman wrote: >>> Laurent Vivier writes: >>> >>>> As for commit 68baf692c435 ("powerpc/pseries: Fix of_node_put() >>>> underflow during DLPAR remove"), the call to of_node_put() >>>> must be removed from pSeries_reconfig_remove_node(). >>>> >>>> dlpar_detach_node() and pSeries_reconfig_remove_node() call >>>> of_detach_node(), and thus the node should not be released >>>> in this case too. >>>> >>>> Signed-off-by: Laurent Vivier >>>> --- >>>> arch/powerpc/platforms/pseries/reconfig.c | 1 - >>>> 1 file changed, 1 deletion(-) >>> >>> Thanks. I'll spare you the swearing about why we have the same bug in >>> two places. >> >> That's probably my bad. I must have failed to test with older powerpc-util tooling where >> drmgr uses the /proc/ofdt interface for device tree modification. > > OK. Really we should have automated tests of the various cases, I've > just never had time to write any. Agreed, some better CI is warranted. > > Mainly the thing that bugs me is that we still have the two separate > paths. Or if we must maintain both they could at least share more code, > the two functions do basically the same thing AFAICS. Yeah, I think that is where I dropped the ball. I wrongly assumed by not looking close enough that code was shared in those two paths. Definitely some code de-duplication work that can be done. -Tyrel > > cheers >