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@xxxxxxxxxx>
> > >
> > > 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
