From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net [217.70.183.194]) (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 0D876762D9; Wed, 6 Mar 2024 11:07:21 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.70.183.194 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709723244; cv=none; b=GVo4ScYLU/sGVURcQAWe/tZEz83ke7pI1ktAUEHwvKYmGi7+CDVsn3hYVqbA4H5+cH5z8Q+IttxJitXhtfZFRGHeBW8PiZhuDTFUaJrWb6EYWC383veOOtpYAXCMYlRkJwWD7xtf02Ub/DLGp8OGF7p6RvcC6LqXzgCLooAqyM0= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1709723244; c=relaxed/simple; bh=dhUYkisSRgaEsKU54+0TtX8xvTK2vt6Z1QeO97lvAIQ=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=e8NZSW7cRyeSP3sYLEfRzvXb3PgyPnWoP12XOYEU5+XWR/kb0PRTW6JPDUIXWlXDLiG6C6Bs4Q3HZXFZ/rApSH+ZGflILk36ciVAY/iDJQuSn9Xl3sMx0EGsTEQb8V6C6hxdUnhzqTi8NWl/SfTFTlOax6tNqJCVLGlmSdaUKsA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=bootlin.com; spf=pass smtp.mailfrom=bootlin.com; dkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com header.b=Wlal6nzd; arc=none smtp.client-ip=217.70.183.194 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=bootlin.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=bootlin.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com header.b="Wlal6nzd" Received: by mail.gandi.net (Postfix) with ESMTPSA id 9772540006; Wed, 6 Mar 2024 11:07:18 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bootlin.com; s=gm1; t=1709723240; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=LkpSPLFwPA6sIr441GTPKMTLs5vOapZYIPq9ZxQdhg8=; b=Wlal6nzd3buzIfRLQsB5cqSN6nx1HoJ3I03GuUShz7k5AzVIlhhZC/dQJkiMZHplmw5yof UpOVZQRDUKyhnoanVLdafiTX3NSMPhUEbKzTbHfmJF8jyeK0M9UP6a4UrtHayFyGyMd1LC QdB+DLPHvRrphVoG5UHHTtm9Goze7bOlCWUrBN6kDe9ZHo8LUK5FSQOyIU1hBGqFV/mdq/ tDNGvii0WMUeOFmd8obQtT1y1wzKIMbJ0CUH+L+Y4fJjM8SuaOGlIcoubDNWGnwQl/igqd 3g1Tx7tHd3ztaOX9luNqHyTYCYGGES545n9dLDTlndXjMr+2KAg7hwnDvd7t3Q== Date: Wed, 6 Mar 2024 12:07:17 +0100 From: Luca Ceresoli To: Herve Codina Cc: Greg Kroah-Hartman , "Rafael J. Wysocki" , Rob Herring , Frank Rowand , Saravana Kannan , Lizhi Hou , Max Zhen , Sonal Santan , Stefano Stabellini , Jonathan Cameron , linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, Allan Nielsen , Horatiu Vultur , Steen Hegelund , Nuno Sa , Thomas Petazzoni , stable@vger.kernel.org Subject: Re: [PATCH v4 2/2] of: dynamic: Synchronize of_changeset_destroy() with the devlink removals Message-ID: <20240306120717.524bcd36@booty> In-Reply-To: <20240306085007.169771-3-herve.codina@bootlin.com> References: <20240306085007.169771-1-herve.codina@bootlin.com> <20240306085007.169771-3-herve.codina@bootlin.com> Organization: Bootlin X-Mailer: Claws Mail 4.0.0 (GTK+ 3.24.33; x86_64-pc-linux-gnu) Precedence: bulk X-Mailing-List: devicetree@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-GND-Sasl: luca.ceresoli@bootlin.com On Wed, 6 Mar 2024 09:50:03 +0100 Herve Codina wrote: > In the following sequence: > 1) of_platform_depopulate() > 2) of_overlay_remove() > > During the step 1, devices are destroyed and devlinks are removed. > During the step 2, OF nodes are destroyed but > __of_changeset_entry_destroy() can raise warnings related to missing > of_node_put(): > ERROR: memory leak, expected refcount 1 instead of 2 ... > > Indeed, during the devlink removals performed at step 1, the removal > itself releasing the device (and the attached of_node) is done by a job > queued in a workqueue and so, it is done asynchronously with respect to > function calls. > When the warning is present, of_node_put() will be called but wrongly > too late from the workqueue job. > > In order to be sure that any ongoing devlink removals are done before > the of_node destruction, synchronize the of_changeset_destroy() with the > devlink removals. > > Fixes: 80dd33cf72d1 ("drivers: base: Fix device link removal") > Cc: stable@vger.kernel.org > Signed-off-by: Herve Codina > --- > drivers/of/dynamic.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c > index 3bf27052832f..169e2a9ae22f 100644 > --- a/drivers/of/dynamic.c > +++ b/drivers/of/dynamic.c > @@ -9,6 +9,7 @@ > > #define pr_fmt(fmt) "OF: " fmt > > +#include > #include > #include > #include > @@ -667,6 +668,12 @@ void of_changeset_destroy(struct of_changeset *ocs) > { > struct of_changeset_entry *ce, *cen; > > + /* > + * Wait for any ongoing device link removals before destroying some of > + * nodes. > + */ > + device_link_wait_removal(); Tested-by: Luca Ceresoli And no problem appeared in my tests due to the removed unlock/lock around device_link_wait_removal(). Luca -- Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com