Re: [PATCH v4 rdma-next 2/6] RDMA/mlx5: Remove explicit ODP cache entry

[Jason Gunthorpe <jgg@nvidia.com>]

On Tue, Jan 17, 2023 at 01:24:34AM +0200, Michael Guralnik wrote:
> 
> On 1/16/2023 6:59 PM, Jason Gunthorpe wrote:
> > On Sun, Jan 15, 2023 at 03:34:50PM +0200, Michael Guralnik wrote:
> > > From: Aharon Landau <aharonl@nvidia.com>
> > > 
> > > Explicit ODP mkey doesn't have unique properties. It shares the same
> > > properties as the order 18 cache entry. There is no need to devote a special
> > > entry for that.
> > IMR is "implicit mr" for implicit ODP, the commit message is wrong
> 
> Yes. I'll change to: "IMR MTT mkeys don't have unique properties..."
> 
> > > @@ -1591,20 +1593,8 @@ void mlx5_odp_init_mkey_cache_entry(struct mlx5_cache_ent *ent)
> > >   {
> > >   	if (!(ent->dev->odp_caps.general_caps & IB_ODP_SUPPORT_IMPLICIT))
> > >   		return;
> > > -
> > > -	switch (ent->order - 2) {
> > > -	case MLX5_IMR_MTT_CACHE_ENTRY:
> > > -		ent->ndescs = MLX5_IMR_MTT_ENTRIES;
> > > -		ent->access_mode = MLX5_MKC_ACCESS_MODE_MTT;
> > > -		ent->limit = 0;
> > > -		break;
> > > -
> > > -	case MLX5_IMR_KSM_CACHE_ENTRY:
> > > -		ent->ndescs = mlx5_imr_ksm_entries;
> > > -		ent->access_mode = MLX5_MKC_ACCESS_MODE_KSM;
> > > -		ent->limit = 0;
> > > -		break;
> > > -	}
> > > +	ent->ndescs = mlx5_imr_ksm_entries;
> > > +	ent->access_mode = MLX5_MKC_ACCESS_MODE_KSM;
> > And you didn't answer my question, is this URMable?
> Yes, we can UMR between access modes.
> > Because I don't quite understand how this can work at this point, for
> > lower orders the access_mode is assumed to be MTT, a KLM cannot be put
> > in a low order entry at this point.
> 
> In our current code, the only non-MTT mkeys using the cache are the IMR KSM
> that this patch doesn't change.

It does change it, the isolation between the special IMR and the
normal MTT order is removed right here.

Now it is broken

> > Ideally you'd teach UMR to switch between MTT/KSM and then the cache
> > is fine, size the amount of space required based on the number of
> > bytes in the memory.

> Agreed, access_mode and ndescs can be dropped from the rb_key that this
> series introduces and instead we'll add the size of the descriptors as a
> cache entry property.
> Doing this will reduce number of entries in the RB tree but will add
> complexity to the dereg and rereg flows .

Not really, you just always set the access mode in the UMR like
everything else.

Jason