> @@ -166,14 +169,14 @@ static void create_mkey_callback(int status, struct mlx5_async_work *context)
>
> WRITE_ONCE(dev->cache.last_add, jiffies);
>
> - spin_lock_irqsave(&ent->lock, flags);
> - list_add_tail(&mr->list, &ent->head);
> - ent->available_mrs++;
> + xa_lock_irqsave(&ent->mkeys, flags);
> + xa_ent = __xa_store(&ent->mkeys, ent->stored++, mr, GFP_ATOMIC);
> + WARN_ON(xa_ent != NULL);
> + ent->pending--;
> ent->total_mrs++;
> /* If we are doing fill_to_high_water then keep going. */
> queue_adjust_cache_locked(ent);
> - ent->pending--;
> - spin_unlock_irqrestore(&ent->lock, flags);
> + xa_unlock_irqrestore(&ent->mkeys, flags);
> }
>
> static struct mlx5_ib_mr *alloc_cache_mr(struct mlx5_cache_ent *ent, void *mkc)
> @@ -196,6 +199,25 @@ static struct mlx5_ib_mr *alloc_cache_mr(struct mlx5_cache_ent *ent, void *mkc)
> return mr;
> }
>
> +static int _push_reserve_mkey(struct mlx5_cache_ent *ent)
> +{
> + unsigned long to_reserve;
> + int rc;
> +
> + while (true) {
> + to_reserve = ent->reserved;
> + rc = xa_err(__xa_cmpxchg(&ent->mkeys, to_reserve, NULL,
> + XA_ZERO_ENTRY, GFP_KERNEL));
> + if (rc)
> + return rc;
What about when old != NULL ?
> + if (to_reserve != ent->reserved)
> + continue;
There is an edge case where where reserved could have shrunk alot
while the lock was released, and xa_cmpxchg could succeed. The above
if will protect things, however a ZERO_ENTRY will have been written to
some weird place in the XA. It needs a
if (old == NULL) // ie we stored something someplace weird
__xa_erase(&ent->mkeys, to_reserve)
> + ent->reserved++;
> + break;
> + }
> + return 0;
> +}
> +
> /* Asynchronously schedule new MRs to be populated in the cache. */
> static int add_keys(struct mlx5_cache_ent *ent, unsigned int num)
> {
> @@ -217,23 +239,32 @@ static int add_keys(struct mlx5_cache_ent *ent, unsigned int num)
> err = -ENOMEM;
> break;
> }
> - spin_lock_irq(&ent->lock);
> + xa_lock_irq(&ent->mkeys);
> if (ent->pending >= MAX_PENDING_REG_MR) {
> + xa_unlock_irq(&ent->mkeys);
> err = -EAGAIN;
> - spin_unlock_irq(&ent->lock);
> + kfree(mr);
> + break;
> + }
> +
> + err = _push_reserve_mkey(ent);
The test of ent->pending is out of date now since this can drop the
lock
It feels like pending and (reserved - stored) are really the same
thing, so maybe just directly limit the number of reserved and test it
after
> @@ -287,39 +318,37 @@ static void remove_cache_mr_locked(struct mlx5_cache_ent *ent)
> {
> struct mlx5_ib_mr *mr;
>
> - lockdep_assert_held(&ent->lock);
> - if (list_empty(&ent->head))
> + if (!ent->stored)
> return;
> - mr = list_first_entry(&ent->head, struct mlx5_ib_mr, list);
> - list_del(&mr->list);
> - ent->available_mrs--;
> + mr = __xa_store(&ent->mkeys, --ent->stored, NULL, GFP_KERNEL);
> + WARN_ON(mr == NULL || xa_is_err(mr));
Add a if (reserved != stored) before the below?
> + WARN_ON(__xa_erase(&ent->mkeys, --ent->reserved) != NULL);
Also please avoid writing WARN_ON(something with side effects)
old = __xa_erase(&ent->mkeys, --ent->reserved);
WARN_ON(old != NULL);
Same for all places
> static int resize_available_mrs(struct mlx5_cache_ent *ent, unsigned int target,
> bool limit_fill)
> + __acquires(&ent->lock) __releases(&ent->lock)
Why?
> {
> int err;
>
> - lockdep_assert_held(&ent->lock);
> -
Why?
> static void clean_keys(struct mlx5_ib_dev *dev, int c)
> {
> struct mlx5_mr_cache *cache = &dev->cache;
> struct mlx5_cache_ent *ent = &cache->ent[c];
> - struct mlx5_ib_mr *tmp_mr;
> struct mlx5_ib_mr *mr;
> - LIST_HEAD(del_list);
> + unsigned long index;
>
> cancel_delayed_work(&ent->dwork);
> - while (1) {
> - spin_lock_irq(&ent->lock);
> - if (list_empty(&ent->head)) {
> - spin_unlock_irq(&ent->lock);
> - break;
> - }
> - mr = list_first_entry(&ent->head, struct mlx5_ib_mr, list);
> - list_move(&mr->list, &del_list);
> - ent->available_mrs--;
> + xa_for_each(&ent->mkeys, index, mr) {
This isn't quite the same thing, the above tolerates concurrent add,
this does not.
It should be more like
while (ent->stored) {
mr = __xa_erase(stored--);
> @@ -1886,6 +1901,17 @@ mlx5_free_priv_descs(struct mlx5_ib_mr *mr)
> }
> }
>
> +static int push_reserve_mkey(struct mlx5_cache_ent *ent)
> +{
> + int ret;
> +
> + xa_lock_irq(&ent->mkeys);
> + ret = _push_reserve_mkey(ent);
> + xa_unlock_irq(&ent->mkeys);
> +
> + return ret;
> +}
Put this close to _push_reserve_mkey() please
Jason
