Skip to content

Commit

Permalink
RDMA/mlx5: Fix MR cache temp entries cleanup
Browse files Browse the repository at this point in the history 
Fix the cleanup of the temp cache entries that are dynamically created
in the MR cache.

The cleanup of the temp cache entries is currently scheduled only when a
new entry is created. Since in the cleanup of the entries only the mkeys
are destroyed and the cache entry stays in the cache, subsequent
registrations might reuse the entry and it will eventually be filled with
new mkeys without cleanup ever getting scheduled again.

On workloads that register and deregister MRs with a wide range of
properties we see the cache ends up holding many cache entries, each
holding the max number of mkeys that were ever used through it.

Additionally, as the cleanup work is scheduled to run over the whole
cache, any mkey that is returned to the cache after the cleanup was
scheduled will be held for less than the intended 30 seconds timeout.

Solve both issues by dropping the existing remove_ent_work and reusing
the existing per-entry work to also handle the temp entries cleanup.

Schedule the work to run with a 30 seconds delay every time we push an
mkey to a clean temp entry.
This ensures the cleanup runs on each entry only 30 seconds after the
first mkey was pushed to an empty entry.

As we have already been distinguishing between persistent and temp entries
when scheduling the cache_work_func, it is not being scheduled in any
other flows for the temp entries.

Another benefit from moving to a per-entry cleanup is we now not
required to hold the rb_tree mutex, thus enabling other flow to run
concurrently.

Fixes: dd1b913 ("RDMA/mlx5: Cache all user cacheable mkeys on dereg MR flow")
Signed-off-by: Michael Guralnik <michaelgur@nvidia.com>
Link: https://patch.msgid.link/e4fa4bb03bebf20dceae320f26816cd2dde23a26.1725362530.git.leon@kernel.org
Signed-off-by: Leon Romanovsky <leon@kernel.org>
  • Loading branch information
Michael Guralnik authored and Leon Romanovsky committed Sep 9, 2024
1 parent ee6d57a commit 7ebb00c
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 52 deletions.
2 changes: 1 addition & 1 deletion drivers/infiniband/hw/mlx5/mlx5_ib.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -802,6 +802,7 @@ struct mlx5_cache_ent {
u8 is_tmp:1;
u8 disabled:1;
u8 fill_to_high_water:1;
u8 tmp_cleanup_scheduled:1;

/*
* - limit is the low water mark for stored mkeys, 2* limit is the
Expand Down Expand Up @@ -833,7 +834,6 @@ struct mlx5_mkey_cache {
struct mutex rb_lock;
struct dentry *fs_root;
unsigned long last_add;
struct delayed_work remove_ent_dwork;
};

struct mlx5_ib_port_resources {
Expand Down
82 changes: 31 additions & 51 deletions drivers/infiniband/hw/mlx5/mr.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,21 @@ static void queue_adjust_cache_locked(struct mlx5_cache_ent *ent)
}
}

static void clean_keys(struct mlx5_ib_dev *dev, struct mlx5_cache_ent *ent)
{
u32 mkey;

spin_lock_irq(&ent->mkeys_queue.lock);
while (ent->mkeys_queue.ci) {
mkey = pop_mkey_locked(ent);
spin_unlock_irq(&ent->mkeys_queue.lock);
mlx5_core_destroy_mkey(dev->mdev, mkey);
spin_lock_irq(&ent->mkeys_queue.lock);
}
ent->tmp_cleanup_scheduled = false;
spin_unlock_irq(&ent->mkeys_queue.lock);
}

static void __cache_work_func(struct mlx5_cache_ent *ent)
{
struct mlx5_ib_dev *dev = ent->dev;
Expand Down Expand Up @@ -602,7 +617,11 @@ static void delayed_cache_work_func(struct work_struct *work)
struct mlx5_cache_ent *ent;

ent = container_of(work, struct mlx5_cache_ent, dwork.work);
__cache_work_func(ent);
/* temp entries are never filled, only cleaned */
if (ent->is_tmp)
clean_keys(ent->dev, ent);
else
__cache_work_func(ent);
}

static int cache_ent_key_cmp(struct mlx5r_cache_rb_key key1,
Expand Down Expand Up @@ -778,20 +797,6 @@ struct mlx5_ib_mr *mlx5_mr_cache_alloc(struct mlx5_ib_dev *dev,
return _mlx5_mr_cache_alloc(dev, ent, access_flags);
}

static void clean_keys(struct mlx5_ib_dev *dev, struct mlx5_cache_ent *ent)
{
u32 mkey;

spin_lock_irq(&ent->mkeys_queue.lock);
while (ent->mkeys_queue.ci) {
mkey = pop_mkey_locked(ent);
spin_unlock_irq(&ent->mkeys_queue.lock);
mlx5_core_destroy_mkey(dev->mdev, mkey);
spin_lock_irq(&ent->mkeys_queue.lock);
}
spin_unlock_irq(&ent->mkeys_queue.lock);
}

static void mlx5_mkey_cache_debugfs_cleanup(struct mlx5_ib_dev *dev)
{
if (!mlx5_debugfs_root || dev->is_rep)
Expand Down Expand Up @@ -904,10 +909,6 @@ mlx5r_cache_create_ent_locked(struct mlx5_ib_dev *dev,
ent->limit = 0;

mlx5_mkey_cache_debugfs_add_ent(dev, ent);
} else {
mod_delayed_work(ent->dev->cache.wq,
&ent->dev->cache.remove_ent_dwork,
msecs_to_jiffies(30 * 1000));
}

return ent;
Expand All @@ -918,35 +919,6 @@ mlx5r_cache_create_ent_locked(struct mlx5_ib_dev *dev,
return ERR_PTR(ret);
}

static void remove_ent_work_func(struct work_struct *work)
{
struct mlx5_mkey_cache *cache;
struct mlx5_cache_ent *ent;
struct rb_node *cur;

cache = container_of(work, struct mlx5_mkey_cache,
remove_ent_dwork.work);
mutex_lock(&cache->rb_lock);
cur = rb_last(&cache->rb_root);
while (cur) {
ent = rb_entry(cur, struct mlx5_cache_ent, node);
cur = rb_prev(cur);
mutex_unlock(&cache->rb_lock);

spin_lock_irq(&ent->mkeys_queue.lock);
if (!ent->is_tmp) {
spin_unlock_irq(&ent->mkeys_queue.lock);
mutex_lock(&cache->rb_lock);
continue;
}
spin_unlock_irq(&ent->mkeys_queue.lock);

clean_keys(ent->dev, ent);
mutex_lock(&cache->rb_lock);
}
mutex_unlock(&cache->rb_lock);
}

int mlx5_mkey_cache_init(struct mlx5_ib_dev *dev)
{
struct mlx5_mkey_cache *cache = &dev->cache;
Expand All @@ -962,7 +934,6 @@ int mlx5_mkey_cache_init(struct mlx5_ib_dev *dev)
mutex_init(&dev->slow_path_mutex);
mutex_init(&dev->cache.rb_lock);
dev->cache.rb_root = RB_ROOT;
INIT_DELAYED_WORK(&dev->cache.remove_ent_dwork, remove_ent_work_func);
cache->wq = alloc_ordered_workqueue("mkey_cache", WQ_MEM_RECLAIM);
if (!cache->wq) {
mlx5_ib_warn(dev, "failed to create work queue\n");
Expand Down Expand Up @@ -1013,7 +984,6 @@ void mlx5_mkey_cache_cleanup(struct mlx5_ib_dev *dev)
return;

mutex_lock(&dev->cache.rb_lock);
cancel_delayed_work(&dev->cache.remove_ent_dwork);
for (node = rb_first(root); node; node = rb_next(node)) {
ent = rb_entry(node, struct mlx5_cache_ent, node);
spin_lock_irq(&ent->mkeys_queue.lock);
Expand Down Expand Up @@ -2054,8 +2024,18 @@ static int mlx5_revoke_mr(struct mlx5_ib_mr *mr)
struct mlx5_ib_dev *dev = to_mdev(mr->ibmr.device);
struct mlx5_cache_ent *ent = mr->mmkey.cache_ent;

if (mr->mmkey.cacheable && !mlx5r_umr_revoke_mr(mr) && !cache_ent_find_and_store(dev, mr))
if (mr->mmkey.cacheable && !mlx5r_umr_revoke_mr(mr) && !cache_ent_find_and_store(dev, mr)) {
ent = mr->mmkey.cache_ent;
/* upon storing to a clean temp entry - schedule its cleanup */
spin_lock_irq(&ent->mkeys_queue.lock);
if (ent->is_tmp && !ent->tmp_cleanup_scheduled) {
mod_delayed_work(ent->dev->cache.wq, &ent->dwork,
msecs_to_jiffies(30 * 1000));
ent->tmp_cleanup_scheduled = true;
}
spin_unlock_irq(&ent->mkeys_queue.lock);
return 0;
}

if (ent) {
spin_lock_irq(&ent->mkeys_queue.lock);
Expand Down

0 comments on commit 7ebb00c

Please sign in to comment.