MR cache cleanup
Subsystems (5)
cover MR cache cleanup
From: Leon Romanovsky <[email protected]>
Hi,
In this series, Aharon continues to clean mlx5 MR cache logic.
Thanks
Aharon Landau (5):
RDMA/mlx5: Replace ent->lock with xa_lock
RDMA/mlx5: Replace cache list with Xarray
RDMA/mlx5: Store the number of in_use cache mkeys instead of total_mrs RDMA/mlx5: Store in the cache mkeys instead of mrs RDMA/mlx5: Rename the mkey cache variables and functions
drivers/infiniband/hw/mlx5/main.c | 4 +- drivers/infiniband/hw/mlx5/mlx5_ib.h | 61 ++-- drivers/infiniband/hw/mlx5/mr.c | 505 +++++++++++++++------------ drivers/infiniband/hw/mlx5/odp.c | 2 +- include/linux/mlx5/driver.h | 6 +- 5 files changed, 304 insertions(+), 274 deletions(-)
Signature · 1 line
2.36.1
PATCH 1/5 RDMA/mlx5: Replace ent->lock with xa_lock
From: Aharon Landau <[email protected]>
In the next patch, ent->list will be replaced with an xarray. The xarray uses an internal lock to protect the indexes. Use it to protect all the entry fields, and get rid of ent->lock.
Signed-off-by Aharon Landau <[email protected]>
Signed-off-by Leon Romanovsky <[email protected]>
--- drivers/infiniband/hw/mlx5/mlx5_ib.h | 5 +- drivers/infiniband/hw/mlx5/mr.c | 92 ++++++++++++++-------------- 2 files changed, 47 insertions(+), 50 deletions(-)
drivers/infiniband/hw/mlx5/mlx5_ib.h +1 -4
@@ -737,11 +737,8 @@ struct umr_common { };struct mlx5_cache_ent { +struct xarray mkeys;struct list_head head; -/* sync access to the cahce entry -*/ -spinlock_t lock; -char name[4 ];u32 order;
drivers/infiniband/hw/mlx5/mr.c +46 -46
295 lines not on this page · raw message
Signature · 1 line
2.36.1
PATCH 2/5 RDMA/mlx5: Replace cache list with Xarray
In reply to Leon RomanovskyFrom: Aharon Landau <[email protected]>
The Xarray allows us to store the cached mkeys in memory efficient way.
Entries are reserved in the Xarray using xa_cmpxchg before calling to the upcoming callbacks to avoid allocations in interrupt context. The xa_cmpxchg can sleep when using GFP_KERNEL, so we call it in a loop to ensure one reserved entry for each process trying to reserve.
Signed-off-by Aharon Landau <[email protected]>
Signed-off-by Leon Romanovsky <[email protected]>
--- drivers/infiniband/hw/mlx5/mlx5_ib.h | 14 +- drivers/infiniband/hw/mlx5/mr.c | 204 ++++++++++++++++++--------- 2 files changed, 143 insertions(+), 75 deletions(-)
drivers/infiniband/hw/mlx5/mlx5_ib.h +4 -10
@@ -645,8 +645,6 @@ struct mlx5_ib_mr {struct {u32 out[MLX5_ST_SZ_DW (create_mkey_out)];struct mlx5_async_work cb_work; -/* Cache list element */ -struct list_head list; };/* Used only by kernel MRs (umem == NULL) */
@@ -738,7 +736,8 @@ struct umr_common {struct mlx5_cache_ent {struct xarray mkeys; -struct list_head head; +unsigned long stored; +unsigned long reserved;char name[4 ];u32 order;
@@ -750,18 +749,13 @@ struct mlx5_cache_ent { u8 fill_to_high_water:1;/* -* - available_mrs is the length of list head, ie the number of MRs -* available for immediate allocation. -* - total_mrs is available_mrs plus all in use MRs that could be +* - total_mrs is stored mkeys plus all in use MRs that could be * returned to the cache. -* - limit is the low water mark for available_mrs, 2* limit is the +* - limit is the low water mark for stored mkeys, 2* limit is the * upper water mark. -* - pending is the number of MRs currently being created */ u32 total_mrs; -u32 available_mrs;u32 limit; -u32 pending;/* Statistics */ u32 miss;
drivers/infiniband/hw/mlx5/mr.c +139 -65
363 lines not on this page · raw message
Signature · 1 line
2.36.1
Jason Gunthorpe
maintainerRe: [PATCH rdma-next 2/5] RDMA/mlx5: Replace cache list with Xarray
Quoted 38 lines · Leon Romanovsky
drivers/infiniband/hw/mlx5/mr.c +38
-0+static int push_reserve_mkey (struct mlx5_cache_ent *ent,bool limit_pendings) +{ +unsigned long to_reserve; +void *old; +int err; + +xa_lock_irq (&ent->mkeys); +while (true) { +if (limit_pendings && + (ent->reserved - ent->stored) >MAX_PENDING_REG_MR ) { + err = -EAGAIN ; + goto err; + } + + to_reserve = ent->reserved; + old =__xa_cmpxchg (&ent->mkeys, to_reserve,NULL ,XA_ZERO_ENTRY , +GFP_KERNEL ); + +if (xa_is_err (old)) { + err =xa_err (old); + goto err; + } + +/* +* __xa_cmpxchg() might drop the lock, thus ent->reserved can +* change. +*/ +if (to_reserve != ent->reserved) { +if (to_reserve > ent->reserved) +__xa_erase (&ent->mkeys, to_reserve); +continue ; + } + +/* +* If old != NULL to_reserve cannot be equal to ent->reserved. +*/ +WARN_ON (old); +
drivers/infiniband/hw/mlx5/mr.c static void create_mkey_warn(struct mlx5_ib_dev *dev, int status, void *out) · line 192
Quoted 10 lines
drivers/infiniband/hw/mlx5/mr.c +10
-0+ ent->reserved++; +break ; + } +xa_unlock_irq (&ent->mkeys); +return 0 ; + +err: +xa_unlock_irq (&ent->mkeys); +return err; +}
So, I looked at this for a good long time and I think we can replace it with this version:
static int push_mkey(struct mlx5_cache_ent *ent, bool limit_pendings,
void *to_store)
{
XA_STATE(xas, &ent->mkeys, 0);
void *curr;
xa_lock_irq(&ent->mkeys);
if (limit_pendings &&
(ent->reserved - ent->stored) > MAX_PENDING_REG_MR) {
xa_unlock_irq(&ent->mkeys);
return -EAGAIN;
}
while (1) {
/*
* This is cmpxchg (NULL, XA_ZERO_ENTRY) however this version
* doesn't transparently unlock. Instead we set the xas index to
* the current value of reserved every iteration.
*/
xas_set(&xas, ent->reserved);
curr = xas_load(&xas);
if (!curr) {
if (to_store && ent->stored == ent->reserved)
xas_store(&xas, to_store);
else
xas_store(&xas, XA_ZERO_ENTRY);
if (xas_valid(&xas)) {
ent->reserved++;
if (to_store) {
if (ent->stored != ent->reserved)
__xa_store(&ent->mkeys,
ent->stored,
to_store,
GFP_KERNEL);
ent->stored++;
}
}
}
xa_unlock_irq(&ent->mkeys);
/*
* Notice xas_nomem() must always be called as it cleans
* up any cached allocation.
*/
if (!xas_nomem(&xas, GFP_KERNEL))
break;
xa_lock_irq(&ent->mkeys);
}
if (xas_error(&xas))
return xas_error(&xas);
if (WARN_ON(curr))
return -EINVAL;
return 0;
}Which can do either reserve or a store and is a little more direct as to how it works
Which allows this:
Quoted 8 lines
if (mr->mmkey.cache_ent) {
xa_lock_irq(&mr->mmkey.cache_ent->mkeys);
mr->mmkey.cache_ent->in_use--;
xa_unlock_irq(&mr->mmkey.cache_ent->mkeys);
if (mlx5r_umr_revoke_mr(mr) ||
push_reserve_mkey(mr->mmkey.cache_ent, false))To just call
push_mkey((mr->mmkey.cache_ent, false, mr->mmkey.key)
And with some locking shuffling avoid a lot of lock/unlocking/xarray cycles on the common path of just appending to an xarray with no reservation.
But I didn't notice anything wrong with this series, it does look good.
Thanks,
Jason
A message that is not in the archive (sent off-list or not yet received): [email protected]
Jason Gunthorpe
maintainerRe: [PATCH rdma-next 2/5] RDMA/mlx5: Replace cache list with Xarray
Quoted 1 line · Aharon Landau
Isn't it a problem to store an mkey before
ib_umem_release()?
Shouldn't, the mkey has no connection to the umem other than the umem was used as the source of DMA addresses to put in the mkey.
Previously it was ordered the way it was because thr mr->umem pointer would get overwritten as soon as the mr was put in the cache, but now that the mr is just freed that isn't a problem.
Jason
PATCH 3/5 RDMA/mlx5: Store the number of in_use cache mkeys instead of total_mrs
In reply to Leon RomanovskyFrom: Aharon Landau <[email protected]>
total_mrs is used only to calculate the number of mkeys currently in use. To simplify things, replace it with a new member called "in_use" and directly store the number of mkeys currently in use.
Signed-off-by Aharon Landau <[email protected]>
Signed-off-by Leon Romanovsky <[email protected]>
--- drivers/infiniband/hw/mlx5/mlx5_ib.h | 4 +--- drivers/infiniband/hw/mlx5/mr.c | 30 ++++++++++++++-------------- 2 files changed, 16 insertions(+), 18 deletions(-)
drivers/infiniband/hw/mlx5/mlx5_ib.h +1 -3
@@ -749,12 +749,10 @@ struct mlx5_cache_ent { u8 fill_to_high_water:1;/* -* - total_mrs is stored mkeys plus all in use MRs that could be -* returned to the cache. * - limit is the low water mark for stored mkeys, 2* limit is the * upper water mark. */ -u32 total_mrs; +u32 in_use;u32 limit;/* Statistics */
drivers/infiniband/hw/mlx5/mr.c +15 -15
@@ -259,7 +259,6 @@ static void create_mkey_callback(int status, struct mlx5_async_work *context)xa_lock_irqsave (&ent->mkeys, flags);push_to_reserved (ent, mr); - ent->total_mrs++;/* If we are doing fill_to_high_water then keep going. */ queue_adjust_cache_locked (ent);xa_unlock_irqrestore (&ent->mkeys, flags);
@@ -382,9 +381,6 @@ static struct mlx5_ib_mr *create_cache_mr(struct mlx5_cache_ent *ent)init_waitqueue_head (&mr->mmkey.wait); mr->mmkey.type =MLX5_MKEY_MR ;WRITE_ONCE (ent->dev->cache.last_add, jiffies); -xa_lock_irq (&ent->mkeys); - ent->total_mrs++; -xa_unlock_irq (&ent->mkeys);kfree (in);return mr; free_mr:
@@ -402,7 +398,6 @@ static void remove_cache_mr_locked(struct mlx5_cache_ent *ent)if (!ent->stored)return ; mr =pop_stored_mkey (ent); - ent->total_mrs--;xa_unlock_irq (&ent->mkeys);mlx5_core_destroy_mkey (ent->dev->mdev, mr->mmkey.key);kfree (mr);
@@ -458,11 +453,11 @@ static ssize_t size_write(struct file *filp, const char __user *buf,* mkeys. */ xa_lock_irq (&ent->mkeys); -if (target < ent->total_mrs - ent->stored) { +if (target < ent->in_use) { err = -EINVAL ; goto err_unlock; } - target = target - (ent->total_mrs - ent->stored); + target = target - ent->in_use;if (target < ent->limit || target > ent->limit*2 ) { err = -EINVAL ; goto err_unlock;
@@ -486,7 +481,7 @@ static ssize_t size_read(struct file *filp, char __user *buf, size_t count,char lbuf[20 ];int err; - err =snprintf (lbuf,sizeof (lbuf),"%d\n" , ent->total_mrs); + err =snprintf (lbuf,sizeof (lbuf),"%ld\n" , ent->stored + ent->in_use);if (err <0 )return err;
@@ -680,13 +675,19 @@ struct mlx5_ib_mr *mlx5_mr_cache_alloc(struct mlx5_ib_dev *dev,return ERR_PTR (-EOPNOTSUPP );xa_lock_irq (&ent->mkeys); + ent->in_use++; +if (!ent->stored) {queue_adjust_cache_locked (ent); ent->miss++;xa_unlock_irq (&ent->mkeys); mr =create_cache_mr (ent); -if (IS_ERR (mr)) +if (IS_ERR (mr)) { +xa_lock_irq (&ent->mkeys); + ent->in_use--; +xa_unlock_irq (&ent->mkeys);return mr; + } }else { mr =pop_stored_mkey (ent);queue_adjust_cache_locked (ent);
@@ -718,7 +719,6 @@ static void clean_keys(struct mlx5_ib_dev *dev, int c)xa_lock_irq (&ent->mkeys);while (ent->stored) { mr =pop_stored_mkey (ent); - ent->total_mrs--;xa_unlock_irq (&ent->mkeys);mlx5_core_destroy_mkey (dev->mdev, mr->mmkey.key);kfree (mr);
@@ -1643,13 +1643,13 @@ int mlx5_ib_dereg_mr(struct ib_mr *ibmr, struct ib_udata *udata)/* Stop DMA */ if (mr->cache_ent) { +xa_lock_irq (&mr->cache_ent->mkeys); + mr->cache_ent->in_use--; +xa_unlock_irq (&mr->cache_ent->mkeys); +if (mlx5r_umr_revoke_mr (mr) || -push_reserve_mkey (mr->cache_ent, false)) { -xa_lock_irq (&mr->cache_ent->mkeys); - mr->cache_ent->total_mrs--; -xa_unlock_irq (&mr->cache_ent->mkeys); +push_reserve_mkey (mr->cache_ent, false)) mr->cache_ent =NULL ; - } }if (!mr->cache_ent) { rc =destroy_mkey (to_mdev (mr->ibmr.device), mr);
Signature · 1 line
2.36.1
PATCH 4/5 RDMA/mlx5: Store in the cache mkeys instead of mrs
In reply to Leon RomanovskyFrom: Aharon Landau <[email protected]>
Currently, the driver stores mlx5_ib_mr struct in the cache entries, although the only use of the cached MR is the mkey. Store only the mkey in the cache.
Signed-off-by Aharon Landau <[email protected]>
Signed-off-by Leon Romanovsky <[email protected]>
--- drivers/infiniband/hw/mlx5/mlx5_ib.h | 26 ++-- drivers/infiniband/hw/mlx5/mr.c | 205 ++++++++++++--------------- 2 files changed, 99 insertions(+), 132 deletions(-)
drivers/infiniband/hw/mlx5/mlx5_ib.h +11 -15
@@ -613,6 +613,7 @@ struct mlx5_ib_mkey {unsigned int ndescs;struct wait_queue_head wait;refcount_t usecount; +struct mlx5_cache_ent *cache_ent; };#define MLX5_IB_MTT_PRESENT (MLX5_IB_MTT_READ | MLX5_IB_MTT_WRITE)
@@ -635,18 +636,9 @@ struct mlx5_ib_mr {struct ib_mr ibmr;struct mlx5_ib_mkey mmkey; -/* User MR data */ -struct mlx5_cache_ent *cache_ent; -/* Everything after cache_ent is zero'd when MR allocated */ struct ib_umem *umem; union { -/* Used only while the MR is in the cache */ -struct { -u32 out[MLX5_ST_SZ_DW (create_mkey_out)]; -struct mlx5_async_work cb_work; - }; - /* Used only by kernel MRs (umem == NULL) */ struct { void *descs;
@@ -686,12 +678,6 @@ struct mlx5_ib_mr { }; }; -/* Zero the fields in the mr that are variant depending on usage */ -static inline void mlx5_clear_mr (struct mlx5_ib_mr *mr) -{ -memset_after (mr,0 , cache_ent); -} -static inline bool is_odp_mr (struct mlx5_ib_mr *mr) { return IS_ENABLED(CONFIG_INFINIBAND_ON_DEMAND_PAGING) && mr->umem &&
@@ -762,6 +748,16 @@ struct mlx5_cache_ent {struct delayed_work dwork; }; +struct mlx5r_async_create_mkey { +union { +u32 in[MLX5_ST_SZ_BYTES (create_mkey_in)]; +u32 out[MLX5_ST_SZ_DW (create_mkey_out)]; + }; +struct mlx5_async_work cb_work; +struct mlx5_cache_ent *ent; +u32 mkey; +}; +struct mlx5_mr_cache {struct workqueue_struct *wq;struct mlx5_cache_ent ent[MAX_MR_CACHE_ENTRIES ];
drivers/infiniband/hw/mlx5/mr.c +88 -117
412 lines not on this page · raw message
Signature · 1 line
2.36.1
PATCH 5/5 RDMA/mlx5: Rename the mkey cache variables and functions
In reply to Leon RomanovskyFrom: Aharon Landau <[email protected]>
After replacing the MR cache with an Mkey cache, rename the variables and functions to fit the new meaning.
Signed-off-by Aharon Landau <[email protected]>
Signed-off-by Leon Romanovsky <[email protected]>
--- drivers/infiniband/hw/mlx5/main.c | 4 +- drivers/infiniband/hw/mlx5/mlx5_ib.h | 14 +++---- drivers/infiniband/hw/mlx5/mr.c | 58 ++++++++++++++-------------- drivers/infiniband/hw/mlx5/odp.c | 2 +- include/linux/mlx5/driver.h | 6 +-- 5 files changed, 42 insertions(+), 42 deletions(-)
drivers/infiniband/hw/mlx5/main.c +2 -2
@@ -4002,7 +4002,7 @@ static void mlx5_ib_stage_pre_ib_reg_umr_cleanup(struct mlx5_ib_dev *dev) {int err; - err =mlx5_mr_cache_cleanup (dev); + err =mlx5_mkey_cache_cleanup (dev);if (err)mlx5_ib_warn (dev,"mr cache cleanup failed\n" );
@@ -4022,7 +4022,7 @@ static int mlx5_ib_stage_post_ib_reg_umr_init(struct mlx5_ib_dev *dev)if (ret)return ret; - ret =mlx5_mr_cache_init (dev); + ret =mlx5_mkey_cache_init (dev);if (ret) {mlx5_ib_warn (dev,"mr cache init failed %d\n" , ret);mlx5r_umr_resource_cleanup (dev);
drivers/infiniband/hw/mlx5/mlx5_ib.h +7 -7
@@ -758,9 +758,9 @@ struct mlx5r_async_create_mkey {u32 mkey; }; -struct mlx5_mr_cache { +struct mlx5_mkey_cache {struct workqueue_struct *wq; -struct mlx5_cache_ent ent[MAX_MR_CACHE_ENTRIES ]; +struct mlx5_cache_ent ent[MAX_MKEY_CACHE_ENTRIES ];struct dentry *root;unsigned long last_add; };
@@ -1059,7 +1059,7 @@ struct mlx5_ib_dev {struct mlx5_ib_resources devr;atomic_t mkey_var; -struct mlx5_mr_cache cache; +struct mlx5_mkey_cache cache;struct timer_list delay_timer;/* Prevents soft lock on massive reg MRs */ struct mutex slow_path_mutex;
@@ -1304,8 +1304,8 @@ void mlx5_ib_populate_pas(struct ib_umem *umem, size_t page_size, __be64 *pas, u64 access_flags);void mlx5_ib_copy_pas (u64 *old,u64 *new,int step,int num);int mlx5_ib_get_cqe_size (struct ib_cq *ibcq); -int mlx5_mr_cache_init (struct mlx5_ib_dev *dev); -int mlx5_mr_cache_cleanup (struct mlx5_ib_dev *dev); +int mlx5_mkey_cache_init (struct mlx5_ib_dev *dev); +int mlx5_mkey_cache_cleanup (struct mlx5_ib_dev *dev); struct mlx5_ib_mr *mlx5_mr_cache_alloc(struct mlx5_ib_dev *dev, struct mlx5_cache_ent *ent,
@@ -1333,7 +1333,7 @@ int mlx5r_odp_create_eq(struct mlx5_ib_dev *dev, struct mlx5_ib_pf_eq *eq);void mlx5_ib_odp_cleanup_one (struct mlx5_ib_dev *ibdev);int __init mlx5_ib_odp_init (void );void mlx5_ib_odp_cleanup (void ); -void mlx5_odp_init_mr_cache_entry (struct mlx5_cache_ent *ent); +void mlx5_odp_init_mkey_cache_entry (struct mlx5_cache_ent *ent);void mlx5_odp_populate_xlt (void *xlt,size_t idx,size_t nentries,struct mlx5_ib_mr *mr,int flags);
@@ -1352,7 +1352,7 @@ static inline int mlx5r_odp_create_eq(struct mlx5_ib_dev *dev,static inline void mlx5_ib_odp_cleanup_one (struct mlx5_ib_dev *ibdev) {}static inline int mlx5_ib_odp_init (void ) {return 0 ; }static inline void mlx5_ib_odp_cleanup (void ) {} -static inline void mlx5_odp_init_mr_cache_entry (struct mlx5_cache_ent *ent) {} +static inline void mlx5_odp_init_mkey_cache_entry (struct mlx5_cache_ent *ent) {}static inline void mlx5_odp_populate_xlt (void *xlt,size_t idx,size_t nentries,struct mlx5_ib_mr *mr,int flags) {}
drivers/infiniband/hw/mlx5/mr.c +29 -29
184 lines not on this page · raw message
drivers/infiniband/hw/mlx5/odp.c +1 -1
@@ -1588,7 +1588,7 @@ mlx5_ib_odp_destroy_eq(struct mlx5_ib_dev *dev, struct mlx5_ib_pf_eq *eq)return err; } -void mlx5_odp_init_mr_cache_entry (struct mlx5_cache_ent *ent) +void mlx5_odp_init_mkey_cache_entry (struct mlx5_cache_ent *ent) {if (!(ent->dev->odp_caps.general_caps &IB_ODP_SUPPORT_IMPLICIT ))return ;
include/linux/mlx5/driver.h +3 -3
@@ -727,10 +727,10 @@ enum { };enum { -MR_CACHE_LAST_STD_ENTRY =20 , +MKEY_CACHE_LAST_STD_ENTRY =20 ,MLX5_IMR_MTT_CACHE_ENTRY ,MLX5_IMR_KSM_CACHE_ENTRY , -MAX_MR_CACHE_ENTRIES +MAX_MKEY_CACHE_ENTRIES };struct mlx5_profile {
@@ -739,7 +739,7 @@ struct mlx5_profile {struct {int size;int limit; - } mr_cache[MAX_MR_CACHE_ENTRIES ]; + } mr_cache[MAX_MKEY_CACHE_ENTRIES ]; };struct mlx5_hca_cap {
Signature · 1 line
2.36.1