On Fri, Jan 12, 2024 at 04:55:23PM +0800, Zhipeng Lu wrote:
> When dma_alloc_coherent fails to allocate dd->cr_base[i].va,
> init_credit_return should deallocate dd->cr_base and
> dd->cr_base[i] that allocated before. Or those resources
> would be never freed and a memleak is triggered.
>
> Fixes: 7724105686e7 ("IB/hfi1: add driver files")
> Signed-off-by: Zhipeng Lu <alexious@xxxxxxxxxx>
> ---
> drivers/infiniband/hw/hfi1/pio.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/infiniband/hw/hfi1/pio.c b/drivers/infiniband/hw/hfi1/pio.c
> index 68c621ff59d0..5a91cbda4aee 100644
> --- a/drivers/infiniband/hw/hfi1/pio.c
> +++ b/drivers/infiniband/hw/hfi1/pio.c
> @@ -2086,7 +2086,7 @@ int init_credit_return(struct hfi1_devdata *dd)
> "Unable to allocate credit return DMA range for NUMA %d\n",
> i);
> ret = -ENOMEM;
> - goto done;
> + goto free_cr_base;
> }
> }
> set_dev_node(&dd->pcidev->dev, dd->node);
> @@ -2094,6 +2094,10 @@ int init_credit_return(struct hfi1_devdata *dd)
> ret = 0;
> done:
> return ret;
> +
> +free_cr_base:
> + free_credit_return(dd);
Dennis,
The idea of this patch is right, but it made me wonder, if
free_credit_return() is correct.
init_credit_return() iterates with help of for_each_node_with_cpus():
2062 int init_credit_return(struct hfi1_devdata *dd)
2063 {
...
2075 for_each_node_with_cpus(i) {
2076 int bytes = TXE_NUM_CONTEXTS * sizeof(struct credit_return);
2077
But free_credit_return uses something else:
2099 void free_credit_return(struct hfi1_devdata *dd)
2100 {
...
2105 for (i = 0; i < node_affinity.num_possible_nodes; i++) {
2106 if (dd->cr_base[i].va) {
Thanks
> + goto done;
> }
>
> void free_credit_return(struct hfi1_devdata *dd)
> --
> 2.34.1
>
