Re: [PATCH] IB/mlx5: Fix missing destroy_workqueue on error unwind

[Jason Gunthorpe <jgg@xxxxxxxxxxxx>]

On Thu, Dec 20, 2018 at 04:48:29PM -0700, Mark Bloch wrote:
> 
> 
> On 12/20/18 3:42 PM, Jason Gunthorpe wrote:
> > It got complicated because of the #ifdef.
> > 
> > Fixes: 813e90b1aeaa ("IB/mlx5: Add advise_mr() support")
> > Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxxxx>
> >  drivers/infiniband/hw/mlx5/main.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
> > index 9b40ec73cc646c..cefff6b05d6cc9 100644
> > +++ b/drivers/infiniband/hw/mlx5/main.c
> > @@ -5779,8 +5779,10 @@ int mlx5_ib_stage_init_init(struct mlx5_ib_dev *dev)
> >  	}
> >  
> >  	err = init_srcu_struct(&dev->mr_srcu);
> > -	if (err)
> > +	if (err) {
> > +		destroy_workqueue(dev->advise_mr_wq);
> >  		goto err_free_port;
> > +	}
> 
> Shouldn't this be: goto err_mp ?

Sure looks like it, doesn't it? Also the goto above!

Thanks,
Jason

commit be8bb738488bf137477ea6c7008de39231e72805 (HEAD -> tmp)
Author: Jason Gunthorpe <jgg@xxxxxxxxxxxx>
Date:   Thu Dec 20 16:39:26 2018 -0700

    IB/mlx5: Fix wrong error unwind
    
    The destroy_workqueue on error unwind is missing, and the code jumps to
    the wrong exit label.
    
    Fixes: 813e90b1aeaa ("IB/mlx5: Add advise_mr() support")
    Signed-off-by: Jason Gunthorpe <jgg@xxxxxxxxxxxx>


diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
index 9b40ec73cc646c..75edb080435851 100644
--- a/drivers/infiniband/hw/mlx5/main.c
+++ b/drivers/infiniband/hw/mlx5/main.c
@@ -5775,12 +5775,14 @@ int mlx5_ib_stage_init_init(struct mlx5_ib_dev *dev)
        dev->advise_mr_wq = alloc_ordered_workqueue("mlx5_ib_advise_mr_wq", 0);
        if (!dev->advise_mr_wq) {
                err = -ENOMEM;
-               goto err_free_port;
+               goto err_mp;
        }
 
        err = init_srcu_struct(&dev->mr_srcu);
-       if (err)
-               goto err_free_port;
+       if (err) {
+               destroy_workqueue(dev->advise_mr_wq);
+               goto err_mp;
+       }
 #endif
 
        return 0;