* [PATCH] PCI: mediatek-gen3: Fix refcount leak in mtk_pcie_init_irq_domains
@ 2022-05-26 11:02 ` Miaoqian Lin
0 siblings, 0 replies; 24+ messages in thread
From: Miaoqian Lin @ 2022-05-26 11:02 UTC (permalink / raw)
To: Ryder Lee, Jianjun Wang, Lorenzo Pieralisi, Rob Herring,
Krzysztof Wilczyński, Bjorn Helgaas, Matthias Brugger,
Marc Zyngier, linux-pci, linux-mediatek, linux-kernel,
linux-arm-kernel
Cc: linmq006
of_get_child_by_name() returns a node pointer with refcount
incremented, we should use of_node_put() on it when not need anymore.
Add missing of_node_put() to avoid refcount leak.
Fixes: 814cceebba9b ("PCI: mediatek-gen3: Add INTx support")
Signed-off-by: Miaoqian Lin <linmq006@gmail.com>
---
drivers/pci/controller/pcie-mediatek-gen3.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c
index 3e8d70bfabc6..da8e9db0abdf 100644
--- a/drivers/pci/controller/pcie-mediatek-gen3.c
+++ b/drivers/pci/controller/pcie-mediatek-gen3.c
@@ -600,6 +600,7 @@ static int mtk_pcie_init_irq_domains(struct mtk_gen3_pcie *pcie)
&intx_domain_ops, pcie);
if (!pcie->intx_domain) {
dev_err(dev, "failed to create INTx IRQ domain\n");
+ of_node_put(intc_node);
return -ENODEV;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH] PCI: mediatek-gen3: Fix refcount leak in mtk_pcie_init_irq_domains
@ 2022-05-26 11:02 ` Miaoqian Lin
0 siblings, 0 replies; 24+ messages in thread
From: Miaoqian Lin @ 2022-05-26 11:02 UTC (permalink / raw)
To: Ryder Lee, Jianjun Wang, Lorenzo Pieralisi, Rob Herring,
Krzysztof Wilczyński, Bjorn Helgaas, Matthias Brugger,
Marc Zyngier, linux-pci, linux-mediatek, linux-kernel,
linux-arm-kernel
Cc: linmq006
of_get_child_by_name() returns a node pointer with refcount
incremented, we should use of_node_put() on it when not need anymore.
Add missing of_node_put() to avoid refcount leak.
Fixes: 814cceebba9b ("PCI: mediatek-gen3: Add INTx support")
Signed-off-by: Miaoqian Lin <linmq006@gmail.com>
---
drivers/pci/controller/pcie-mediatek-gen3.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c
index 3e8d70bfabc6..da8e9db0abdf 100644
--- a/drivers/pci/controller/pcie-mediatek-gen3.c
+++ b/drivers/pci/controller/pcie-mediatek-gen3.c
@@ -600,6 +600,7 @@ static int mtk_pcie_init_irq_domains(struct mtk_gen3_pcie *pcie)
&intx_domain_ops, pcie);
if (!pcie->intx_domain) {
dev_err(dev, "failed to create INTx IRQ domain\n");
+ of_node_put(intc_node);
return -ENODEV;
}
--
2.25.1
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH] PCI: mediatek-gen3: Fix refcount leak in mtk_pcie_init_irq_domains
@ 2022-05-26 11:02 ` Miaoqian Lin
0 siblings, 0 replies; 24+ messages in thread
From: Miaoqian Lin @ 2022-05-26 11:02 UTC (permalink / raw)
To: Ryder Lee, Jianjun Wang, Lorenzo Pieralisi, Rob Herring,
Krzysztof Wilczyński, Bjorn Helgaas, Matthias Brugger,
Marc Zyngier, linux-pci, linux-mediatek, linux-kernel,
linux-arm-kernel
Cc: linmq006
of_get_child_by_name() returns a node pointer with refcount
incremented, we should use of_node_put() on it when not need anymore.
Add missing of_node_put() to avoid refcount leak.
Fixes: 814cceebba9b ("PCI: mediatek-gen3: Add INTx support")
Signed-off-by: Miaoqian Lin <linmq006@gmail.com>
---
drivers/pci/controller/pcie-mediatek-gen3.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c
index 3e8d70bfabc6..da8e9db0abdf 100644
--- a/drivers/pci/controller/pcie-mediatek-gen3.c
+++ b/drivers/pci/controller/pcie-mediatek-gen3.c
@@ -600,6 +600,7 @@ static int mtk_pcie_init_irq_domains(struct mtk_gen3_pcie *pcie)
&intx_domain_ops, pcie);
if (!pcie->intx_domain) {
dev_err(dev, "failed to create INTx IRQ domain\n");
+ of_node_put(intc_node);
return -ENODEV;
}
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH] PCI: mediatek-gen3: Fix refcount leak in mtk_pcie_init_irq_domains
2022-05-26 11:02 ` Miaoqian Lin
(?)
@ 2022-05-26 18:44 ` Rob Herring
-1 siblings, 0 replies; 24+ messages in thread
From: Rob Herring @ 2022-05-26 18:44 UTC (permalink / raw)
To: Miaoqian Lin
Cc: Ryder Lee, Jianjun Wang, Lorenzo Pieralisi,
Krzysztof Wilczyński, Bjorn Helgaas, Matthias Brugger,
Marc Zyngier, linux-pci, linux-mediatek, linux-kernel,
linux-arm-kernel
On Thu, May 26, 2022 at 03:02:46PM +0400, Miaoqian Lin wrote:
> of_get_child_by_name() returns a node pointer with refcount
> incremented, we should use of_node_put() on it when not need anymore.
> Add missing of_node_put() to avoid refcount leak.
>
> Fixes: 814cceebba9b ("PCI: mediatek-gen3: Add INTx support")
> Signed-off-by: Miaoqian Lin <linmq006@gmail.com>
> ---
> drivers/pci/controller/pcie-mediatek-gen3.c | 1 +
> 1 file changed, 1 insertion(+)
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] PCI: mediatek-gen3: Fix refcount leak in mtk_pcie_init_irq_domains
@ 2022-05-26 18:44 ` Rob Herring
0 siblings, 0 replies; 24+ messages in thread
From: Rob Herring @ 2022-05-26 18:44 UTC (permalink / raw)
To: Miaoqian Lin
Cc: Ryder Lee, Jianjun Wang, Lorenzo Pieralisi,
Krzysztof Wilczyński, Bjorn Helgaas, Matthias Brugger,
Marc Zyngier, linux-pci, linux-mediatek, linux-kernel,
linux-arm-kernel
On Thu, May 26, 2022 at 03:02:46PM +0400, Miaoqian Lin wrote:
> of_get_child_by_name() returns a node pointer with refcount
> incremented, we should use of_node_put() on it when not need anymore.
> Add missing of_node_put() to avoid refcount leak.
>
> Fixes: 814cceebba9b ("PCI: mediatek-gen3: Add INTx support")
> Signed-off-by: Miaoqian Lin <linmq006@gmail.com>
> ---
> drivers/pci/controller/pcie-mediatek-gen3.c | 1 +
> 1 file changed, 1 insertion(+)
Acked-by: Rob Herring <robh@kernel.org>
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] PCI: mediatek-gen3: Fix refcount leak in mtk_pcie_init_irq_domains
@ 2022-05-26 18:44 ` Rob Herring
0 siblings, 0 replies; 24+ messages in thread
From: Rob Herring @ 2022-05-26 18:44 UTC (permalink / raw)
To: Miaoqian Lin
Cc: Ryder Lee, Jianjun Wang, Lorenzo Pieralisi,
Krzysztof Wilczyński, Bjorn Helgaas, Matthias Brugger,
Marc Zyngier, linux-pci, linux-mediatek, linux-kernel,
linux-arm-kernel
On Thu, May 26, 2022 at 03:02:46PM +0400, Miaoqian Lin wrote:
> of_get_child_by_name() returns a node pointer with refcount
> incremented, we should use of_node_put() on it when not need anymore.
> Add missing of_node_put() to avoid refcount leak.
>
> Fixes: 814cceebba9b ("PCI: mediatek-gen3: Add INTx support")
> Signed-off-by: Miaoqian Lin <linmq006@gmail.com>
> ---
> drivers/pci/controller/pcie-mediatek-gen3.c | 1 +
> 1 file changed, 1 insertion(+)
Acked-by: Rob Herring <robh@kernel.org>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] PCI: mediatek-gen3: Fix refcount leak in mtk_pcie_init_irq_domains
2022-05-26 11:02 ` Miaoqian Lin
(?)
@ 2022-05-27 8:45 ` Miles Chen
-1 siblings, 0 replies; 24+ messages in thread
From: Miles Chen @ 2022-05-27 8:45 UTC (permalink / raw)
To: linmq006
Cc: bhelgaas, jianjun.wang, kw, linux-arm-kernel, linux-kernel,
linux-mediatek, linux-pci, lorenzo.pieralisi, matthias.bgg, maz,
robh, ryder.lee
Hi Miaoqian,
>Signed-off-by: Miaoqian Lin <linmq006@gmail.com>
>---
> drivers/pci/controller/pcie-mediatek-gen3.c | 1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c
>index 3e8d70bfabc6..da8e9db0abdf 100644
>--- a/drivers/pci/controller/pcie-mediatek-gen3.c
>+++ b/drivers/pci/controller/pcie-mediatek-gen3.c
>@@ -600,6 +600,7 @@ static int mtk_pcie_init_irq_domains(struct mtk_gen3_pcie *pcie)
> &intx_domain_ops, pcie);
> if (!pcie->intx_domain) {
> dev_err(dev, "failed to create INTx IRQ domain\n");
>+ of_node_put(intc_node);
> return -ENODEV;
> }
Thanks for doing this.
I checked mtk_pcie_init_irq_domains() and there are multiple exit paths like
err_msi_domain and err_msi_bottom_domain and the normal path which also
need of_node_put(intc_node).
Maybe we can move the of_node_put(intc_node) to #54 below and cover
all possible paths?
cheers,
Miles
e.g.,
static int mtk_pcie_init_irq_domains(struct mtk_gen3_pcie *pcie)
{
struct device *dev = pcie->dev;
struct device_node *intc_node, *node = dev->of_node;
int ret;
raw_spin_lock_init(&pcie->irq_lock);
/* Setup INTx */
intc_node = of_get_child_by_name(node, "interrupt-controller");
if (!intc_node) {
dev_err(dev, "missing interrupt-controller node\n");
return -ENODEV;
}
pcie->intx_domain = irq_domain_add_linear(intc_node, PCI_NUM_INTX,
&intx_domain_ops, pcie);
of_node_put(intc_node);
if (!pcie->intx_domain) {
dev_err(dev, "failed to create INTx IRQ domain\n");
return -ENODEV;
}
/* Setup MSI */
mutex_init(&pcie->lock);
pcie->msi_bottom_domain = irq_domain_add_linear(node, PCIE_MSI_IRQS_NUM,
&mtk_msi_bottom_domain_ops, pcie);
if (!pcie->msi_bottom_domain) {
dev_err(dev, "failed to create MSI bottom domain\n");
ret = -ENODEV;
goto err_msi_bottom_domain;
}
pcie->msi_domain = pci_msi_create_irq_domain(dev->fwnode,
&mtk_msi_domain_info,
pcie->msi_bottom_domain);
if (!pcie->msi_domain) {
dev_err(dev, "failed to create MSI domain\n");
ret = -ENODEV;
goto err_msi_domain;
}
return 0;
err_msi_domain:
irq_domain_remove(pcie->msi_bottom_domain);
err_msi_bottom_domain:
irq_domain_remove(pcie->intx_domain);
return ret;
}
>
>--
>2.25.1
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] PCI: mediatek-gen3: Fix refcount leak in mtk_pcie_init_irq_domains
@ 2022-05-27 8:45 ` Miles Chen
0 siblings, 0 replies; 24+ messages in thread
From: Miles Chen @ 2022-05-27 8:45 UTC (permalink / raw)
To: linmq006
Cc: bhelgaas, jianjun.wang, kw, linux-arm-kernel, linux-kernel,
linux-mediatek, linux-pci, lorenzo.pieralisi, matthias.bgg, maz,
robh, ryder.lee
Hi Miaoqian,
>Signed-off-by: Miaoqian Lin <linmq006@gmail.com>
>---
> drivers/pci/controller/pcie-mediatek-gen3.c | 1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c
>index 3e8d70bfabc6..da8e9db0abdf 100644
>--- a/drivers/pci/controller/pcie-mediatek-gen3.c
>+++ b/drivers/pci/controller/pcie-mediatek-gen3.c
>@@ -600,6 +600,7 @@ static int mtk_pcie_init_irq_domains(struct mtk_gen3_pcie *pcie)
> &intx_domain_ops, pcie);
> if (!pcie->intx_domain) {
> dev_err(dev, "failed to create INTx IRQ domain\n");
>+ of_node_put(intc_node);
> return -ENODEV;
> }
Thanks for doing this.
I checked mtk_pcie_init_irq_domains() and there are multiple exit paths like
err_msi_domain and err_msi_bottom_domain and the normal path which also
need of_node_put(intc_node).
Maybe we can move the of_node_put(intc_node) to #54 below and cover
all possible paths?
cheers,
Miles
e.g.,
static int mtk_pcie_init_irq_domains(struct mtk_gen3_pcie *pcie)
{
struct device *dev = pcie->dev;
struct device_node *intc_node, *node = dev->of_node;
int ret;
raw_spin_lock_init(&pcie->irq_lock);
/* Setup INTx */
intc_node = of_get_child_by_name(node, "interrupt-controller");
if (!intc_node) {
dev_err(dev, "missing interrupt-controller node\n");
return -ENODEV;
}
pcie->intx_domain = irq_domain_add_linear(intc_node, PCI_NUM_INTX,
&intx_domain_ops, pcie);
of_node_put(intc_node);
if (!pcie->intx_domain) {
dev_err(dev, "failed to create INTx IRQ domain\n");
return -ENODEV;
}
/* Setup MSI */
mutex_init(&pcie->lock);
pcie->msi_bottom_domain = irq_domain_add_linear(node, PCIE_MSI_IRQS_NUM,
&mtk_msi_bottom_domain_ops, pcie);
if (!pcie->msi_bottom_domain) {
dev_err(dev, "failed to create MSI bottom domain\n");
ret = -ENODEV;
goto err_msi_bottom_domain;
}
pcie->msi_domain = pci_msi_create_irq_domain(dev->fwnode,
&mtk_msi_domain_info,
pcie->msi_bottom_domain);
if (!pcie->msi_domain) {
dev_err(dev, "failed to create MSI domain\n");
ret = -ENODEV;
goto err_msi_domain;
}
return 0;
err_msi_domain:
irq_domain_remove(pcie->msi_bottom_domain);
err_msi_bottom_domain:
irq_domain_remove(pcie->intx_domain);
return ret;
}
>
>--
>2.25.1
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] PCI: mediatek-gen3: Fix refcount leak in mtk_pcie_init_irq_domains
@ 2022-05-27 8:45 ` Miles Chen
0 siblings, 0 replies; 24+ messages in thread
From: Miles Chen @ 2022-05-27 8:45 UTC (permalink / raw)
To: linmq006
Cc: bhelgaas, jianjun.wang, kw, linux-arm-kernel, linux-kernel,
linux-mediatek, linux-pci, lorenzo.pieralisi, matthias.bgg, maz,
robh, ryder.lee
Hi Miaoqian,
>Signed-off-by: Miaoqian Lin <linmq006@gmail.com>
>---
> drivers/pci/controller/pcie-mediatek-gen3.c | 1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c
>index 3e8d70bfabc6..da8e9db0abdf 100644
>--- a/drivers/pci/controller/pcie-mediatek-gen3.c
>+++ b/drivers/pci/controller/pcie-mediatek-gen3.c
>@@ -600,6 +600,7 @@ static int mtk_pcie_init_irq_domains(struct mtk_gen3_pcie *pcie)
> &intx_domain_ops, pcie);
> if (!pcie->intx_domain) {
> dev_err(dev, "failed to create INTx IRQ domain\n");
>+ of_node_put(intc_node);
> return -ENODEV;
> }
Thanks for doing this.
I checked mtk_pcie_init_irq_domains() and there are multiple exit paths like
err_msi_domain and err_msi_bottom_domain and the normal path which also
need of_node_put(intc_node).
Maybe we can move the of_node_put(intc_node) to #54 below and cover
all possible paths?
cheers,
Miles
e.g.,
static int mtk_pcie_init_irq_domains(struct mtk_gen3_pcie *pcie)
{
struct device *dev = pcie->dev;
struct device_node *intc_node, *node = dev->of_node;
int ret;
raw_spin_lock_init(&pcie->irq_lock);
/* Setup INTx */
intc_node = of_get_child_by_name(node, "interrupt-controller");
if (!intc_node) {
dev_err(dev, "missing interrupt-controller node\n");
return -ENODEV;
}
pcie->intx_domain = irq_domain_add_linear(intc_node, PCI_NUM_INTX,
&intx_domain_ops, pcie);
of_node_put(intc_node);
if (!pcie->intx_domain) {
dev_err(dev, "failed to create INTx IRQ domain\n");
return -ENODEV;
}
/* Setup MSI */
mutex_init(&pcie->lock);
pcie->msi_bottom_domain = irq_domain_add_linear(node, PCIE_MSI_IRQS_NUM,
&mtk_msi_bottom_domain_ops, pcie);
if (!pcie->msi_bottom_domain) {
dev_err(dev, "failed to create MSI bottom domain\n");
ret = -ENODEV;
goto err_msi_bottom_domain;
}
pcie->msi_domain = pci_msi_create_irq_domain(dev->fwnode,
&mtk_msi_domain_info,
pcie->msi_bottom_domain);
if (!pcie->msi_domain) {
dev_err(dev, "failed to create MSI domain\n");
ret = -ENODEV;
goto err_msi_domain;
}
return 0;
err_msi_domain:
irq_domain_remove(pcie->msi_bottom_domain);
err_msi_bottom_domain:
irq_domain_remove(pcie->intx_domain);
return ret;
}
>
>--
>2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] PCI: mediatek-gen3: Fix refcount leak in mtk_pcie_init_irq_domains
2022-05-27 8:45 ` Miles Chen
(?)
@ 2022-05-28 9:19 ` Miaoqian Lin
-1 siblings, 0 replies; 24+ messages in thread
From: Miaoqian Lin @ 2022-05-28 9:19 UTC (permalink / raw)
To: Miles Chen
Cc: bhelgaas, jianjun.wang, kw, linux-arm-kernel, linux-kernel,
linux-mediatek, linux-pci, lorenzo.pieralisi, matthias.bgg, maz,
robh, ryder.lee
Hi Miles,
On 2022/5/27 16:45, Miles Chen wrote:
> Hi Miaoqian,
>
>> Signed-off-by: Miaoqian Lin <linmq006@gmail.com>
>> ---
>> drivers/pci/controller/pcie-mediatek-gen3.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c
>> index 3e8d70bfabc6..da8e9db0abdf 100644
>> --- a/drivers/pci/controller/pcie-mediatek-gen3.c
>> +++ b/drivers/pci/controller/pcie-mediatek-gen3.c
>> @@ -600,6 +600,7 @@ static int mtk_pcie_init_irq_domains(struct mtk_gen3_pcie *pcie)
>> &intx_domain_ops, pcie);
>> if (!pcie->intx_domain) {
>> dev_err(dev, "failed to create INTx IRQ domain\n");
>> + of_node_put(intc_node);
>> return -ENODEV;
>> }
> Thanks for doing this.
>
> I checked mtk_pcie_init_irq_domains() and there are multiple exit paths like
> err_msi_domain and err_msi_bottom_domain and the normal path which also
> need of_node_put(intc_node).
Thanks for your reply,
I didn't add of_node_put() in other paths because I am not sure if the reference passed through irq_domain_add_linear(), since intc_node is passed to irq_domain_add_linear().
__irq_domain_add() keeps &node->fwnode in the irq_domain structure.
and use fwnode_handle_get() to get the reference of fwnode, but I still uncertain.
If the reference don't needed anymore after irq_domain_add_linear(),
your suggestion looks fine, and I will submit v2.
> Maybe we can move the of_node_put(intc_node) to #54 below and cover
> all possible paths?
>
>
> cheers,
> Miles
>
> e.g.,
>
> static int mtk_pcie_init_irq_domains(struct mtk_gen3_pcie *pcie)
> {
> struct device *dev = pcie->dev;
> struct device_node *intc_node, *node = dev->of_node;
> int ret;
>
> raw_spin_lock_init(&pcie->irq_lock);
>
> /* Setup INTx */
> intc_node = of_get_child_by_name(node, "interrupt-controller");
> if (!intc_node) {
> dev_err(dev, "missing interrupt-controller node\n");
> return -ENODEV;
> }
>
> pcie->intx_domain = irq_domain_add_linear(intc_node, PCI_NUM_INTX,
> &intx_domain_ops, pcie);
> of_node_put(intc_node);
> if (!pcie->intx_domain) {
> dev_err(dev, "failed to create INTx IRQ domain\n");
> return -ENODEV;
> }
>
> /* Setup MSI */
> mutex_init(&pcie->lock);
>
> pcie->msi_bottom_domain = irq_domain_add_linear(node, PCIE_MSI_IRQS_NUM,
> &mtk_msi_bottom_domain_ops, pcie);
> if (!pcie->msi_bottom_domain) {
> dev_err(dev, "failed to create MSI bottom domain\n");
> ret = -ENODEV;
> goto err_msi_bottom_domain;
> }
>
> pcie->msi_domain = pci_msi_create_irq_domain(dev->fwnode,
> &mtk_msi_domain_info,
> pcie->msi_bottom_domain);
> if (!pcie->msi_domain) {
> dev_err(dev, "failed to create MSI domain\n");
> ret = -ENODEV;
> goto err_msi_domain;
> }
>
> return 0;
>
> err_msi_domain:
> irq_domain_remove(pcie->msi_bottom_domain);
> err_msi_bottom_domain:
> irq_domain_remove(pcie->intx_domain);
>
> return ret;
> }
>> --
>> 2.25.1
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] PCI: mediatek-gen3: Fix refcount leak in mtk_pcie_init_irq_domains
@ 2022-05-28 9:19 ` Miaoqian Lin
0 siblings, 0 replies; 24+ messages in thread
From: Miaoqian Lin @ 2022-05-28 9:19 UTC (permalink / raw)
To: Miles Chen
Cc: bhelgaas, jianjun.wang, kw, linux-arm-kernel, linux-kernel,
linux-mediatek, linux-pci, lorenzo.pieralisi, matthias.bgg, maz,
robh, ryder.lee
Hi Miles,
On 2022/5/27 16:45, Miles Chen wrote:
> Hi Miaoqian,
>
>> Signed-off-by: Miaoqian Lin <linmq006@gmail.com>
>> ---
>> drivers/pci/controller/pcie-mediatek-gen3.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c
>> index 3e8d70bfabc6..da8e9db0abdf 100644
>> --- a/drivers/pci/controller/pcie-mediatek-gen3.c
>> +++ b/drivers/pci/controller/pcie-mediatek-gen3.c
>> @@ -600,6 +600,7 @@ static int mtk_pcie_init_irq_domains(struct mtk_gen3_pcie *pcie)
>> &intx_domain_ops, pcie);
>> if (!pcie->intx_domain) {
>> dev_err(dev, "failed to create INTx IRQ domain\n");
>> + of_node_put(intc_node);
>> return -ENODEV;
>> }
> Thanks for doing this.
>
> I checked mtk_pcie_init_irq_domains() and there are multiple exit paths like
> err_msi_domain and err_msi_bottom_domain and the normal path which also
> need of_node_put(intc_node).
Thanks for your reply,
I didn't add of_node_put() in other paths because I am not sure if the reference passed through irq_domain_add_linear(), since intc_node is passed to irq_domain_add_linear().
__irq_domain_add() keeps &node->fwnode in the irq_domain structure.
and use fwnode_handle_get() to get the reference of fwnode, but I still uncertain.
If the reference don't needed anymore after irq_domain_add_linear(),
your suggestion looks fine, and I will submit v2.
> Maybe we can move the of_node_put(intc_node) to #54 below and cover
> all possible paths?
>
>
> cheers,
> Miles
>
> e.g.,
>
> static int mtk_pcie_init_irq_domains(struct mtk_gen3_pcie *pcie)
> {
> struct device *dev = pcie->dev;
> struct device_node *intc_node, *node = dev->of_node;
> int ret;
>
> raw_spin_lock_init(&pcie->irq_lock);
>
> /* Setup INTx */
> intc_node = of_get_child_by_name(node, "interrupt-controller");
> if (!intc_node) {
> dev_err(dev, "missing interrupt-controller node\n");
> return -ENODEV;
> }
>
> pcie->intx_domain = irq_domain_add_linear(intc_node, PCI_NUM_INTX,
> &intx_domain_ops, pcie);
> of_node_put(intc_node);
> if (!pcie->intx_domain) {
> dev_err(dev, "failed to create INTx IRQ domain\n");
> return -ENODEV;
> }
>
> /* Setup MSI */
> mutex_init(&pcie->lock);
>
> pcie->msi_bottom_domain = irq_domain_add_linear(node, PCIE_MSI_IRQS_NUM,
> &mtk_msi_bottom_domain_ops, pcie);
> if (!pcie->msi_bottom_domain) {
> dev_err(dev, "failed to create MSI bottom domain\n");
> ret = -ENODEV;
> goto err_msi_bottom_domain;
> }
>
> pcie->msi_domain = pci_msi_create_irq_domain(dev->fwnode,
> &mtk_msi_domain_info,
> pcie->msi_bottom_domain);
> if (!pcie->msi_domain) {
> dev_err(dev, "failed to create MSI domain\n");
> ret = -ENODEV;
> goto err_msi_domain;
> }
>
> return 0;
>
> err_msi_domain:
> irq_domain_remove(pcie->msi_bottom_domain);
> err_msi_bottom_domain:
> irq_domain_remove(pcie->intx_domain);
>
> return ret;
> }
>> --
>> 2.25.1
>
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] PCI: mediatek-gen3: Fix refcount leak in mtk_pcie_init_irq_domains
@ 2022-05-28 9:19 ` Miaoqian Lin
0 siblings, 0 replies; 24+ messages in thread
From: Miaoqian Lin @ 2022-05-28 9:19 UTC (permalink / raw)
To: Miles Chen
Cc: bhelgaas, jianjun.wang, kw, linux-arm-kernel, linux-kernel,
linux-mediatek, linux-pci, lorenzo.pieralisi, matthias.bgg, maz,
robh, ryder.lee
Hi Miles,
On 2022/5/27 16:45, Miles Chen wrote:
> Hi Miaoqian,
>
>> Signed-off-by: Miaoqian Lin <linmq006@gmail.com>
>> ---
>> drivers/pci/controller/pcie-mediatek-gen3.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c
>> index 3e8d70bfabc6..da8e9db0abdf 100644
>> --- a/drivers/pci/controller/pcie-mediatek-gen3.c
>> +++ b/drivers/pci/controller/pcie-mediatek-gen3.c
>> @@ -600,6 +600,7 @@ static int mtk_pcie_init_irq_domains(struct mtk_gen3_pcie *pcie)
>> &intx_domain_ops, pcie);
>> if (!pcie->intx_domain) {
>> dev_err(dev, "failed to create INTx IRQ domain\n");
>> + of_node_put(intc_node);
>> return -ENODEV;
>> }
> Thanks for doing this.
>
> I checked mtk_pcie_init_irq_domains() and there are multiple exit paths like
> err_msi_domain and err_msi_bottom_domain and the normal path which also
> need of_node_put(intc_node).
Thanks for your reply,
I didn't add of_node_put() in other paths because I am not sure if the reference passed through irq_domain_add_linear(), since intc_node is passed to irq_domain_add_linear().
__irq_domain_add() keeps &node->fwnode in the irq_domain structure.
and use fwnode_handle_get() to get the reference of fwnode, but I still uncertain.
If the reference don't needed anymore after irq_domain_add_linear(),
your suggestion looks fine, and I will submit v2.
> Maybe we can move the of_node_put(intc_node) to #54 below and cover
> all possible paths?
>
>
> cheers,
> Miles
>
> e.g.,
>
> static int mtk_pcie_init_irq_domains(struct mtk_gen3_pcie *pcie)
> {
> struct device *dev = pcie->dev;
> struct device_node *intc_node, *node = dev->of_node;
> int ret;
>
> raw_spin_lock_init(&pcie->irq_lock);
>
> /* Setup INTx */
> intc_node = of_get_child_by_name(node, "interrupt-controller");
> if (!intc_node) {
> dev_err(dev, "missing interrupt-controller node\n");
> return -ENODEV;
> }
>
> pcie->intx_domain = irq_domain_add_linear(intc_node, PCI_NUM_INTX,
> &intx_domain_ops, pcie);
> of_node_put(intc_node);
> if (!pcie->intx_domain) {
> dev_err(dev, "failed to create INTx IRQ domain\n");
> return -ENODEV;
> }
>
> /* Setup MSI */
> mutex_init(&pcie->lock);
>
> pcie->msi_bottom_domain = irq_domain_add_linear(node, PCIE_MSI_IRQS_NUM,
> &mtk_msi_bottom_domain_ops, pcie);
> if (!pcie->msi_bottom_domain) {
> dev_err(dev, "failed to create MSI bottom domain\n");
> ret = -ENODEV;
> goto err_msi_bottom_domain;
> }
>
> pcie->msi_domain = pci_msi_create_irq_domain(dev->fwnode,
> &mtk_msi_domain_info,
> pcie->msi_bottom_domain);
> if (!pcie->msi_domain) {
> dev_err(dev, "failed to create MSI domain\n");
> ret = -ENODEV;
> goto err_msi_domain;
> }
>
> return 0;
>
> err_msi_domain:
> irq_domain_remove(pcie->msi_bottom_domain);
> err_msi_bottom_domain:
> irq_domain_remove(pcie->intx_domain);
>
> return ret;
> }
>> --
>> 2.25.1
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] PCI: mediatek-gen3: Fix refcount leak in mtk_pcie_init_irq_domains
2022-05-28 9:19 ` Miaoqian Lin
(?)
@ 2022-05-30 2:19 ` Miles Chen
-1 siblings, 0 replies; 24+ messages in thread
From: Miles Chen @ 2022-05-30 2:19 UTC (permalink / raw)
To: linmq006
Cc: bhelgaas, jianjun.wang, kw, linux-arm-kernel, linux-kernel,
linux-mediatek, linux-pci, lorenzo.pieralisi, matthias.bgg, maz,
miles.chen, robh, ryder.lee
Hi Miaoqian,
>>> &intx_domain_ops, pcie);
>>> if (!pcie->intx_domain) {
>>> dev_err(dev, "failed to create INTx IRQ domain\n");
>>> + of_node_put(intc_node);
>>> return -ENODEV;
>>> }
>> Thanks for doing this.
>>
>> I checked mtk_pcie_init_irq_domains() and there are multiple exit paths like
>> err_msi_domain and err_msi_bottom_domain and the normal path which also
>> need of_node_put(intc_node).
>
>Thanks for your reply,
>
>I didn't add of_node_put() in other paths because I am not sure if the reference passed through irq_domain_add_linear(), since intc_node is passed to irq_domain_add_linear().
>
>__irq_domain_add() keeps &node->fwnode in the irq_domain structure.
>
>and use fwnode_handle_get() to get the reference of fwnode, but I still uncertain.
>
>If the reference don't needed anymore after irq_domain_add_linear(),
>
>your suggestion looks fine, and I will submit v2.
Thanks for your reply, I think we can do similar things like
rtl8365mb_irq_setup() in drivers/net/dsa/realtek/rtl8365mb.c
Thanks,
Miles
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] PCI: mediatek-gen3: Fix refcount leak in mtk_pcie_init_irq_domains
@ 2022-05-30 2:19 ` Miles Chen
0 siblings, 0 replies; 24+ messages in thread
From: Miles Chen @ 2022-05-30 2:19 UTC (permalink / raw)
To: linmq006
Cc: bhelgaas, jianjun.wang, kw, linux-arm-kernel, linux-kernel,
linux-mediatek, linux-pci, lorenzo.pieralisi, matthias.bgg, maz,
miles.chen, robh, ryder.lee
Hi Miaoqian,
>>> &intx_domain_ops, pcie);
>>> if (!pcie->intx_domain) {
>>> dev_err(dev, "failed to create INTx IRQ domain\n");
>>> + of_node_put(intc_node);
>>> return -ENODEV;
>>> }
>> Thanks for doing this.
>>
>> I checked mtk_pcie_init_irq_domains() and there are multiple exit paths like
>> err_msi_domain and err_msi_bottom_domain and the normal path which also
>> need of_node_put(intc_node).
>
>Thanks for your reply,
>
>I didn't add of_node_put() in other paths because I am not sure if the reference passed through irq_domain_add_linear(), since intc_node is passed to irq_domain_add_linear().
>
>__irq_domain_add() keeps &node->fwnode in the irq_domain structure.
>
>and use fwnode_handle_get() to get the reference of fwnode, but I still uncertain.
>
>If the reference don't needed anymore after irq_domain_add_linear(),
>
>your suggestion looks fine, and I will submit v2.
Thanks for your reply, I think we can do similar things like
rtl8365mb_irq_setup() in drivers/net/dsa/realtek/rtl8365mb.c
Thanks,
Miles
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] PCI: mediatek-gen3: Fix refcount leak in mtk_pcie_init_irq_domains
@ 2022-05-30 2:19 ` Miles Chen
0 siblings, 0 replies; 24+ messages in thread
From: Miles Chen @ 2022-05-30 2:19 UTC (permalink / raw)
To: linmq006
Cc: bhelgaas, jianjun.wang, kw, linux-arm-kernel, linux-kernel,
linux-mediatek, linux-pci, lorenzo.pieralisi, matthias.bgg, maz,
miles.chen, robh, ryder.lee
Hi Miaoqian,
>>> &intx_domain_ops, pcie);
>>> if (!pcie->intx_domain) {
>>> dev_err(dev, "failed to create INTx IRQ domain\n");
>>> + of_node_put(intc_node);
>>> return -ENODEV;
>>> }
>> Thanks for doing this.
>>
>> I checked mtk_pcie_init_irq_domains() and there are multiple exit paths like
>> err_msi_domain and err_msi_bottom_domain and the normal path which also
>> need of_node_put(intc_node).
>
>Thanks for your reply,
>
>I didn't add of_node_put() in other paths because I am not sure if the reference passed through irq_domain_add_linear(), since intc_node is passed to irq_domain_add_linear().
>
>__irq_domain_add() keeps &node->fwnode in the irq_domain structure.
>
>and use fwnode_handle_get() to get the reference of fwnode, but I still uncertain.
>
>If the reference don't needed anymore after irq_domain_add_linear(),
>
>your suggestion looks fine, and I will submit v2.
Thanks for your reply, I think we can do similar things like
rtl8365mb_irq_setup() in drivers/net/dsa/realtek/rtl8365mb.c
Thanks,
Miles
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] PCI: mediatek-gen3: Fix refcount leak in mtk_pcie_init_irq_domains
2022-05-30 2:19 ` Miles Chen
(?)
@ 2022-05-30 6:54 ` Miaoqian Lin
-1 siblings, 0 replies; 24+ messages in thread
From: Miaoqian Lin @ 2022-05-30 6:54 UTC (permalink / raw)
To: Miles Chen
Cc: bhelgaas, jianjun.wang, kw, linux-arm-kernel, linux-kernel,
linux-mediatek, linux-pci, lorenzo.pieralisi, matthias.bgg, maz,
robh, ryder.lee
Hi, Miles
On 2022/5/30 10:19, Miles Chen wrote:
> Hi Miaoqian,
>
>>>> &intx_domain_ops, pcie);
>>>> if (!pcie->intx_domain) {
>>>> dev_err(dev, "failed to create INTx IRQ domain\n");
>>>> + of_node_put(intc_node);
>>>> return -ENODEV;
>>>> }
>>> Thanks for doing this.
>>>
>>> I checked mtk_pcie_init_irq_domains() and there are multiple exit paths like
>>> err_msi_domain and err_msi_bottom_domain and the normal path which also
>>> need of_node_put(intc_node).
>> Thanks for your reply,
>>
>> I didn't add of_node_put() in other paths because I am not sure if the reference passed through irq_domain_add_linear(), since intc_node is passed to irq_domain_add_linear().
>>
>> __irq_domain_add() keeps &node->fwnode in the irq_domain structure.
>>
>> and use fwnode_handle_get() to get the reference of fwnode, but I still uncertain.
>>
>> If the reference don't needed anymore after irq_domain_add_linear(),
>>
>> your suggestion looks fine, and I will submit v2.
>
> Thanks for your reply, I think we can do similar things like
> rtl8365mb_irq_setup() in drivers/net/dsa/realtek/rtl8365mb.c
I checked rtl8365mb_irq_setup(), it calls of_node_put() by goto statement for error paths.
and calls of_node_put() before return 0 in normal path. I didn't see the same problem.
> Thanks,
> Miles
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] PCI: mediatek-gen3: Fix refcount leak in mtk_pcie_init_irq_domains
@ 2022-05-30 6:54 ` Miaoqian Lin
0 siblings, 0 replies; 24+ messages in thread
From: Miaoqian Lin @ 2022-05-30 6:54 UTC (permalink / raw)
To: Miles Chen
Cc: bhelgaas, jianjun.wang, kw, linux-arm-kernel, linux-kernel,
linux-mediatek, linux-pci, lorenzo.pieralisi, matthias.bgg, maz,
robh, ryder.lee
Hi, Miles
On 2022/5/30 10:19, Miles Chen wrote:
> Hi Miaoqian,
>
>>>> &intx_domain_ops, pcie);
>>>> if (!pcie->intx_domain) {
>>>> dev_err(dev, "failed to create INTx IRQ domain\n");
>>>> + of_node_put(intc_node);
>>>> return -ENODEV;
>>>> }
>>> Thanks for doing this.
>>>
>>> I checked mtk_pcie_init_irq_domains() and there are multiple exit paths like
>>> err_msi_domain and err_msi_bottom_domain and the normal path which also
>>> need of_node_put(intc_node).
>> Thanks for your reply,
>>
>> I didn't add of_node_put() in other paths because I am not sure if the reference passed through irq_domain_add_linear(), since intc_node is passed to irq_domain_add_linear().
>>
>> __irq_domain_add() keeps &node->fwnode in the irq_domain structure.
>>
>> and use fwnode_handle_get() to get the reference of fwnode, but I still uncertain.
>>
>> If the reference don't needed anymore after irq_domain_add_linear(),
>>
>> your suggestion looks fine, and I will submit v2.
>
> Thanks for your reply, I think we can do similar things like
> rtl8365mb_irq_setup() in drivers/net/dsa/realtek/rtl8365mb.c
I checked rtl8365mb_irq_setup(), it calls of_node_put() by goto statement for error paths.
and calls of_node_put() before return 0 in normal path. I didn't see the same problem.
> Thanks,
> Miles
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] PCI: mediatek-gen3: Fix refcount leak in mtk_pcie_init_irq_domains
@ 2022-05-30 6:54 ` Miaoqian Lin
0 siblings, 0 replies; 24+ messages in thread
From: Miaoqian Lin @ 2022-05-30 6:54 UTC (permalink / raw)
To: Miles Chen
Cc: bhelgaas, jianjun.wang, kw, linux-arm-kernel, linux-kernel,
linux-mediatek, linux-pci, lorenzo.pieralisi, matthias.bgg, maz,
robh, ryder.lee
Hi, Miles
On 2022/5/30 10:19, Miles Chen wrote:
> Hi Miaoqian,
>
>>>> &intx_domain_ops, pcie);
>>>> if (!pcie->intx_domain) {
>>>> dev_err(dev, "failed to create INTx IRQ domain\n");
>>>> + of_node_put(intc_node);
>>>> return -ENODEV;
>>>> }
>>> Thanks for doing this.
>>>
>>> I checked mtk_pcie_init_irq_domains() and there are multiple exit paths like
>>> err_msi_domain and err_msi_bottom_domain and the normal path which also
>>> need of_node_put(intc_node).
>> Thanks for your reply,
>>
>> I didn't add of_node_put() in other paths because I am not sure if the reference passed through irq_domain_add_linear(), since intc_node is passed to irq_domain_add_linear().
>>
>> __irq_domain_add() keeps &node->fwnode in the irq_domain structure.
>>
>> and use fwnode_handle_get() to get the reference of fwnode, but I still uncertain.
>>
>> If the reference don't needed anymore after irq_domain_add_linear(),
>>
>> your suggestion looks fine, and I will submit v2.
>
> Thanks for your reply, I think we can do similar things like
> rtl8365mb_irq_setup() in drivers/net/dsa/realtek/rtl8365mb.c
I checked rtl8365mb_irq_setup(), it calls of_node_put() by goto statement for error paths.
and calls of_node_put() before return 0 in normal path. I didn't see the same problem.
> Thanks,
> Miles
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] PCI: mediatek-gen3: Fix refcount leak in mtk_pcie_init_irq_domains
2022-05-30 6:54 ` Miaoqian Lin
(?)
@ 2022-05-30 7:35 ` Miles Chen
-1 siblings, 0 replies; 24+ messages in thread
From: Miles Chen @ 2022-05-30 7:35 UTC (permalink / raw)
To: linmq006
Cc: bhelgaas, jianjun.wang, kw, linux-arm-kernel, linux-kernel,
linux-mediatek, linux-pci, lorenzo.pieralisi, matthias.bgg, maz,
miles.chen, robh, ryder.lee
Hi Miaoqian,
>Hi, Miles
>
>On 2022/5/30 10:19, Miles Chen wrote:
>> Hi Miaoqian,
>>
>>>>> &intx_domain_ops, pcie);
>>>>> if (!pcie->intx_domain) {
>>>>> dev_err(dev, "failed to create INTx IRQ domain\n");
>>>>> + of_node_put(intc_node);
>>>>> return -ENODEV;
>>>>> }
>>>> Thanks for doing this.
>>>>
>>>> I checked mtk_pcie_init_irq_domains() and there are multiple exit paths like
>>>> err_msi_domain and err_msi_bottom_domain and the normal path which also
>>>> need of_node_put(intc_node).
>>> Thanks for your reply,
>>>
>>> I didn't add of_node_put() in other paths because I am not sure if the reference passed through irq_domain_add_linear(), since intc_node is passed to irq_domain_add_linear().
>>>
>>> __irq_domain_add() keeps &node->fwnode in the irq_domain structure.
>>>
>>> and use fwnode_handle_get() to get the reference of fwnode, but I still uncertain.
>>>
>>> If the reference don't needed anymore after irq_domain_add_linear(),
>>>
>>> your suggestion looks fine, and I will submit v2.
>>
>> Thanks for your reply, I think we can do similar things like
>> rtl8365mb_irq_setup() in drivers/net/dsa/realtek/rtl8365mb.c
>
>I checked rtl8365mb_irq_setup(), it calls of_node_put() by goto statement for error paths.
>
>and calls of_node_put() before return 0 in normal path. I didn't see the same problem.
Sorry for the confusing. I meant that we can do the same thing -
it calls of_node_put() by goto statement for error paths
and calls of_node_put() before return 0 in normal path. :-)
Thanks,
Miles
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] PCI: mediatek-gen3: Fix refcount leak in mtk_pcie_init_irq_domains
@ 2022-05-30 7:35 ` Miles Chen
0 siblings, 0 replies; 24+ messages in thread
From: Miles Chen @ 2022-05-30 7:35 UTC (permalink / raw)
To: linmq006
Cc: bhelgaas, jianjun.wang, kw, linux-arm-kernel, linux-kernel,
linux-mediatek, linux-pci, lorenzo.pieralisi, matthias.bgg, maz,
miles.chen, robh, ryder.lee
Hi Miaoqian,
>Hi, Miles
>
>On 2022/5/30 10:19, Miles Chen wrote:
>> Hi Miaoqian,
>>
>>>>> &intx_domain_ops, pcie);
>>>>> if (!pcie->intx_domain) {
>>>>> dev_err(dev, "failed to create INTx IRQ domain\n");
>>>>> + of_node_put(intc_node);
>>>>> return -ENODEV;
>>>>> }
>>>> Thanks for doing this.
>>>>
>>>> I checked mtk_pcie_init_irq_domains() and there are multiple exit paths like
>>>> err_msi_domain and err_msi_bottom_domain and the normal path which also
>>>> need of_node_put(intc_node).
>>> Thanks for your reply,
>>>
>>> I didn't add of_node_put() in other paths because I am not sure if the reference passed through irq_domain_add_linear(), since intc_node is passed to irq_domain_add_linear().
>>>
>>> __irq_domain_add() keeps &node->fwnode in the irq_domain structure.
>>>
>>> and use fwnode_handle_get() to get the reference of fwnode, but I still uncertain.
>>>
>>> If the reference don't needed anymore after irq_domain_add_linear(),
>>>
>>> your suggestion looks fine, and I will submit v2.
>>
>> Thanks for your reply, I think we can do similar things like
>> rtl8365mb_irq_setup() in drivers/net/dsa/realtek/rtl8365mb.c
>
>I checked rtl8365mb_irq_setup(), it calls of_node_put() by goto statement for error paths.
>
>and calls of_node_put() before return 0 in normal path. I didn't see the same problem.
Sorry for the confusing. I meant that we can do the same thing -
it calls of_node_put() by goto statement for error paths
and calls of_node_put() before return 0 in normal path. :-)
Thanks,
Miles
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] PCI: mediatek-gen3: Fix refcount leak in mtk_pcie_init_irq_domains
@ 2022-05-30 7:35 ` Miles Chen
0 siblings, 0 replies; 24+ messages in thread
From: Miles Chen @ 2022-05-30 7:35 UTC (permalink / raw)
To: linmq006
Cc: bhelgaas, jianjun.wang, kw, linux-arm-kernel, linux-kernel,
linux-mediatek, linux-pci, lorenzo.pieralisi, matthias.bgg, maz,
miles.chen, robh, ryder.lee
Hi Miaoqian,
>Hi, Miles
>
>On 2022/5/30 10:19, Miles Chen wrote:
>> Hi Miaoqian,
>>
>>>>> &intx_domain_ops, pcie);
>>>>> if (!pcie->intx_domain) {
>>>>> dev_err(dev, "failed to create INTx IRQ domain\n");
>>>>> + of_node_put(intc_node);
>>>>> return -ENODEV;
>>>>> }
>>>> Thanks for doing this.
>>>>
>>>> I checked mtk_pcie_init_irq_domains() and there are multiple exit paths like
>>>> err_msi_domain and err_msi_bottom_domain and the normal path which also
>>>> need of_node_put(intc_node).
>>> Thanks for your reply,
>>>
>>> I didn't add of_node_put() in other paths because I am not sure if the reference passed through irq_domain_add_linear(), since intc_node is passed to irq_domain_add_linear().
>>>
>>> __irq_domain_add() keeps &node->fwnode in the irq_domain structure.
>>>
>>> and use fwnode_handle_get() to get the reference of fwnode, but I still uncertain.
>>>
>>> If the reference don't needed anymore after irq_domain_add_linear(),
>>>
>>> your suggestion looks fine, and I will submit v2.
>>
>> Thanks for your reply, I think we can do similar things like
>> rtl8365mb_irq_setup() in drivers/net/dsa/realtek/rtl8365mb.c
>
>I checked rtl8365mb_irq_setup(), it calls of_node_put() by goto statement for error paths.
>
>and calls of_node_put() before return 0 in normal path. I didn't see the same problem.
Sorry for the confusing. I meant that we can do the same thing -
it calls of_node_put() by goto statement for error paths
and calls of_node_put() before return 0 in normal path. :-)
Thanks,
Miles
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] PCI: mediatek-gen3: Fix refcount leak in mtk_pcie_init_irq_domains
2022-05-30 7:35 ` Miles Chen
(?)
@ 2022-05-31 14:01 ` Miaoqian Lin
-1 siblings, 0 replies; 24+ messages in thread
From: Miaoqian Lin @ 2022-05-31 14:01 UTC (permalink / raw)
To: Miles Chen
Cc: bhelgaas, jianjun.wang, kw, linux-arm-kernel, linux-kernel,
linux-mediatek, linux-pci, lorenzo.pieralisi, matthias.bgg, maz,
robh, ryder.lee
Hi, Miles
On 2022/5/30 15:35, Miles Chen wrote:
> Hi Miaoqian,
>
>> Hi, Miles
>>
>> On 2022/5/30 10:19, Miles Chen wrote:
>>> Hi Miaoqian,
>>>
>>>>>> &intx_domain_ops, pcie);
>>>>>> if (!pcie->intx_domain) {
>>>>>> dev_err(dev, "failed to create INTx IRQ domain\n");
>>>>>> + of_node_put(intc_node);
>>>>>> return -ENODEV;
>>>>>> }
>>>>> Thanks for doing this.
>>>>>
>>>>> I checked mtk_pcie_init_irq_domains() and there are multiple exit paths like
>>>>> err_msi_domain and err_msi_bottom_domain and the normal path which also
>>>>> need of_node_put(intc_node).
>>>> Thanks for your reply,
>>>>
>>>> I didn't add of_node_put() in other paths because I am not sure if the reference passed through irq_domain_add_linear(), since intc_node is passed to irq_domain_add_linear().
>>>>
>>>> __irq_domain_add() keeps &node->fwnode in the irq_domain structure.
>>>>
>>>> and use fwnode_handle_get() to get the reference of fwnode, but I still uncertain.
>>>>
>>>> If the reference don't needed anymore after irq_domain_add_linear(),
>>>>
>>>> your suggestion looks fine, and I will submit v2.
>>> Thanks for your reply, I think we can do similar things like
>>> rtl8365mb_irq_setup() in drivers/net/dsa/realtek/rtl8365mb.c
>> I checked rtl8365mb_irq_setup(), it calls of_node_put() by goto statement for error paths.
>>
>> and calls of_node_put() before return 0 in normal path. I didn't see the same problem.
> Sorry for the confusing. I meant that we can do the same thing -
> it calls of_node_put() by goto statement for error paths
> and calls of_node_put() before return 0 in normal path. :-)
I'll sent a v2 for this: https://lore.kernel.org/all/20220530064807.34534-1-linmq006@gmail.com/
following your original suggestion.
> Thanks,
> Miles
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] PCI: mediatek-gen3: Fix refcount leak in mtk_pcie_init_irq_domains
@ 2022-05-31 14:01 ` Miaoqian Lin
0 siblings, 0 replies; 24+ messages in thread
From: Miaoqian Lin @ 2022-05-31 14:01 UTC (permalink / raw)
To: Miles Chen
Cc: bhelgaas, jianjun.wang, kw, linux-arm-kernel, linux-kernel,
linux-mediatek, linux-pci, lorenzo.pieralisi, matthias.bgg, maz,
robh, ryder.lee
Hi, Miles
On 2022/5/30 15:35, Miles Chen wrote:
> Hi Miaoqian,
>
>> Hi, Miles
>>
>> On 2022/5/30 10:19, Miles Chen wrote:
>>> Hi Miaoqian,
>>>
>>>>>> &intx_domain_ops, pcie);
>>>>>> if (!pcie->intx_domain) {
>>>>>> dev_err(dev, "failed to create INTx IRQ domain\n");
>>>>>> + of_node_put(intc_node);
>>>>>> return -ENODEV;
>>>>>> }
>>>>> Thanks for doing this.
>>>>>
>>>>> I checked mtk_pcie_init_irq_domains() and there are multiple exit paths like
>>>>> err_msi_domain and err_msi_bottom_domain and the normal path which also
>>>>> need of_node_put(intc_node).
>>>> Thanks for your reply,
>>>>
>>>> I didn't add of_node_put() in other paths because I am not sure if the reference passed through irq_domain_add_linear(), since intc_node is passed to irq_domain_add_linear().
>>>>
>>>> __irq_domain_add() keeps &node->fwnode in the irq_domain structure.
>>>>
>>>> and use fwnode_handle_get() to get the reference of fwnode, but I still uncertain.
>>>>
>>>> If the reference don't needed anymore after irq_domain_add_linear(),
>>>>
>>>> your suggestion looks fine, and I will submit v2.
>>> Thanks for your reply, I think we can do similar things like
>>> rtl8365mb_irq_setup() in drivers/net/dsa/realtek/rtl8365mb.c
>> I checked rtl8365mb_irq_setup(), it calls of_node_put() by goto statement for error paths.
>>
>> and calls of_node_put() before return 0 in normal path. I didn't see the same problem.
> Sorry for the confusing. I meant that we can do the same thing -
> it calls of_node_put() by goto statement for error paths
> and calls of_node_put() before return 0 in normal path. :-)
I'll sent a v2 for this: https://lore.kernel.org/all/20220530064807.34534-1-linmq006@gmail.com/
following your original suggestion.
> Thanks,
> Miles
_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] PCI: mediatek-gen3: Fix refcount leak in mtk_pcie_init_irq_domains
@ 2022-05-31 14:01 ` Miaoqian Lin
0 siblings, 0 replies; 24+ messages in thread
From: Miaoqian Lin @ 2022-05-31 14:01 UTC (permalink / raw)
To: Miles Chen
Cc: bhelgaas, jianjun.wang, kw, linux-arm-kernel, linux-kernel,
linux-mediatek, linux-pci, lorenzo.pieralisi, matthias.bgg, maz,
robh, ryder.lee
Hi, Miles
On 2022/5/30 15:35, Miles Chen wrote:
> Hi Miaoqian,
>
>> Hi, Miles
>>
>> On 2022/5/30 10:19, Miles Chen wrote:
>>> Hi Miaoqian,
>>>
>>>>>> &intx_domain_ops, pcie);
>>>>>> if (!pcie->intx_domain) {
>>>>>> dev_err(dev, "failed to create INTx IRQ domain\n");
>>>>>> + of_node_put(intc_node);
>>>>>> return -ENODEV;
>>>>>> }
>>>>> Thanks for doing this.
>>>>>
>>>>> I checked mtk_pcie_init_irq_domains() and there are multiple exit paths like
>>>>> err_msi_domain and err_msi_bottom_domain and the normal path which also
>>>>> need of_node_put(intc_node).
>>>> Thanks for your reply,
>>>>
>>>> I didn't add of_node_put() in other paths because I am not sure if the reference passed through irq_domain_add_linear(), since intc_node is passed to irq_domain_add_linear().
>>>>
>>>> __irq_domain_add() keeps &node->fwnode in the irq_domain structure.
>>>>
>>>> and use fwnode_handle_get() to get the reference of fwnode, but I still uncertain.
>>>>
>>>> If the reference don't needed anymore after irq_domain_add_linear(),
>>>>
>>>> your suggestion looks fine, and I will submit v2.
>>> Thanks for your reply, I think we can do similar things like
>>> rtl8365mb_irq_setup() in drivers/net/dsa/realtek/rtl8365mb.c
>> I checked rtl8365mb_irq_setup(), it calls of_node_put() by goto statement for error paths.
>>
>> and calls of_node_put() before return 0 in normal path. I didn't see the same problem.
> Sorry for the confusing. I meant that we can do the same thing -
> it calls of_node_put() by goto statement for error paths
> and calls of_node_put() before return 0 in normal path. :-)
I'll sent a v2 for this: https://lore.kernel.org/all/20220530064807.34534-1-linmq006@gmail.com/
following your original suggestion.
> Thanks,
> Miles
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2022-05-31 14:03 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-26 11:02 [PATCH] PCI: mediatek-gen3: Fix refcount leak in mtk_pcie_init_irq_domains Miaoqian Lin
2022-05-26 11:02 ` Miaoqian Lin
2022-05-26 11:02 ` Miaoqian Lin
2022-05-26 18:44 ` Rob Herring
2022-05-26 18:44 ` Rob Herring
2022-05-26 18:44 ` Rob Herring
2022-05-27 8:45 ` Miles Chen
2022-05-27 8:45 ` Miles Chen
2022-05-27 8:45 ` Miles Chen
2022-05-28 9:19 ` Miaoqian Lin
2022-05-28 9:19 ` Miaoqian Lin
2022-05-28 9:19 ` Miaoqian Lin
2022-05-30 2:19 ` Miles Chen
2022-05-30 2:19 ` Miles Chen
2022-05-30 2:19 ` Miles Chen
2022-05-30 6:54 ` Miaoqian Lin
2022-05-30 6:54 ` Miaoqian Lin
2022-05-30 6:54 ` Miaoqian Lin
2022-05-30 7:35 ` Miles Chen
2022-05-30 7:35 ` Miles Chen
2022-05-30 7:35 ` Miles Chen
2022-05-31 14:01 ` Miaoqian Lin
2022-05-31 14:01 ` Miaoqian Lin
2022-05-31 14:01 ` Miaoqian Lin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.