[ofa-general] Re: Probable bug in mlx4 driver

[Nicolas Morey-Chaisemartin]

Le 20/04/2009 23:11, Roland Dreier a écrit :
>   >  diff --git a/drivers/net/mlx4/main.c b/drivers/net/mlx4/main.c
>   >  index 102bac9..ae692f1 100644
>   >  --- a/drivers/net/mlx4/main.c
>   >  +++ b/drivers/net/mlx4/main.c
>   >  @@ -977,6 +977,7 @@ static void mlx4_enable_msi_x(struct mlx4_dev *dev)
>   >   				goto retry;
>   >   			}
>   >
>   >  +			kfree(entries);
>   >   			goto no_msi;
>   >   		}
>
> This part of the patch is correct I believe -- entries is leaked
> otherwise if enabling MSI-X fails.
>
>   >  @@ -993,7 +994,7 @@ static void mlx4_enable_msi_x(struct mlx4_dev *dev)
>   >   no_msi:
>   >   	dev->caps.num_comp_vectors = 1;
>   >
>   >  -	for (i = 0; i<  2; ++i)
>   >  +	for (i = 0; i<  nreq; ++i)
>   >   		priv->eq_table.eq[i].irq = dev->pdev->irq;
>   >   }
>
> This is incorrect -- if msi_x is not set, then the function will fall
> through to here and nreq will not even be initialized.  If we are not
> using MSI-X, then only one completion event queue will ever be used, and
> so only the first two EQs need IRQs assigned.
>

Ok I got wrong there from the previous implementation.
There were more much more irq set than this.
for (i = 0; i < MLX4_NUM_EQ; ++i)
where MLX4_NUM_EQ was the same number used when using msi_x.

> Care to resend the first half of the patch with a proper
> subject/changelog/signed-off-by/etc?  (cf Documentation/SubmittingPatches)

Sure. Who/What ML shall I send it to?

Nicolas