On Mon, Oct 14, 2019 at 01:20:35PM +0530, Vishal Kulkarni wrote:
> Release resources when attaching to ULD fail. Otherwise, data
> mismatch is seen between LLD and ULD later on, which lead to
> kernel panic when accessing resources that should not even
> exist in the first place.
>
> Fixes: 94cdb8bb993a ("cxgb4: Add support for dynamic allocation of resources
> for ULD")
> Signed-off-by: Shahjada Abul Husain <shahj...@chelsio.com>
> Signed-off-by: Vishal Kulkarni <vis...@chelsio.com>
> ---
> drivers/net/ethernet/chelsio/cxgb4/cxgb4_uld.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_uld.c
> b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_uld.c
> index 5b60224..0482ef8 100644
> --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_uld.c
> +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_uld.c
> @@ -692,10 +692,10 @@ static void uld_init(struct adapter *adap, struct
> cxgb4_lld_info *lld)
> lld->write_cmpl_support = adap->params.write_cmpl_support;
> }
>
> -static void uld_attach(struct adapter *adap, unsigned int uld)
> +static int uld_attach(struct adapter *adap, unsigned int uld)
> {
> - void *handle;
> struct cxgb4_lld_info lli;
> + void *handle;
>
> uld_init(adap, &lli);
> uld_queue_init(adap, uld, &lli);
> @@ -705,7 +705,7 @@ static void uld_attach(struct adapter *adap, unsigned int
> uld)
> dev_warn(adap->pdev_dev,
> "could not attach to the %s driver, error %ld\n",
> adap->uld[uld].name, PTR_ERR(handle));
> - return;
> + return PTR_ERR(handle);
> }
>
> adap->uld[uld].handle = handle;
> @@ -713,6 +713,8 @@ static void uld_attach(struct adapter *adap, unsigned int
> uld)
>
> if (adap->flags & CXGB4_FULL_INIT_DONE)
> adap->uld[uld].state_change(handle, CXGB4_STATE_UP);
> +
> + return 0;
> }
>
> /**
> @@ -727,8 +729,8 @@ static void uld_attach(struct adapter *adap, unsigned int
> uld)
> void cxgb4_register_uld(enum cxgb4_uld type,
> const struct cxgb4_uld_info *p)
Not part of this patch, but the comment above this function describes
it as returning -EBUSY and yet the return type is void. Also, the comment
seems to be in semi-kdoc format, perhaps converting it would be worthwhile.
> {
> - int ret = 0;
> struct adapter *adap;
> + int ret = 0;
>
> if (type >= CXGB4_ULD_MAX)
> return;
> @@ -760,7 +762,9 @@ void cxgb4_register_uld(enum cxgb4_uld type,
> if (ret)
> goto free_irq;
> adap->uld[type] = *p;
> - uld_attach(adap, type);
> + ret = uld_attach(adap, type);
> + if (ret)
> + goto free_irq;
Is it desired that the loop continues and that only the current iteration
is cleaned up?
> continue;
> free_irq:
> if (adap->flags & CXGB4_FULL_INIT_DONE)
> --
> 1.8.3.1
>
