On 12/8/2021 6:46 PM, Jason Gunthorpe wrote:
@@ -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
The mlx5_ib_dereg_mr is reserving entries as well. Should I limit
create_mkey_cb due to pending deregs?
@@ -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?
I initiated the xarray using XA_FLAGS_ALLOC, therefore, the __xa_store
above will mark the entry as ZERO_ENTRY.
+ 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
