When I test the irqchip code of Loongson, I read the related code of other
chips in drivers/irqchip and I find some potential resource leaks in the
error path, I think it is better to fix them.
v2:
- Split the first patch into a new patch series which
includes small patches and add "Fixes" tag
- Use "goto" label to handle error path in some patches
v3:
- Add missed variable "ret" in the patch #5 and #13
v4:
- Modify the commit message of each patch suggested by Markus Elfring
- Make "irq_domain_remove(root_domain)" under CONFIG_SMP in patch #3
- Add a return statement before goto label in patch #4
v5:
- Modify the commit messages and do some code cleanups
Tiezhu Yang (14):
irqchip/ath79-misc: Fix potential resource leaks
irqchip/csky-apb-intc: Fix potential resource leaks
irqchip/csky-mpintc: Fix potential resource leaks
irqchip/davinci-aintc: Fix potential resource leaks
irqchip/davinci-cp-intc: Fix potential resource leaks
irqchip/digicolor: Fix potential resource leaks
irqchip/dw-apb-ictl: Fix potential resource leaks
irqchip/ls1x: Fix potential resource leaks
irqchip/mscc-ocelot: Fix potential resource leaks
irqchip/nvic: Fix potential resource leaks
irqchip/omap-intc: Fix potential resource leak
irqchip/riscv-intc: Fix potential resource leak
irqchip/s3c24xx: Fix potential resource leaks
irqchip/xilinx-intc: Fix potential resource leak
drivers/irqchip/irq-ath79-misc.c | 14 +++++++++++---
drivers/irqchip/irq-csky-apb-intc.c | 11 +++++++++--
drivers/irqchip/irq-csky-mpintc.c | 31 ++++++++++++++++++++++---------
drivers/irqchip/irq-davinci-aintc.c | 18 ++++++++++++++----
drivers/irqchip/irq-davinci-cp-intc.c | 18 +++++++++++++++---
drivers/irqchip/irq-digicolor.c | 14 +++++++++++---
drivers/irqchip/irq-dw-apb-ictl.c | 11 ++++++++---
drivers/irqchip/irq-ls1x.c | 4 +++-
drivers/irqchip/irq-mscc-ocelot.c | 6 ++++--
drivers/irqchip/irq-nvic.c | 12 +++++++++---
drivers/irqchip/irq-omap-intc.c | 4 +++-
drivers/irqchip/irq-riscv-intc.c | 1 +
drivers/irqchip/irq-s3c24xx.c | 23 +++++++++++++++++------
drivers/irqchip/irq-xilinx-intc.c | 11 ++++++-----
14 files changed, 133 insertions(+), 45 deletions(-)
--
2.1.0
In the function ocelot_irq_init(), system resource "parent_irq"
was not released in a few error cases. Thus add a jump target
for the completion of the desired exception handling.
Fixes: 19d99164480a ("irqchip: Add a driver for the Microsemi Ocelot controller")
Signed-off-by: Tiezhu Yang <[email protected]>
---
drivers/irqchip/irq-mscc-ocelot.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/irqchip/irq-mscc-ocelot.c b/drivers/irqchip/irq-mscc-ocelot.c
index 88143c0..e676ae2 100644
--- a/drivers/irqchip/irq-mscc-ocelot.c
+++ b/drivers/irqchip/irq-mscc-ocelot.c
@@ -73,7 +73,8 @@ static int __init ocelot_irq_init(struct device_node *node,
&irq_generic_chip_ops, NULL);
if (!domain) {
pr_err("%pOFn: unable to add irq domain\n", node);
- return -ENOMEM;
+ ret = -ENOMEM;
+ goto err_irq_dispose;
}
ret = irq_alloc_domain_generic_chips(domain, OCELOT_NR_IRQ, 1,
@@ -109,9 +110,10 @@ static int __init ocelot_irq_init(struct device_node *node,
err_gc_free:
irq_free_generic_chip(gc);
-
err_domain_remove:
irq_domain_remove(domain);
+err_irq_dispose:
+ irq_dispose_mapping(parent_irq);
return ret;
}
--
2.1.0
In the function omap_init_irq_of(), system resource "omap_irq_base"
was not released in an error case. Thus add a call of the function
"iounmap" in the if branch.
Fixes: 8598066cddd1 ("arm: omap: irq: move irq.c to drivers/irqchip/")
Signed-off-by: Tiezhu Yang <[email protected]>
---
drivers/irqchip/irq-omap-intc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/irqchip/irq-omap-intc.c b/drivers/irqchip/irq-omap-intc.c
index d360a6e..e711530 100644
--- a/drivers/irqchip/irq-omap-intc.c
+++ b/drivers/irqchip/irq-omap-intc.c
@@ -254,8 +254,10 @@ static int __init omap_init_irq_of(struct device_node *node)
omap_irq_soft_reset();
ret = omap_alloc_gc_of(domain, omap_irq_base);
- if (ret < 0)
+ if (ret < 0) {
irq_domain_remove(domain);
+ iounmap(omap_irq_base);
+ }
return ret;
}
--
2.1.0
In the function nvic_of_init(), system resource "nvic_base" and
"nvic_irq_domain" were not released in two error cases. Thus add
jump targets for the completion of the desired exception handling.
Fixes: 292ec080491d ("irqchip: Add support for ARMv7-M NVIC")
Signed-off-by: Tiezhu Yang <[email protected]>
---
drivers/irqchip/irq-nvic.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/irqchip/irq-nvic.c b/drivers/irqchip/irq-nvic.c
index f747e22..cd17f5d 100644
--- a/drivers/irqchip/irq-nvic.c
+++ b/drivers/irqchip/irq-nvic.c
@@ -94,7 +94,8 @@ static int __init nvic_of_init(struct device_node *node,
if (!nvic_irq_domain) {
pr_warn("Failed to allocate irq domain\n");
- return -ENOMEM;
+ ret = -ENOMEM;
+ goto err_iounmap;
}
ret = irq_alloc_domain_generic_chips(nvic_irq_domain, 32, 1,
@@ -102,8 +103,7 @@ static int __init nvic_of_init(struct device_node *node,
clr, 0, IRQ_GC_INIT_MASK_CACHE);
if (ret) {
pr_warn("Failed to allocate irq chips\n");
- irq_domain_remove(nvic_irq_domain);
- return ret;
+ goto err_domain_remove;
}
for (i = 0; i < numbanks; ++i) {
@@ -129,5 +129,11 @@ static int __init nvic_of_init(struct device_node *node,
writel_relaxed(0, nvic_base + NVIC_IPR + i);
return 0;
+
+err_domain_remove:
+ irq_domain_remove(nvic_irq_domain);
+err_iounmap:
+ iounmap(nvic_base);
+ return ret;
}
IRQCHIP_DECLARE(armv7m_nvic, "arm,armv7m-nvic", nvic_of_init);
--
2.1.0
In the function dw_apb_ictl_init(), system resources "irq" and "domain"
were not released in a few error cases. Thus add jump targets for the
completion of the desired exception handling.
Fixes: 350d71b94fc9 ("irqchip: add DesignWare APB ICTL interrupt controller")
Signed-off-by: Tiezhu Yang <[email protected]>
---
drivers/irqchip/irq-dw-apb-ictl.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/irqchip/irq-dw-apb-ictl.c b/drivers/irqchip/irq-dw-apb-ictl.c
index e4550e9..bc9b750 100644
--- a/drivers/irqchip/irq-dw-apb-ictl.c
+++ b/drivers/irqchip/irq-dw-apb-ictl.c
@@ -86,12 +86,13 @@ static int __init dw_apb_ictl_init(struct device_node *np,
ret = of_address_to_resource(np, 0, &r);
if (ret) {
pr_err("%pOF: unable to get resource\n", np);
- return ret;
+ goto err_irq_dispose;
}
if (!request_mem_region(r.start, resource_size(&r), np->full_name)) {
pr_err("%pOF: unable to request mem region\n", np);
- return -ENOMEM;
+ ret = -ENOMEM;
+ goto err_irq_dispose;
}
iobase = ioremap(r.start, resource_size(&r));
@@ -133,7 +134,7 @@ static int __init dw_apb_ictl_init(struct device_node *np,
IRQ_GC_INIT_MASK_CACHE);
if (ret) {
pr_err("%pOF: unable to alloc irq domain gc\n", np);
- goto err_unmap;
+ goto err_domain_remove;
}
for (i = 0; i < DIV_ROUND_UP(nrirqs, 32); i++) {
@@ -150,10 +151,14 @@ static int __init dw_apb_ictl_init(struct device_node *np,
return 0;
+err_domain_remove:
+ irq_domain_remove(domain);
err_unmap:
iounmap(iobase);
err_release:
release_mem_region(r.start, resource_size(&r));
+err_irq_dispose:
+ irq_dispose_mapping(irq);
return ret;
}
IRQCHIP_DECLARE(dw_apb_ictl,
--
2.1.0
In the function s3c_init_intc_of(), system resource "reg_base", "domain"
and "intc" were not released in a few error cases. Thus add jump targets
for the completion of the desired exception handling.
Fixes: f0774d41da0e ("irqchip: s3c24xx: add devicetree support")
Signed-off-by: Tiezhu Yang <[email protected]>
---
drivers/irqchip/irq-s3c24xx.c | 23 +++++++++++++++++------
1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/drivers/irqchip/irq-s3c24xx.c b/drivers/irqchip/irq-s3c24xx.c
index d2031fe..cf48e4b 100644
--- a/drivers/irqchip/irq-s3c24xx.c
+++ b/drivers/irqchip/irq-s3c24xx.c
@@ -1227,7 +1227,7 @@ static int __init s3c_init_intc_of(struct device_node *np,
struct s3c24xx_irq_of_ctrl *ctrl;
struct irq_domain *domain;
void __iomem *reg_base;
- int i;
+ int i, ret;
reg_base = of_iomap(np, 0);
if (!reg_base) {
@@ -1239,7 +1239,8 @@ static int __init s3c_init_intc_of(struct device_node *np,
&s3c24xx_irq_ops_of, NULL);
if (!domain) {
pr_err("irq: could not create irq-domain\n");
- return -EINVAL;
+ ret = -EINVAL;
+ goto err_iounmap;
}
for (i = 0; i < num_ctrl; i++) {
@@ -1248,15 +1249,17 @@ static int __init s3c_init_intc_of(struct device_node *np,
pr_debug("irq: found controller %s\n", ctrl->name);
intc = kzalloc(sizeof(struct s3c_irq_intc), GFP_KERNEL);
- if (!intc)
- return -ENOMEM;
+ if (!intc) {
+ ret = -ENOMEM;
+ goto err_domain_remove;
+ }
intc->domain = domain;
intc->irqs = kcalloc(32, sizeof(struct s3c_irq_data),
GFP_KERNEL);
if (!intc->irqs) {
- kfree(intc);
- return -ENOMEM;
+ ret = -ENOMEM;
+ goto err_free;
}
if (ctrl->parent) {
@@ -1285,6 +1288,14 @@ static int __init s3c_init_intc_of(struct device_node *np,
set_handle_irq(s3c24xx_handle_irq);
return 0;
+
+err_free:
+ kfree(intc);
+err_domain_remove:
+ irq_domain_remove(domain);
+err_iounmap:
+ iounmap(reg_base);
+ return ret;
}
static struct s3c24xx_irq_of_ctrl s3c2410_ctrl[] = {
--
2.1.0
In the function xilinx_intc_of_init(), system resource "irqc->root_domain"
was not released in an error case. Thus add a jump target to call the
function "irq_domain_remove" for the completion of the desired exception
handling.
Fixes: 9689c99e4950 ("irqchip/xilinx: Add support for parent intc")
Signed-off-by: Tiezhu Yang <[email protected]>
---
drivers/irqchip/irq-xilinx-intc.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/irqchip/irq-xilinx-intc.c b/drivers/irqchip/irq-xilinx-intc.c
index 1d3d273..41d9412 100644
--- a/drivers/irqchip/irq-xilinx-intc.c
+++ b/drivers/irqchip/irq-xilinx-intc.c
@@ -192,7 +192,7 @@ static int __init xilinx_intc_of_init(struct device_node *intc,
ret = of_property_read_u32(intc, "xlnx,num-intr-inputs", &irqc->nr_irq);
if (ret < 0) {
pr_err("irq-xilinx: unable to read xlnx,num-intr-inputs\n");
- goto error;
+ goto err_iounmap;
}
ret = of_property_read_u32(intc, "xlnx,kind-of-intr", &irqc->intr_mask);
@@ -229,7 +229,7 @@ static int __init xilinx_intc_of_init(struct device_node *intc,
if (!irqc->root_domain) {
pr_err("irq-xilinx: Unable to create IRQ domain\n");
ret = -EINVAL;
- goto error;
+ goto err_iounmap;
}
if (parent) {
@@ -241,7 +241,7 @@ static int __init xilinx_intc_of_init(struct device_node *intc,
} else {
pr_err("irq-xilinx: interrupts property not in DT\n");
ret = -EINVAL;
- goto error;
+ goto err_domain_remove;
}
} else {
primary_intc = irqc;
@@ -250,11 +250,12 @@ static int __init xilinx_intc_of_init(struct device_node *intc,
return 0;
-error:
+err_domain_remove:
+ irq_domain_remove(irqc->root_domain);
+err_iounmap:
iounmap(irqc->base);
kfree(irqc);
return ret;
-
}
IRQCHIP_DECLARE(xilinx_intc_xps, "xlnx,xps-intc-1.00.a", xilinx_intc_of_init);
--
2.1.0
In the function digicolor_of_init(), system resources "reg_base" and
"digicolor_irq_domain" were not released in a few error cases. Thus
add jump targets for the completion of the desired exception handling.
Fixes: 8041dfbd31cf ("irqchip: Conexant CX92755 interrupts controller driver")
Signed-off-by: Tiezhu Yang <[email protected]>
---
drivers/irqchip/irq-digicolor.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/irqchip/irq-digicolor.c b/drivers/irqchip/irq-digicolor.c
index fc38d2d..18c6e77 100644
--- a/drivers/irqchip/irq-digicolor.c
+++ b/drivers/irqchip/irq-digicolor.c
@@ -89,7 +89,8 @@ static int __init digicolor_of_init(struct device_node *node,
ucregs = syscon_regmap_lookup_by_phandle(node, "syscon");
if (IS_ERR(ucregs)) {
pr_err("%pOF: unable to map UC registers\n", node);
- return PTR_ERR(ucregs);
+ ret = PTR_ERR(ucregs);
+ goto err_iounmap;
}
/* channel 1, regular IRQs */
regmap_write(ucregs, UC_IRQ_CONTROL, 1);
@@ -98,7 +99,8 @@ static int __init digicolor_of_init(struct device_node *node,
irq_domain_add_linear(node, 64, &irq_generic_chip_ops, NULL);
if (!digicolor_irq_domain) {
pr_err("%pOF: unable to create IRQ domain\n", node);
- return -ENOMEM;
+ ret = -ENOMEM;
+ goto err_iounmap;
}
ret = irq_alloc_domain_generic_chips(digicolor_irq_domain, 32, 1,
@@ -106,7 +108,7 @@ static int __init digicolor_of_init(struct device_node *node,
clr, 0, 0);
if (ret) {
pr_err("%pOF: unable to allocate IRQ gc\n", node);
- return ret;
+ goto err_domain_remove;
}
digicolor_set_gc(reg_base, 0, IC_INT0ENABLE_LO, IC_FLAG_CLEAR_LO);
@@ -115,5 +117,11 @@ static int __init digicolor_of_init(struct device_node *node,
set_handle_irq(digicolor_handle_irq);
return 0;
+
+err_domain_remove:
+ irq_domain_remove(digicolor_irq_domain);
+err_iounmap:
+ iounmap(reg_base);
+ return ret;
}
IRQCHIP_DECLARE(conexant_digicolor_ic, "cnxt,cx92755-ic", digicolor_of_init);
--
2.1.0
In the function riscv_intc_init(), system resource "intc_domain"
was not released in an error case. Thus add a call of the function
"irq_domain_remove" in the if branch.
Fixes: 6b7ce8927b5a ("irqchip: RISC-V per-HART local interrupt controller driver")
Signed-off-by: Tiezhu Yang <[email protected]>
Reviewed-by: Anup Patel <[email protected]>
---
drivers/irqchip/irq-riscv-intc.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/irqchip/irq-riscv-intc.c b/drivers/irqchip/irq-riscv-intc.c
index a6f97fa..8d6286c 100644
--- a/drivers/irqchip/irq-riscv-intc.c
+++ b/drivers/irqchip/irq-riscv-intc.c
@@ -122,6 +122,7 @@ static int __init riscv_intc_init(struct device_node *node,
rc = set_handle_irq(&riscv_intc_irq);
if (rc) {
pr_err("failed to set irq handler\n");
+ irq_domain_remove(intc_domain);
return rc;
}
--
2.1.0
On 2020-07-06 02:19, Tiezhu Yang wrote:
> When I test the irqchip code of Loongson, I read the related code of
> other
> chips in drivers/irqchip and I find some potential resource leaks in
> the
> error path, I think it is better to fix them.
>
> v2:
> - Split the first patch into a new patch series which
> includes small patches and add "Fixes" tag
> - Use "goto" label to handle error path in some patches
>
> v3:
> - Add missed variable "ret" in the patch #5 and #13
>
> v4:
> - Modify the commit message of each patch suggested by Markus Elfring
> - Make "irq_domain_remove(root_domain)" under CONFIG_SMP in patch #3
> - Add a return statement before goto label in patch #4
>
> v5:
> - Modify the commit messages and do some code cleanups
Please stop replying to Markus Elfring, and give people who actually
care a chance to review this code. Elfring will keep asking you to make
absolutely pointless changes until you are blue in the face
Thanks,
M.
--
Jazz is not dead. It just smells funny...
> In the function xilinx_intc_of_init(), system resource "irqc->root_domain"
> was not released in an error case. Thus add a jump target to call the
> function "irq_domain_remove" for the completion of the desired exception
> handling.
Do the corresponding diff hunks express also a renaming of the label “error”?
Is there a need to split such an adjustment?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=dcb7fd82c75ee2d6e6f9d8cc71c52519ed52e258#n138
Regards,
Markus
>> v5:
>> - Modify the commit messages and do some code cleanups
>
> Please stop replying to Markus Elfring, and give people who actually
> care a chance to review this code.
You got the usual chances for the desired patch review.
> Elfring will keep asking you to make
> absolutely pointless changes until you are blue in the face
Can the circumstances evolve in ways under which you would admit
the relevance of (my) suggestions for possible software improvements?
Regards,
Markus
On 07/06/2020 03:30 PM, Marc Zyngier wrote:
> On 2020-07-06 02:19, Tiezhu Yang wrote:
>> When I test the irqchip code of Loongson, I read the related code of
>> other
>> chips in drivers/irqchip and I find some potential resource leaks in the
>> error path, I think it is better to fix them.
>>
>> v2:
>> - Split the first patch into a new patch series which
>> includes small patches and add "Fixes" tag
>> - Use "goto" label to handle error path in some patches
>>
>> v3:
>> - Add missed variable "ret" in the patch #5 and #13
>>
>> v4:
>> - Modify the commit message of each patch suggested by Markus Elfring
>> - Make "irq_domain_remove(root_domain)" under CONFIG_SMP in patch #3
>> - Add a return statement before goto label in patch #4
>>
>> v5:
>> - Modify the commit messages and do some code cleanups
>
> Please stop replying to Markus Elfring, and give people who actually
> care a chance to review this code. Elfring will keep asking you to make
> absolutely pointless changes until you are blue in the face
Hi Marc,
Any comments?
Could you please apply this patch series?
Thanks,
Tiezhu
>
>
> Thanks,
>
> M.
On 09/02/2020 11:59 AM, Tiezhu Yang wrote:
> On 07/06/2020 03:30 PM, Marc Zyngier wrote:
>> On 2020-07-06 02:19, Tiezhu Yang wrote:
>>> When I test the irqchip code of Loongson, I read the related code of
>>> other
>>> chips in drivers/irqchip and I find some potential resource leaks in
>>> the
>>> error path, I think it is better to fix them.
>>>
>>> v2:
>>> - Split the first patch into a new patch series which
>>> includes small patches and add "Fixes" tag
>>> - Use "goto" label to handle error path in some patches
>>>
>>> v3:
>>> - Add missed variable "ret" in the patch #5 and #13
>>>
>>> v4:
>>> - Modify the commit message of each patch suggested by Markus Elfring
>>> - Make "irq_domain_remove(root_domain)" under CONFIG_SMP in patch #3
>>> - Add a return statement before goto label in patch #4
>>>
>>> v5:
>>> - Modify the commit messages and do some code cleanups
>>
>> Please stop replying to Markus Elfring, and give people who actually
>> care a chance to review this code. Elfring will keep asking you to make
>> absolutely pointless changes until you are blue in the face
>
> Hi Marc,
>
> Any comments?
> Could you please apply this patch series?
Hi all,
Maybe I should cc the related persons through ./scripts/get_maintainer.pl
to get Acked-by or Reviewed-by.
The cover letter link of this patch series is:
[v5,00/14] irqchip: Fix potential resource leaks
https://lore.kernel.org/patchwork/cover/1268043/
Any comments will be much appreciated.
Thanks,
Tiezhu
>
> Thanks,
> Tiezhu
>
>>
>>
>> Thanks,
>>
>> M.
>