Add the free_percpu for the allocated "vf->hw.lmt_info" in order to avoid
memory leak, same as the "pf->hw.lmt_info" in
`drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c`.
Fixes: 5c0512072f65 ("octeontx2-pf: cn10k: Use runtime allocated LMTLINE region")
Signed-off-by: Jiasheng Jiang <[email protected]>
---
drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
index 7f8ffbf79cf7..9db2e2d218bb 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
@@ -709,6 +709,8 @@ static int otx2vf_probe(struct pci_dev *pdev, const struct pci_device_id *id)
err_ptp_destroy:
otx2_ptp_destroy(vf);
err_detach_rsrc:
+ if (vf->hw.lmt_info)
+ free_percpu(vf->hw.lmt_info);
if (test_bit(CN10K_LMTST, &vf->hw.cap_flag))
qmem_free(vf->dev, vf->dync_lmt);
otx2_detach_resources(&vf->mbox);
@@ -762,6 +764,8 @@ static void otx2vf_remove(struct pci_dev *pdev)
otx2_shutdown_tc(vf);
otx2vf_disable_mbox_intr(vf);
otx2_detach_resources(&vf->mbox);
+ if (vf->hw.lmt_info)
+ free_percpu(vf->hw.lmt_info);
if (test_bit(CN10K_LMTST, &vf->hw.cap_flag))
qmem_free(vf->dev, vf->dync_lmt);
otx2vf_vfaf_mbox_destroy(vf);
--
2.25.1
On Thu, Mar 16, 2023 at 10:39:11AM +0800, Jiasheng Jiang wrote:
> Add the free_percpu for the allocated "vf->hw.lmt_info" in order to avoid
> memory leak, same as the "pf->hw.lmt_info" in
> `drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c`.
>
> Fixes: 5c0512072f65 ("octeontx2-pf: cn10k: Use runtime allocated LMTLINE region")
> Signed-off-by: Jiasheng Jiang <[email protected]>
> ---
> drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
> index 7f8ffbf79cf7..9db2e2d218bb 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
> @@ -709,6 +709,8 @@ static int otx2vf_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> err_ptp_destroy:
> otx2_ptp_destroy(vf);
> err_detach_rsrc:
> + if (vf->hw.lmt_info)
> + free_percpu(vf->hw.lmt_info);
> if (test_bit(CN10K_LMTST, &vf->hw.cap_flag))
> qmem_free(vf->dev, vf->dync_lmt);
I wonder if it wouldn't be more error prune when You create a function
to unroll cn10k_lmtst_init() like cn10k_lmtst_deinit(). These two if can
be there, maybe also sth else is missing.
Otherwise it is fine
Reviewed-by: Michal Swiatkowski <[email protected]>
> otx2_detach_resources(&vf->mbox);
> @@ -762,6 +764,8 @@ static void otx2vf_remove(struct pci_dev *pdev)
> otx2_shutdown_tc(vf);
> otx2vf_disable_mbox_intr(vf);
> otx2_detach_resources(&vf->mbox);
> + if (vf->hw.lmt_info)
> + free_percpu(vf->hw.lmt_info);
> if (test_bit(CN10K_LMTST, &vf->hw.cap_flag))
> qmem_free(vf->dev, vf->dync_lmt);
> otx2vf_vfaf_mbox_destroy(vf);
> --
> 2.25.1
>
>-----Original Message-----
>From: Michal Swiatkowski <[email protected]>
>Sent: Thursday, March 16, 2023 1:40 PM
>To: Jiasheng Jiang <[email protected]>
>Cc: Sunil Kovvuri Goutham <[email protected]>; Geethasowjanya Akula <[email protected]>; Subbaraya Sundeep Bhatta <[email protected]>; Hariprasad Kelam <[email protected]>; >[email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
>Subject: [EXT] Re: [PATCH] octeontx2-vf: Add missing free for alloc_percpu
>External Email
>----------------------------------------------------------------------
>On Thu, Mar 16, 2023 at 10:39:11AM +0800, Jiasheng Jiang wrote:
>> Add the free_percpu for the allocated "vf->hw.lmt_info" in order to
>> avoid memory leak, same as the "pf->hw.lmt_info" in
>> `drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c`.
>>
>> Fixes: 5c0512072f65 ("octeontx2-pf: cn10k: Use runtime allocated
>> LMTLINE region")
>> Signed-off-by: Jiasheng Jiang <[email protected]>
>> ---
>> drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
>> b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
>> index 7f8ffbf79cf7..9db2e2d218bb 100644
>> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
>> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
>> @@ -709,6 +709,8 @@ static int otx2vf_probe(struct pci_dev *pdev,
>> const struct pci_device_id *id)
>> err_ptp_destroy:
>> otx2_ptp_destroy(vf);
>> err_detach_rsrc:
>> + if (vf->hw.lmt_info)
>> + free_percpu(vf->hw.lmt_info);
>> if (test_bit(CN10K_LMTST, &vf->hw.cap_flag))
>> qmem_free(vf->dev, vf->dync_lmt);
>I wonder if it wouldn't be more error prune when You create a function to unroll cn10k_lmtst_init() like cn10k_lmtst_deinit(). These two if can be there, maybe also sth else is missing.
>Otherwise it is fine
>Reviewed-by: Michal Swiatkowski <[email protected]>
>> otx2_detach_resources(&vf->mbox);
>> @@ -762,6 +764,8 @@ static void otx2vf_remove(struct pci_dev *pdev)
>> otx2_shutdown_tc(vf);
>> otx2vf_disable_mbox_intr(vf);
>> otx2_detach_resources(&vf->mbox);
>> + if (vf->hw.lmt_info)
>> + free_percpu(vf->hw.lmt_info);
Ack. Thanks for the patch.
>> if (test_bit(CN10K_LMTST, &vf->hw.cap_flag))
>> qmem_free(vf->dev, vf->dync_lmt);
>> otx2vf_vfaf_mbox_destroy(vf);
>> --
>> 2.25.1
>>
Geetha.
On Thu, Mar 16, 2023 at 10:39:11AM +0800, Jiasheng Jiang wrote:
> On Thu, Mar 16, 2023 at 10:39:11AM +0800, Jiasheng Jiang wrote:
>> Add the free_percpu for the allocated "vf->hw.lmt_info" in order to avoid
>> memory leak, same as the "pf->hw.lmt_info" in
>> `drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c`.
>>
>> Fixes: 5c0512072f65 ("octeontx2-pf: cn10k: Use runtime allocated LMTLINE region")
>> Signed-off-by: Jiasheng Jiang <[email protected]>
>> ---
>> drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
>> index 7f8ffbf79cf7..9db2e2d218bb 100644
>> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
>> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
>> @@ -709,6 +709,8 @@ static int otx2vf_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>> err_ptp_destroy:
>> otx2_ptp_destroy(vf);
>> err_detach_rsrc:
>> + if (vf->hw.lmt_info)
>> + free_percpu(vf->hw.lmt_info);
>> if (test_bit(CN10K_LMTST, &vf->hw.cap_flag))
>> qmem_free(vf->dev, vf->dync_lmt);
> I wonder if it wouldn't be more error prune when You create a function
> to unroll cn10k_lmtst_init() like cn10k_lmtst_deinit(). These two if can
> be there, maybe also sth else is missing.
>
> Otherwise it is fine
> Reviewed-by: Michal Swiatkowski <[email protected]>
I try my best and find nothing else missing.
Thanks,
Jiang
On Thu, 16 Mar 2023 10:39:11 +0800 Jiasheng Jiang wrote:
> + if (vf->hw.lmt_info)
> + free_percpu(vf->hw.lmt_info);
> if (test_bit(CN10K_LMTST, &vf->hw.cap_flag))
> qmem_free(vf->dev, vf->dync_lmt);
> otx2_detach_resources(&vf->mbox);
> @@ -762,6 +764,8 @@ static void otx2vf_remove(struct pci_dev *pdev)
> otx2_shutdown_tc(vf);
> otx2vf_disable_mbox_intr(vf);
> otx2_detach_resources(&vf->mbox);
> + if (vf->hw.lmt_info)
> + free_percpu(vf->hw.lmt_info);
No need for the if () checks, free_percpu() seems to handle NULL just
fine.
On Fri, Mar 17, 2023 at 12:24:46AM +0800, Jakub Kicinski wrote:
> On Thu, 16 Mar 2023 10:39:11 +0800 Jiasheng Jiang wrote:
>> + if (vf->hw.lmt_info)
>> + free_percpu(vf->hw.lmt_info);
>> if (test_bit(CN10K_LMTST, &vf->hw.cap_flag))
>> qmem_free(vf->dev, vf->dync_lmt);
>> otx2_detach_resources(&vf->mbox);
>> @@ -762,6 +764,8 @@ static void otx2vf_remove(struct pci_dev *pdev)
>> otx2_shutdown_tc(vf);
>> otx2vf_disable_mbox_intr(vf);
>> otx2_detach_resources(&vf->mbox);
>> + if (vf->hw.lmt_info)
>> + free_percpu(vf->hw.lmt_info);
>
> No need for the if () checks, free_percpu() seems to handle NULL just
> fine.
I will submit a v2 removing the checks.
Thanks,
Jiang