| Message ID | 20220615153339.3970658-1-windhl@126.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | mips: lantiq: Add missing of_node_put() in irq.c | expand |
On Wed, Jun 15, 2022 at 11:33:39PM +0800, Liang He wrote: > In icu_of_init(), of_find_compatible_node() will return a node > pointer with refcount incremented. We should use of_node_put() > when it is not used anymore. > > Signed-off-by: Liang He <windhl@126.com> > --- > arch/mips/lantiq/irq.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/arch/mips/lantiq/irq.c b/arch/mips/lantiq/irq.c > index b732495f138a..62f1b20a2169 100644 > --- a/arch/mips/lantiq/irq.c > +++ b/arch/mips/lantiq/irq.c > @@ -396,6 +396,9 @@ int __init icu_of_init(struct device_node *node, struct device_node *parent) > > ret = of_property_read_u32_array(eiu_node, "lantiq,eiu-irqs", > ltq_eiu_irq, exin_avail); > + trailing whitespaces > + of_node_put(eiu_node); > + trailing whitespaces > if (ret) > panic("failed to load external irq resources"); > > @@ -409,6 +412,9 @@ int __init icu_of_init(struct device_node *node, struct device_node *parent) > panic("Failed to remap eiu memory"); > } > > + /* if eiu_node&of_address_to_resource */ > + of_node_put(eiu_node); > + if I'm not mistaken you are freeing the taken reference twice. Shouldn't it work by only adding the last of_node_put() ? Thomas.
At 2022-06-21 23:21:19, "Thomas Bogendoerfer" <tsbogend@alpha.franken.de> wrote: >On Wed, Jun 15, 2022 at 11:33:39PM +0800, Liang He wrote: >> In icu_of_init(), of_find_compatible_node() will return a node >> pointer with refcount incremented. We should use of_node_put() >> when it is not used anymore. >> >> Signed-off-by: Liang He <windhl@126.com> >> --- >> arch/mips/lantiq/irq.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/arch/mips/lantiq/irq.c b/arch/mips/lantiq/irq.c >> index b732495f138a..62f1b20a2169 100644 >> --- a/arch/mips/lantiq/irq.c >> +++ b/arch/mips/lantiq/irq.c >> @@ -396,6 +396,9 @@ int __init icu_of_init(struct device_node *node, struct device_node *parent) >> >> ret = of_property_read_u32_array(eiu_node, "lantiq,eiu-irqs", >> ltq_eiu_irq, exin_avail); >> + > >trailing whitespaces > >> + of_node_put(eiu_node); >> + > >trailing whitespaces > >> if (ret) >> panic("failed to load external irq resources"); >> >> @@ -409,6 +412,9 @@ int __init icu_of_init(struct device_node *node, struct device_node *parent) >> panic("Failed to remap eiu memory"); >> } >> >> + /* if eiu_node&of_address_to_resource */ >> + of_node_put(eiu_node); >> + > >if I'm not mistaken you are freeing the taken reference twice. Shouldn't >it work by only adding the last of_node_put() ? > >Thomas. Hi, Thomas. Thanks very much for your effort to review and apply my patches. You are right, this patch is wrong and only the last put is needed. I will send a new patch: removing trailing whitspace and the first put. Thans again. Liang > >-- >Crap can work. Given enough thrust pigs will fly, but it's not necessarily a >good idea. [ RFC1925, 2.3 ]
On 21/06/2022 17:32, Liang He wrote: > > > At 2022-06-21 23:21:19, "Thomas Bogendoerfer" <tsbogend@alpha.franken.de> wrote: >> On Wed, Jun 15, 2022 at 11:33:39PM +0800, Liang He wrote: >>> In icu_of_init(), of_find_compatible_node() will return a node >>> pointer with refcount incremented. We should use of_node_put() >>> when it is not used anymore. >>> >>> Signed-off-by: Liang He <windhl@126.com> >>> --- >>> arch/mips/lantiq/irq.c | 6 ++++++ >>> 1 file changed, 6 insertions(+) >>> >>> diff --git a/arch/mips/lantiq/irq.c b/arch/mips/lantiq/irq.c >>> index b732495f138a..62f1b20a2169 100644 >>> --- a/arch/mips/lantiq/irq.c >>> +++ b/arch/mips/lantiq/irq.c >>> @@ -396,6 +396,9 @@ int __init icu_of_init(struct device_node *node, struct device_node *parent) >>> >>> ret = of_property_read_u32_array(eiu_node, "lantiq,eiu-irqs", >>> ltq_eiu_irq, exin_avail); >>> + >> >> trailing whitespaces >> >>> + of_node_put(eiu_node); >>> + >> >> trailing whitespaces >> >>> if (ret) >>> panic("failed to load external irq resources"); >>> >>> @@ -409,6 +412,9 @@ int __init icu_of_init(struct device_node *node, struct device_node *parent) >>> panic("Failed to remap eiu memory"); >>> } >>> >>> + /* if eiu_node&of_address_to_resource */ >>> + of_node_put(eiu_node); >>> + >> >> if I'm not mistaken you are freeing the taken reference twice. Shouldn't >> it work by only adding the last of_node_put() ? >> >> Thomas. > > Hi, Thomas. > > Thanks very much for your effort to review and apply my patches. > > You are right, this patch is wrong and only the last put is needed. > > I will send a new patch: removing trailing whitspace and the first put. > > Thans again. Thomas, Before applying the patch please check it carefully. Previous evidence [1][2] suggests that not it was not even compiled. [1] https://lore.kernel.org/all/202206221602.odN70SHs-lkp@intel.com/ [2] https://lore.kernel.org/all/16f9a971.44e5.1817068ee3c.Coremail.windhl@126.com/ Best regards, Krzysztof
diff --git a/arch/mips/lantiq/irq.c b/arch/mips/lantiq/irq.c index b732495f138a..62f1b20a2169 100644 --- a/arch/mips/lantiq/irq.c +++ b/arch/mips/lantiq/irq.c @@ -396,6 +396,9 @@ int __init icu_of_init(struct device_node *node, struct device_node *parent) ret = of_property_read_u32_array(eiu_node, "lantiq,eiu-irqs", ltq_eiu_irq, exin_avail); + + of_node_put(eiu_node); + if (ret) panic("failed to load external irq resources"); @@ -409,6 +412,9 @@ int __init icu_of_init(struct device_node *node, struct device_node *parent) panic("Failed to remap eiu memory"); } + /* if eiu_node&of_address_to_resource */ + of_node_put(eiu_node); + return 0; }
In icu_of_init(), of_find_compatible_node() will return a node pointer with refcount incremented. We should use of_node_put() when it is not used anymore. Signed-off-by: Liang He <windhl@126.com> --- arch/mips/lantiq/irq.c | 6 ++++++ 1 file changed, 6 insertions(+)