On 31/01/2024 16:18, Jason Gunthorpe wrote:
On Wed, Jan 31, 2024 at 02:50:03PM +0200, Michael Guralnik wrote:
On 29/01/2024 19:52, Jason Gunthorpe wrote:
On Sun, Jan 28, 2024 at 11:29:15AM +0200, Leon Romanovsky wrote:
From: Or Har-Toov <ohartoov@xxxxxxxxxx>
In the dereg flow, UMEM is not a good enough indication whether an MR
is from userspace since in mlx5_ib_rereg_user_mr there are some cases
when a new MR is created and the UMEM of the old MR is set to NULL.
Why is this a problem though? The only thing the umem has to do is to
trigger the UMR optimization. If UMR is not triggered then the mkey is
destroyed and it shouldn't be part of the cache at all.
The problem is that it doesn't trigger the UMR on mkeys that are dereged
from the rereg flow.
Optimally, we'd want them to return to the cache, if possible.
Right, so you suggest changing the umem and umr_can_load into
is_cacheable_mkey() and carefully ensuring the rb_key.ndescs is
zero for non-umrable?
Yes. The code is already written trying to ensure this and we've rephrased
a comment in the previous patch to describe this more accurately.
We can keep relying on the UMEM to decide whether we want to try to return
them to cache, as you suggested in the revoke_mr() below, but that way those
mkeys will not return to the cache and we have to deal with the in_use in
the revoke flow.
I don't know what this in_use means? in_use should be only an issue if
the cache_ent is set? Are we really having in_use be set and cache_ent
bet NULL? That seems like a different bug that should be fixed by
keeping cache_ent and in_use consistent.
in_use should be handled only if mkey has a cache_ent.
I take back what I wrote previously, in_use should be handled in revoke_mr
no matter how we choose to implement this, since we're not guaranteed to
succeed in UMR and might end up dereging mkeys from the cache.
