On Tue, Jan 17, 2023 at 02:08:35AM +0200, Michael Guralnik wrote:
>
> On 1/17/2023 1:45 AM, Jason Gunthorpe wrote:
> > 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
>
> How do IMR MTT mkeys sharing a cache entry with other MTT mkeys break
> anything?
Oh, I read it wrong, this is still keeping the high order
MLX5_IMR_KSM_CACHE_ENTRY
> > > > 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
>
> ok, I'll give this a second look. if it's really only this, I can probably
> push this quickly.
> BTW, this will mean that IMR KSM mkeys will also share an entry with other
> MTT mkeys
That would be perfect, you should definately do it
But it seems there is not an issue here, so a followup is OK
Jason
