Re: [PATCH 10/10] IB: remove the unused usecnt field from struct ib_mr

Sagi Grimberg
2015-12-20T11:42:30Z

>> However, I think that the patch from Shani 6b52a12bc3fc ("IB/uverbs:
>> Implement memory windows support in uverbs") is wrong. A memory
>> window allocation should really reference the MR and not the PD (which
>> is referenced by the MR). Otherwise the MR deregistration is allowed
>> with windows bound to it. If this is the case then this field is not
>> useless and we should fix ib_uverbs_alloc_mw()?
>>
>> Haggai, can you shed some light here? It's Shani's and your code.
> A memory window takes reference on its PD during creation. It can only
> be used by that PD, and it isn't bound to any memory region yet.

Right! it's still not bound. I mixed up the details. Thanks for
reminding me.

> You are right that we want to prevent deregistration of an MR when there
> are memory windows bound to it. This is suggested (although not
> required) by the IBA specifications (section 10.6.7.2.6 Deregistering
> Regions with Bound Windows). The problem with the usecnt field in ib_mr
> as you wrote is that it needs to reflect what happens at bind /
> invalidate time, but bind can happen in user-space without the kernel
> knowing, and invalidation can similarly occur due to a
> send-with-invalidate message being received.

That's true. I didn't see user-space do that either but if it's not
required it's probably a good idea not to complicate things.

> This is why in the ConnectX-3 implementation of memory windows we didn't use the
> ib_mr.usecnt field, but relied on a hardware implementation of the
> reference count.
>
> As I said, the spec also allows a device to proceed with deregistering
> an MR, leaving an orphan memory window. I don't think we can prevent
> this in the kernel, and I'm not sure it would be a good idea anyway, so
> I think it is okay to remove the use_cnt field.

I completely agree. we don't have any way to maintain mw<->mr reference
so let's drop it.

Reviewed-by: Sagi Grimberg <sagig-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>