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@xxxxxxxxxxxx> -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
