Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753298AbbF3UY6 (ORCPT ); Tue, 30 Jun 2015 16:24:58 -0400 Received: from mail-wg0-f51.google.com ([74.125.82.51]:36276 "EHLO mail-wg0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752119AbbF3UWk (ORCPT ); Tue, 30 Jun 2015 16:22:40 -0400 From: Grant Likely Subject: Re: [PATCH/RFC 3/3] of/dynamic: Update list of aliases on aliases changes To: Geert Uytterhoeven , Pantelis Antoniou , Rob Herring Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Geert Uytterhoeven In-Reply-To: <1435675876-2159-4-git-send-email-geert+renesas@glider.be> References: <1435675876-2159-1-git-send-email-geert+renesas@glider.be> <1435675876-2159-4-git-send-email-geert+renesas@glider.be> Date: Tue, 30 Jun 2015 18:21:31 +0100 Message-Id: <20150630172131.D4E6CC4041A@trevor.secretlab.ca> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2846 Lines: 90 On Tue, 30 Jun 2015 16:51:16 +0200 , Geert Uytterhoeven wrote: > Currently the list of aliases is not updated when an overlay that > modifies /aliases is added or removed. This breaks drivers (e.g. serial) > that rely on of_alias_get_id(). > > Update the list of aliases when a property of the /aliases node is > added, removed, or updated. > > Signed-off-by: Geert Uytterhoeven > --- > - Should the OF core handle this itself, by registering a notifier > using of_reconfig_notifier_register()? Yes. Let's not add new hooks. > - Adding or destroying the /aliases node itself should be handled, > too. Yes > - Is it safe to deallocate struct alias_prop using kfree()? It may > have been allocated using early_init_dt_alloc_memory_arch() / > memblock_alloc(). What's the alternative? Leaking memory? Properties are not refcounted, so yes we leak memory. The memory remains owned by the aliases node, but because the aliases node is never freed, neither are any of the properties. Solving this isn't easy because it would require adding refcounting *everywhere* that properties are accessed. I think we have to just live with it until someone clever can some up with a solution. g. > --- > drivers/of/dynamic.c | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c > index 1901f8870591fe30..65dfb26f879c3a9a 100644 > --- a/drivers/of/dynamic.c > +++ b/drivers/of/dynamic.c > @@ -502,6 +502,16 @@ static void __of_changeset_entry_invert(struct of_changeset_entry *ce, > } > } > > +static void *alias_alloc(u64 size, u64 align) > +{ > + return kzalloc(size, GFP_KERNEL); > +} > + > +static void alias_free(void *p) > +{ > + kfree(p); > +} > + > static void __of_changeset_entry_notify(struct of_changeset_entry *ce, bool revert) > { > struct of_reconfig_data rd; > @@ -513,6 +523,20 @@ static void __of_changeset_entry_notify(struct of_changeset_entry *ce, bool reve > ce = &ce_inverted; > } > > + if (ce->np == of_aliases) { > + switch (ce->action) { > + case OF_RECONFIG_ADD_PROPERTY: > + of_alias_create(ce->prop, alias_alloc); > + break; > + case OF_RECONFIG_REMOVE_PROPERTY: > + of_alias_destroy(ce->prop->name, alias_free); > + break; > + case OF_RECONFIG_UPDATE_PROPERTY: > + of_alias_destroy(ce->old_prop->name, alias_free); > + of_alias_create(ce->prop, alias_alloc); > + break; > + } > + } > switch (ce->action) { > case OF_RECONFIG_ATTACH_NODE: > case OF_RECONFIG_DETACH_NODE: > -- > 1.9.1 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/