Re: [PATCH] IB/ucm: ib_ucm_release_dev clears wrong bit if devnum is greater than IB_UCM_MAX_DEVICES

[Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 2652 bytes --]

On 06/12/2015 04:24 PM, Carol Soto wrote:
> 
> 
> On 6/12/2015 3:08 PM, Doug Ledford wrote:
>> On 06/11/2015 12:06 PM, clsoto-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org wrote:
>>> From: Carol L Soto <clsoto-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
>>>
>>> ib_ucm_release_dev clears wrong bit if devnum is greater than
>>> IB_UCM_MAX_DEVICES.
>>>
>>> Signed-off-by: Carol L Soto <clsoto-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
>>> ---
>>>   drivers/infiniband/core/ucm.c | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/infiniband/core/ucm.c
>>> b/drivers/infiniband/core/ucm.c
>>> index f2f6393..e2fd085 100644
>>> --- a/drivers/infiniband/core/ucm.c
>>> +++ b/drivers/infiniband/core/ucm.c
>>> @@ -1193,6 +1193,7 @@ static int ib_ucm_close(struct inode *inode,
>>> struct file *filp)
>>>       return 0;
>>>   }
>>>   +static DECLARE_BITMAP(overflow_map, IB_UCM_MAX_DEVICES);
>>>   static void ib_ucm_release_dev(struct device *dev)
>>>   {
>>>       struct ib_ucm_device *ucm_dev;
>>> @@ -1202,7 +1203,7 @@ static void ib_ucm_release_dev(struct device *dev)
>>>       if (ucm_dev->devnum < IB_UCM_MAX_DEVICES)
>>>           clear_bit(ucm_dev->devnum, dev_map);
>>>       else
>>> -        clear_bit(ucm_dev->devnum - IB_UCM_MAX_DEVICES, dev_map);
>>> +        clear_bit(ucm_dev->devnum - IB_UCM_MAX_DEVICES, overflow_map);
>>>       kfree(ucm_dev);
>>>   }
>>>   @@ -1226,7 +1227,6 @@ static ssize_t show_ibdev(struct device *dev,
>>> struct device_attribute *attr,
>>>   static DEVICE_ATTR(ibdev, S_IRUGO, show_ibdev, NULL);
>>>     static dev_t overflow_maj;
>>> -static DECLARE_BITMAP(overflow_map, IB_UCM_MAX_DEVICES);
>>>   static int find_overflow_devnum(void)
>>>   {
>>>       int ret;
>>>
>> This doesn't look right to me.  In particular, you are creating a bitmap
>> and clearing bits, but I never see that bitmap get any bits set.  So,
>> are you overflowing on the set routine and you just didn't bother to fix
>> that?
> I just moved the declaration of the bitmap before ib_ucm_release_dev to
> be able to compile. This bitmap is used in ib_ucm_add_one
> in the case of devnum > IB_UCM_MAX_DEVICES. The error path in
> ib_ucm_add_one did the clear bit correctly but not the
> ib_ucm_release_dev function. We saw an issue when using SRIOV. The case
> was that user has 2 Mellanox SRIOV cards and just did an
> unbind/bind a VF that was using a ib ucm device with devnum greater than
> 32. We saw this warning

Got it.  For some reason I saw the add of the bitmap but missed the
removal of the original.



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]