Collection of fixes for 5.3
Subsystems (2)
cover Collection of fixes for 5.3
From: Leon Romanovsky <[email protected]>
Hi,
This is small patch set of fixes targeted for 5.3 and stable@.
Thanks
Moni Shoua (1):
IB/mlx5: Prevent concurrent MR updates during invalidation
Parav Pandit (4):
RDMA/core: Annotate destroy of mutex to ensure that it is released as
unlocked
IB/mlx5: Avoid unnecessary typecast
IB/core: Fix querying total rdma stats
IB/counters: Initialize port counter and annotate mutex_destroy
Yishai Hadas (5):
IB/mlx5: Fix unreg_umr to ignore the mkey state IB/mlx5: Use direct mkey destroy command upon UMR unreg failure IB/mlx5: Fix unreg_umr to set a device PD
IB/mlx5: Fix clean_mr() to work in the expected order IB/mlx5: Fix RSS Toeplitz function to be specification aligned
drivers/infiniband/core/cache.c | 1 + drivers/infiniband/core/cma_configfs.c | 8 +++++++- drivers/infiniband/core/counters.c | 19 +++++++++++-------- drivers/infiniband/core/device.c | 3 +++ drivers/infiniband/core/user_mad.c | 2 +- drivers/infiniband/core/uverbs_main.c | 4 ++++ drivers/infiniband/core/verbs.c | 1 + drivers/infiniband/hw/mlx5/main.c | 2 +- drivers/infiniband/hw/mlx5/mlx5_ib.h | 1 + drivers/infiniband/hw/mlx5/mr.c | 23 ++++++++++++++--------- drivers/infiniband/hw/mlx5/odp.c | 3 ++- drivers/infiniband/hw/mlx5/qp.c | 13 ++++++++----- 12 files changed, 54 insertions(+), 26 deletions(-)
--
2.20.1
PATCH 1/10 RDMA/core: Annotate destroy of mutex to ensure that it is released as unlocked
From: Parav Pandit <[email protected]>
While compiled with CONFIG_DEBUG_MUTEXES, the kernel ensures that mutex is not held during destroy.
Hence add mutex_destroy() for mutexes used in RDMA modules.
--- drivers/infiniband/core/cache.c | 1 + drivers/infiniband/core/cma_configfs.c | 8 +++++++- drivers/infiniband/core/device.c | 3 +++ drivers/infiniband/core/user_mad.c | 2 +- drivers/infiniband/core/uverbs_main.c | 4 ++++ drivers/infiniband/core/verbs.c | 1 + 6 files changed, 17 insertions(+), 2 deletions(-)
drivers/infiniband/core/cache.c +1 -0
@@ -810,6 +810,7 @@ static void release_gid_table(struct ib_device *device,if (leak)return ; +mutex_destroy (&table->lock);kfree (table->data_vec);kfree (table); }
drivers/infiniband/core/cma_configfs.c +7 -1
@@ -342,12 +342,18 @@ static struct configfs_subsystem cma_subsys = {int __init cma_configfs_init (void ) { +int ret; +config_group_init (&cma_subsys.su_group);mutex_init (&cma_subsys.su_mutex); -return configfs_register_subsystem (&cma_subsys); + ret =configfs_register_subsystem (&cma_subsys); +if (ret) +mutex_destroy (&cma_subsys.su_mutex); +return ret; } void __exit cma_configfs_exit(void) {configfs_unregister_subsystem (&cma_subsys); +mutex_destroy (&cma_subsys.su_mutex); }
drivers/infiniband/core/device.c +3 -0
@@ -497,6 +497,9 @@ static void ib_device_release(struct device *device) rcu_head); } +mutex_destroy (&dev->unregistration_lock); +mutex_destroy (&dev->compat_devs_mutex); +xa_destroy (&dev->compat_devs);xa_destroy (&dev->client_data);kfree_rcu (dev, rcu_head);
drivers/infiniband/core/user_mad.c +1 -1
@@ -1038,7 +1038,7 @@ static int ib_umad_close(struct inode *inode, struct file *filp)ib_unregister_mad_agent (file->agent[i]);mutex_unlock (&file->port->file_mutex); - +mutex_destroy (&file->mutex);kfree (file);return 0 ; }
drivers/infiniband/core/uverbs_main.c +4 -0
@@ -120,6 +120,8 @@ static void ib_uverbs_release_dev(struct device *device)uverbs_destroy_api (dev->uapi);cleanup_srcu_struct (&dev->disassociate_srcu); +mutex_destroy (&dev->lists_mutex); +mutex_destroy (&dev->xrcd_tree_mutex);kfree (dev); }
@@ -212,6 +214,8 @@ void ib_uverbs_release_file(struct kref *ref)if (file->disassociate_page)__free_pages (file->disassociate_page,0 ); +mutex_destroy (&file->umap_lock); +mutex_destroy (&file->ucontext_lock);kfree (file); }
drivers/infiniband/core/verbs.c +1 -0
@@ -2259,6 +2259,7 @@ int ib_dealloc_xrcd(struct ib_xrcd *xrcd, struct ib_udata *udata)if (ret)return ret; } +mutex_destroy (&xrcd->tgt_qp_mutex);return xrcd->device->ops.dealloc_xrcd (xrcd, udata); }
--
2.20.1
PATCH 2/10 IB/mlx5: Fix unreg_umr to ignore the mkey state
In reply to Leon RomanovskyFrom: Yishai Hadas <[email protected]>
Fix unreg_umr to ignore the mkey state and do not fail if was freed. This prevents a case that a user space application already changed the mkey state to free and then the UMR operation will fail leaving the mkey in an un-applicable state.
--- drivers/infiniband/hw/mlx5/mlx5_ib.h | 1 + drivers/infiniband/hw/mlx5/mr.c | 4 ++-- drivers/infiniband/hw/mlx5/qp.c | 12 ++++++++---- 3 files changed, 11 insertions(+), 6 deletions(-)
drivers/infiniband/hw/mlx5/mlx5_ib.h +1 -0
@@ -481,6 +481,7 @@ struct mlx5_umr_wr {u64 length;int access_flags;u32 mkey; + u8 ignore_free_state:1; };static inline const struct mlx5_umr_wr *umr_wr (const struct ib_send_wr *wr)
drivers/infiniband/hw/mlx5/mr.c +2 -2
@@ -1372,10 +1372,10 @@ static int unreg_umr(struct mlx5_ib_dev *dev, struct mlx5_ib_mr *mr)if (mdev->state ==MLX5_DEVICE_STATE_INTERNAL_ERROR )return 0 ; - umrwr.wr.send_flags =MLX5_IB_SEND_UMR_DISABLE_MR | -MLX5_IB_SEND_UMR_FAIL_IF_FREE ; + umrwr.wr.send_flags =MLX5_IB_SEND_UMR_DISABLE_MR ; umrwr.wr.opcode =MLX5_IB_WR_UMR ; umrwr.mkey = mr->mmkey.key; + umrwr.ignore_free_state =1 ;return mlx5_ib_post_send_wait (dev, &umrwr); }
drivers/infiniband/hw/mlx5/qp.c +8 -4
@@ -4295,10 +4295,14 @@ static int set_reg_umr_segment(struct mlx5_ib_dev *dev,memset (umr,0 ,sizeof (*umr)); -if (wr->send_flags &MLX5_IB_SEND_UMR_FAIL_IF_FREE ) - umr->flags =MLX5_UMR_CHECK_FREE ;/* fail if free */ -else - umr->flags =MLX5_UMR_CHECK_NOT_FREE ;/* fail if not free */ +if (!umrwr->ignore_free_state) { +if (wr->send_flags &MLX5_IB_SEND_UMR_FAIL_IF_FREE ) +/* fail if free */ + umr->flags =MLX5_UMR_CHECK_FREE ; +else +/* fail if not free */ + umr->flags =MLX5_UMR_CHECK_NOT_FREE ; + } umr->xlt_octowords =cpu_to_be16 (get_xlt_octo (umrwr->xlt_size));if (wr->send_flags &MLX5_IB_SEND_UMR_UPDATE_XLT ) {
--
2.20.1
PATCH 3/10 IB/mlx5: Use direct mkey destroy command upon UMR unreg failure
In reply to Leon RomanovskyFrom: Yishai Hadas <[email protected]>
Use direct firmware command to destroy the mkey in case the unreg UMR operation has failed.
This prevents a case that an mkey will leak out from the cache post a failure to be destroyed by a UMR WR.
In case the MR cache limit didn't reach a call to add another entry to the cache instead of the destroyed one is issued.
In addition, replaced a warn message to WARN_ON() as this flow is fatal and can't happen unless some bug around.
--- drivers/infiniband/hw/mlx5/mr.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)
drivers/infiniband/hw/mlx5/mr.c +8 -5
@@ -545,13 +545,16 @@ void mlx5_mr_cache_free(struct mlx5_ib_dev *dev, struct mlx5_ib_mr *mr)return ; c =order2idx (dev, mr->order); -if (c <0 || c >=MAX_MR_CACHE_ENTRIES ) { -mlx5_ib_warn (dev,"order %d, cache index %d\n" , mr->order, c); -return ; - } +WARN_ON (c <0 || c >=MAX_MR_CACHE_ENTRIES ); -if (unreg_umr (dev, mr)) +if (unreg_umr (dev, mr)) { + mr->allocated_from_cache = false; +destroy_mkey (dev, mr); + ent = &cache->ent[c]; +if (ent->cur < ent->limit) +queue_work (cache->wq, &ent->work);return ; + } ent = &cache->ent[c];spin_lock_irq (&ent->lock);
--
2.20.1
PATCH 4/10 IB/mlx5: Fix unreg_umr to set a device PD
In reply to Leon RomanovskyFrom: Yishai Hadas <[email protected]>
Fix unreg_umr to set a device PD (i.e. UMR PD) which can't be given outside to the kernel.
As the MR addresses are still in place we must ensure that this MR can't be used by a user space that will get the original PD number and guess the MR lkey.
--- drivers/infiniband/hw/mlx5/mr.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/infiniband/hw/mlx5/mr.c +3 -1
@@ -1375,8 +1375,10 @@ static int unreg_umr(struct mlx5_ib_dev *dev, struct mlx5_ib_mr *mr)if (mdev->state ==MLX5_DEVICE_STATE_INTERNAL_ERROR )return 0 ; - umrwr.wr.send_flags =MLX5_IB_SEND_UMR_DISABLE_MR ; + umrwr.wr.send_flags =MLX5_IB_SEND_UMR_DISABLE_MR | +MLX5_IB_SEND_UMR_UPDATE_PD_ACCESS ; umrwr.wr.opcode =MLX5_IB_WR_UMR ; + umrwr.pd = dev->umrc.pd; umrwr.mkey = mr->mmkey.key; umrwr.ignore_free_state =1 ;
--
2.20.1
PATCH 5/10 IB/mlx5: Fix clean_mr() to work in the expected order
In reply to Leon RomanovskyFrom: Yishai Hadas <[email protected]>
Any dma map underlying the MR should only be freed once the MR is fenced at the hardware.
As of the above we first destroy the MKEY and just after that can safely call to dma_unmap_single().
--- drivers/infiniband/hw/mlx5/mr.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/infiniband/hw/mlx5/mr.c +3 -3
@@ -1582,10 +1582,10 @@ static void clean_mr(struct mlx5_ib_dev *dev, struct mlx5_ib_mr *mr) mr->sig =NULL ; } -mlx5_free_priv_descs (mr); - -if (!allocated_from_cache) +if (!allocated_from_cache) {destroy_mkey (dev, mr); +mlx5_free_priv_descs (mr); + } }static void dereg_mr (struct mlx5_ib_dev *dev,struct mlx5_ib_mr *mr)
--
2.20.1
PATCH 6/10 IB/mlx5: Fix RSS Toeplitz function to be specification aligned
In reply to Leon RomanovskyFrom: Yishai Hadas <[email protected]>
The specification for the Toeplitz function doesn't mention to set the key explicitly to be symmetric. In case a symmetric functionality is required a symmetric key can be simply used.
Fix the code accordingly.
--- drivers/infiniband/hw/mlx5/qp.c | 1 - 1 file changed, 1 deletion(-)
drivers/infiniband/hw/mlx5/qp.c +0 -1
@@ -1713,7 +1713,6 @@ static int create_rss_raw_qp_tir(struct mlx5_ib_dev *dev, struct mlx5_ib_qp *qp, }MLX5_SET (tirc, tirc, rx_hash_fn,MLX5_RX_HASH_FN_TOEPLITZ ); -MLX5_SET (tirc, tirc, rx_hash_symmetric,1 );memcpy (rss_key, ucmd.rx_hash_key, len);break ; }
--
2.20.1
PATCH 7/10 IB/mlx5: Prevent concurrent MR updates during invalidation
In reply to Leon RomanovskyFrom: Moni Shoua <[email protected]>
Device requires that memory registration work requests that update the address translation table of a MR will be fenced if posted together. This scenario can happen when address ranges are invalidated by the mmu in separate concurrent calls to the invalidation callback.
We prefer to block concurrent address updates for a single MR over fencing since making the decision if a WQE needs fencing will be more expensive and fencing all WQEs is a too radical choice.
--- drivers/infiniband/hw/mlx5/odp.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/infiniband/hw/mlx5/odp.c +2 -1
@@ -246,7 +246,7 @@ void mlx5_ib_invalidate_range(struct ib_umem_odp *umem_odp, unsigned long start,* overwrite the same MTTs. Concurent invalidations might race us, * but they will write 0s as well, so no difference in the end result. */ - +mutex_lock (&umem_odp->umem_mutex);for (addr = start; addr < end; addr +=BIT (umem_odp->page_shift)) { idx = (addr -ib_umem_start (umem_odp)) >> umem_odp->page_shift;/*
@@ -278,6 +278,7 @@ void mlx5_ib_invalidate_range(struct ib_umem_odp *umem_odp, unsigned long start, idx - blk_start_idx +1 ,0 ,MLX5_IB_UPD_XLT_ZAP | MLX5_IB_UPD_XLT_ATOMIC); +mutex_unlock (&umem_odp->umem_mutex);/* * We are now sure that the device will not access the * memory. We can safely unmap it, and mark it as dirty if
--
2.20.1
PATCH 8/10 IB/mlx5: Avoid unnecessary typecast
In reply to Leon RomanovskyFrom: Parav Pandit <[email protected]>
IB device pointer is already available while deallocating IB device, Hence do not typecast it.
--- drivers/infiniband/hw/mlx5/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/infiniband/hw/mlx5/main.c +1 -1
@@ -6933,7 +6933,7 @@ static void *mlx5_ib_add(struct mlx5_core_dev *mdev) dev->port =kcalloc (num_ports,sizeof (*dev->port),GFP_KERNEL );if (!dev->port) { -ib_dealloc_device ((struct ib_device *)dev); +ib_dealloc_device (&dev->ib_dev);return NULL ; }
--
2.20.1
PATCH 9/10 IB/core: Fix querying total rdma stats
In reply to Leon RomanovskyFrom: Parav Pandit <[email protected]>
rdma_counter_init() may fail for a device. In such case while calculating total sum, ignore NULL hstats.
This fixes below observed call trace.
BUG: kernel NULL pointer dereference, address: 00000000000000a0 PGD 8000001009b30067 P4D 8000001009b30067 PUD 10549c9067 PMD 0 Oops: 0000 [#1] SMP PTI CPU: 55 PID: 20887 Comm: cat Kdump: loaded Not tainted 5.2.0-rc6-jdc+ #13 RIP: 0010:rdma_counter_get_hwstat_value+0xf2/0x150 [ib_core] Call Trace: show_hw_stats+0x5e/0x130 [ib_core] dev_attr_show+0x15/0x50 sysfs_kf_seq_show+0xc6/0x1a0 seq_read+0x132/0x370 vfs_read+0x89/0x140 ksys_read+0x5c/0xd0 do_syscall_64+0x5a/0x240 entry_SYSCALL_64_after_hwframe+0x49/0xbe
--- drivers/infiniband/core/counters.c | 3 +++ 1 file changed, 3 insertions(+)
drivers/infiniband/core/counters.c +3 -0
@@ -393,6 +393,9 @@ u64 rdma_counter_get_hwstat_value(struct ib_device *dev, u8 port, u32 index)u64 sum; port_counter = &dev->port_data[port].port_counter; +if (!port_counter->hstats) +return 0 ; + sum =get_running_counters_hwstat_sum (dev, port, index); sum += port_counter->hstats->value[index];
--
2.20.1
PATCH 10/10 IB/counters: Initialize port counter and annotate mutex_destroy
In reply to Leon RomanovskyFrom: Parav Pandit <[email protected]>
Annotate mutex_destroy for port counters during counters release operation and during error unwinding during init flow.
Also port counter object should be initialized even if alloc_stats is unsupported, so that other QP bind operations can avoid call trace if they try to bind QP on RDMA device which doesn't support counters.
--- drivers/infiniband/core/counters.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
drivers/infiniband/core/counters.c +8 -8
@@ -595,9 +595,9 @@ int rdma_counter_get_mode(struct ib_device *dev, u8 port,void rdma_counter_init (struct ib_device *dev) {struct rdma_port_counter *port_counter; -u32 port; +u32 port, i; -if (!dev->ops.alloc_hw_stats || !dev->port_data) +if (!dev->port_data)return ;rdma_for_each_port (dev, port) {
@@ -605,6 +605,9 @@ void rdma_counter_init(struct ib_device *dev) port_counter->mode.mode =RDMA_COUNTER_MODE_NONE ;mutex_init (&port_counter->lock); +if (!dev->ops.alloc_hw_stats) +continue ; + port_counter->hstats = dev->ops.alloc_hw_stats (dev, port);if (!port_counter->hstats) goto fail;
@@ -613,13 +616,12 @@ void rdma_counter_init(struct ib_device *dev)return ; fail: -rdma_for_each_port (dev, port) { +for (i = port; i >=rdma_start_port (dev); i--) { port_counter = &dev->port_data[port].port_counter;kfree (port_counter->hstats); port_counter->hstats =NULL ; +mutex_destroy (&port_counter->lock); } - -return ; }void rdma_counter_release (struct ib_device *dev)
@@ -627,11 +629,9 @@ void rdma_counter_release(struct ib_device *dev)struct rdma_port_counter *port_counter;u32 port; -if (!dev->ops.alloc_hw_stats) -return ; -rdma_for_each_port (dev, port) { port_counter = &dev->port_data[port].port_counter;kfree (port_counter->hstats); +mutex_destroy (&port_counter->lock); } }
--
2.20.1
Re: [PATCH rdma-rc 10/10] IB/counters: Initialize port counter and annotate mutex_destroy
Quoted 9 lines · Leon Romanovsky
From: Parav Pandit <[email protected]>
Annotate
mutex_destroyfor port counters during counters release operation and during error unwinding during init flow.Also port counter object should be initialized even if
alloc_statsis unsupported, so that other QP bind operations can avoid call trace if they try to bind QP on RDMA device which doesn't support counters.
Quoted 7 lines
Fixes f34a55e497e81 ("RDMA/core: Get sum value of all counters when perform a sysfs stat read")Signed-off-by Parav Pandit <[email protected]>Reviewed-by Mark Zhang <[email protected]>Signed-off-by Leon Romanovsky <[email protected]>--- drivers/infiniband/core/counters.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
Don't put two things in a for-rc patch, I split it.
Jason
Quoted 15 lines · Leon Romanovsky
From: Leon Romanovsky <[email protected]>
Hi,
This is small patch set of fixes targeted for 5.3 and stable@.
Thanks
Moni Shoua (1):
IB/mlx5: Prevent concurrent MR updates during invalidationParav Pandit (4):
IB/core: Fix querying total rdma stats
IB/counters: Initialize port counter and annotatemutex_destroy
Quoted 6 lines
Yishai Hadas (5):
IB/mlx5: Fixunreg_umrto ignore the mkey state IB/mlx5: Use direct mkey destroy command upon UMR unreg failure IB/mlx5: Fixunreg_umrto set a device PD
IB/mlx5: Fixclean_mr()to work in the expected order IB/mlx5: Fix RSS Toeplitz function to be specification aligned
I took the above for for-rc
Quoted 3 lines
IB/mlx5: Avoid unnecessary typecast
RDMA/core: Annotate destroy of mutex to ensure that it is released as
unlocked
These are not really -rc patches, I took them to for-next
Jason
Quoted 39 lines
-----Original Message-----
From: Jason Gunthorpe <[email protected]>
Sent: Thursday, July 25, 2019 8:43 PM
To: Leon Romanovsky <[email protected]>
Cc: Doug Ledford <[email protected]>; Leon Romanovsky <[email protected]>; RDMA mailing list <[email protected]>; Alex Vainman <[email protected]>; Artemy Kovalyov <[email protected]>; Daniel Jurgens <[email protected]>; Eli Cohen <[email protected]>; Haggai Eran <[email protected]>; Mark Zhang <[email protected]>; Moni Shoua <[email protected]>; Parav Pandit <[email protected]>; Sagi Grimberg <[email protected]>; Yishai Hadas <[email protected]>
Subject: Re: [PATCH rdma-rc 00/10] Collection of fixes for 5.3Quoted 15 lines · Leon Romanovsky
From: Leon Romanovsky <[email protected]>
Hi,
This is small patch set of fixes targeted for 5.3 and stable@.
Thanks
Moni Shoua (1):
IB/mlx5: Prevent concurrent MR updates during invalidationParav Pandit (4):
IB/core: Fix querying total rdma stats
IB/counters: Initialize port counter and annotatemutex_destroyQuoted 6 lines
Yishai Hadas (5):
IB/mlx5: Fixunreg_umrto ignore the mkey state IB/mlx5: Use direct mkey destroy command upon UMR unreg failure IB/mlx5: Fixunreg_umrto set a device PD
IB/mlx5: Fixclean_mr()to work in the expected order IB/mlx5: Fix RSS Toeplitz function to be specification aligned
Quoted 8 lines
I took the above for for-rc
Quoted 3 lines
IB/mlx5: Avoid unnecessary typecast
RDMA/core: Annotate destroy of mutex to ensure that it is released as
unlockedThese are not really -rc patches, I took them to for-next
Thank you Jason.