On 12/8/2022 2:39 AM, Jason Gunthorpe wrote:
On Wed, Dec 07, 2022 at 10:57:50AM +0200, Michael Guralnik wrote:
Switch from using the mkey order to using the new struct as the key to
the RB tree of cache entries.
The key is all the mkey properties that UMR operations can't modify.
Using this key to define the cache entries and to search and create
cache mkeys.
Signed-off-by: Michael Guralnik <michaelgur@xxxxxxxxxx>
---
drivers/infiniband/hw/mlx5/mlx5_ib.h | 32 +++--
drivers/infiniband/hw/mlx5/mr.c | 196 +++++++++++++++++++--------
drivers/infiniband/hw/mlx5/odp.c | 26 ++--
3 files changed, 180 insertions(+), 74 deletions(-)
diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h b/drivers/infiniband/hw/mlx5/mlx5_ib.h
index 10e22fb01e1b..d795e9fc2c2f 100644
--- a/drivers/infiniband/hw/mlx5/mlx5_ib.h
+++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
@@ -731,17 +731,26 @@ struct umr_common {
unsigned int state;
};
+struct mlx5r_cache_rb_key {
+ u8 ats:1;
+ unsigned int access_mode;
+ unsigned int access_flags;
+ /*
+ * keep ndescs as the last member so entries with about the same ndescs
+ * will be close in the tree
+ */
? How does this happen? The compare function doesn't use memcmp..
I think this comment should go in the compare function because the
search function does this:
- return smallest;
+ return (smallest &&
+ smallest->rb_key.access_mode == rb_key.access_mode &&
+ smallest->rb_key.access_flags == rb_key.access_flags &&
+ smallest->rb_key.ats == rb_key.ats) ?
+ smallest :
+ NULL;
So it isn't that they have to be close in the tree, it is that
"smallest" has to find a matching mode/flags/ats before finding the
smallest ndescs of the matching list. Thus ndescs must always by the
last thing in the compare ladder.
Correct, I'll move the comment to the compare function.
We've had a previous version of the compare that used memcmp but we've
changed it so now the comment is not relevant in the struct anymore
I'll fix this and rest of the comments in v3.
Thanks
Michael
+struct mlx5_ib_mr *mlx5_mr_cache_alloc(struct mlx5_ib_dev *dev,
+ int access_flags, int access_mode,
+ int ndescs)
+{
+ struct mlx5r_cache_rb_key rb_key = {
+ .ndescs = ndescs,
+ .access_mode = access_mode,
+ .access_flags = get_unchangeable_access_flags(dev, access_flags)
+ };
+ struct mlx5_cache_ent *ent = mkey_cache_ent_from_rb_key(dev, rb_key);
+ if (!ent)
Missing newline
struct mlx5_cache_ent *mlx5r_cache_create_ent(struct mlx5_ib_dev *dev,
- int order)
+ struct mlx5r_cache_rb_key rb_key,
+ bool debugfs)
{
struct mlx5_cache_ent *ent;
int ret;
@@ -808,7 +873,10 @@ struct mlx5_cache_ent *mlx5r_cache_create_ent(struct mlx5_ib_dev *dev,
return ERR_PTR(-ENOMEM);
xa_init_flags(&ent->mkeys, XA_FLAGS_LOCK_IRQ);
- ent->order = order;
+ ent->rb_key.ats = rb_key.ats;
+ ent->rb_key.access_mode = rb_key.access_mode;
+ ent->rb_key.access_flags = rb_key.access_flags;
+ ent->rb_key.ndescs = rb_key.ndescs;
ent->rb_key = rb_key
int mlx5_mkey_cache_init(struct mlx5_ib_dev *dev)
{
+ struct mlx5r_cache_rb_key rb_key = {
+ .access_mode = MLX5_MKC_ACCESS_MODE_MTT,
+ };
struct mlx5_mkey_cache *cache = &dev->cache;
struct mlx5_cache_ent *ent;
int i;
@@ -838,19 +913,26 @@ int mlx5_mkey_cache_init(struct mlx5_ib_dev *dev)
mlx5_cmd_init_async_ctx(dev->mdev, &dev->async_ctx);
timer_setup(&dev->delay_timer, delay_time_func, 0);
+ mlx5_mkey_cache_debugfs_init(dev);
for (i = 0; i < MAX_MKEY_CACHE_ENTRIES; i++) {
- ent = mlx5r_cache_create_ent(dev, i);
-
- if (i > MKEY_CACHE_LAST_STD_ENTRY) {
- mlx5_odp_init_mkey_cache_entry(ent);
+ if (i > mkey_cache_max_order(dev))
continue;
This is goofy, just make the for loop go from 2 to
mkey_cache_max_order() + 2 (and probably have the function do the + 2
internally)
Get rid of MAX_MKEY_CACHE_ENTRIES
+
+ if (i == MLX5_IMR_KSM_CACHE_ENTRY) {
+ ent = mlx5_odp_init_mkey_cache_entry(dev);
+ if (!ent)
+ continue;
This too, just call mlx5_odp_init_mkey_cache_entry() outside the loop
And rename it to something like mlx5_odp_init_mkey_cache(), and don't
return ent.
Set ent->limit inside mlx5r_cache_create_ent()
And run over the whole rbtree in a final loop to do the final
queue_adjust_cache_locked() step.
-void mlx5_odp_init_mkey_cache_entry(struct mlx5_cache_ent *ent)
+struct mlx5_cache_ent *mlx5_odp_init_mkey_cache_entry(struct mlx5_ib_dev *dev)
{
- if (!(ent->dev->odp_caps.general_caps & IB_ODP_SUPPORT_IMPLICIT))
- return;
- ent->ndescs = mlx5_imr_ksm_entries;
- ent->access_mode = MLX5_MKC_ACCESS_MODE_KSM;
+ struct mlx5r_cache_rb_key rb_key = {
+ .ats = 0,
+ .access_mode = MLX5_MKC_ACCESS_MODE_KSM,
+ .access_flags = 0,
+ .ndescs = mlx5_imr_ksm_entries,
Don't need to zero init things here
Jason
