[ewg] [PATCH] ipoib: disable napi while cq is being drained

Yossi Etigin yosefe at voltaire.com
Wed Apr 22 09:27:00 PDT 2009


Jack Morgenstein wrote:
> On Friday 17 April 2009 18:26, Yossi Etigin wrote:
>> -       ipoib_dbg(priv, "bringing up interface\n");
>> - 
>> --      if (!test_and_set_bit(IPOIB_FLAG_ADMIN_UP, &priv->flags))
>> --              napi_enable(&priv->napi);
>> -+      set_bit(IPOIB_FLAG_ADMIN_UP, &priv->flags);
>> - 
>> -       if (ipoib_pkey_dev_delay_open(dev))
>> -               return 0;
>> - 
>> --      if (ipoib_ib_dev_open(dev)) {
>> --              napi_disable(&priv->napi);
>> --              return -EINVAL;
>> --      }
>> -+      if (ipoib_ib_dev_open(dev))
>> -+              return -EINVAL;
>>
> 
> I think there is a bug here in the error flow. 
> You do
>    set_bit(IPOIB_FLAG_ADMIN_UP, &priv->flags);
> 
> However, if there is an error return, you do not do
>    clear_bit(IPOIB_FLAG_ADMIN_UP, &priv->flags);
> 
> Note that in the patch you prepared for Roland (in the general list),
> the clear_bit is done properly.
> (you probably need to arrange for an "err_out:" goto label which will do
> the clear_bit and return -EINVAL).
> 
> - Jack
> 
> P.S. we need the fix for ofed 1.4.1 ASAP.


You are right - there probably is a bug here, the bit should be cleared.
However, seems like it was there before this patch too.
(the only relevant change in the patch is test_and_set_bit -> set_bit)


--Yossi




More information about the ewg mailing list