| Message ID | 20181220234251.GA26180@ziepe.ca (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Jason Gunthorpe |
| Headers | show |
| Series | IB/mlx5: Fix missing destroy_workqueue on error unwind | expand |
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@mellanox.com> > --- > 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 > --- a/drivers/infiniband/hw/mlx5/main.c > +++ 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 ? Mark > #endif > > return 0; >
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@mellanox.com> > > 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@mellanox.com> 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@mellanox.com> 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;
On Fri, Dec 21, 2018 at 03:35:46AM +0000, Jason Gunthorpe wrote: > 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@mellanox.com> > > > 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@mellanox.com> > 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@mellanox.com> > > Thanks, Acked-by: Leon Romanovsky <leonro@mellanox.com>
diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c index 9b40ec73cc646c..cefff6b05d6cc9 100644 --- a/drivers/infiniband/hw/mlx5/main.c +++ 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; + } #endif return 0;
It got complicated because of the #ifdef. Fixes: 813e90b1aeaa ("IB/mlx5: Add advise_mr() support") Signed-off-by: Jason Gunthorpe <jgg@mellanox.com> --- drivers/infiniband/hw/mlx5/main.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)