RDMA fixes for-4.13
Subsystems (8)
The following changes since commit 5dc78ad1904db597bdb4427f3ead437aae86f54c:
IB/ipoib: Notify on modify QP failure only when relevant (2017-07-23 10:52:00 +0300)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/leon/linux-rdma.git tags/rdma-rc-2017-07-31
for you to fetch changes up to 38a974d578451dbbde0c40fc2d81fba44027a338:
IB/uverbs: Fix device cleanup (2017-07-30 10:43:15 +0300)
----------------------------------------------------------------
RDMA fixes for 4.13
This is cleanup series to various static checkers errors along with two major kernel panics fixes from from Parav and Yishai.
----------------------------------------------------------------
Leon Romanovsky (12):
RDMA/(core,ulp): Convert register/unregister event handler to be void RDMA/core: Cleanup device capability enum
RDMA/core: Remove unimplemented node_types and node transport RDMA/core: Delete BUG() from unreachable flow
RDMA/core: Refactor get link layer wrapper
RDMA/uverbs: Prevent leak of reserved field
RDMA/mlx4: Avoid potential derefence warning
RDMA/mlx4: Remove gfp_mask argument from acquire_group call RDMA/mlx5: Fix existence check for extended address vector RDMA/bnxt_re: Delete unsupported modify_port function RDMA/usnic: Fix remove address space warning
RDMA/mthca: Make explicit conversion to 64bit value
Parav Pandit (1):
IB/core: Fix race condition in resolving IP to MAC
Yishai Hadas (1):
IB/uverbs: Fix device cleanup
drivers/infiniband/core/addr.c | 62 +++++++++++++++++++------ drivers/infiniband/core/cache.c | 23 ++++----- drivers/infiniband/core/device.c | 8 +--- drivers/infiniband/core/sa_query.c | 3 +- drivers/infiniband/core/sysfs.c | 3 -- drivers/infiniband/core/uverbs_cmd.c | 2 +- drivers/infiniband/core/uverbs_main.c | 16 +------ drivers/infiniband/core/verbs.c | 32 ++++--------- drivers/infiniband/hw/bnxt_re/ib_verbs.c | 17 ------- drivers/infiniband/hw/bnxt_re/ib_verbs.h | 3 -- drivers/infiniband/hw/bnxt_re/main.c | 1 - drivers/infiniband/hw/mlx4/mcg.c | 11 ++--- drivers/infiniband/hw/mlx5/odp.c | 2 +- drivers/infiniband/hw/mthca/mthca_cmd.c | 2 +- drivers/infiniband/hw/usnic/usnic_fwd.c | 12 +---- drivers/infiniband/hw/usnic/usnic_fwd.h | 2 +- drivers/infiniband/hw/usnic/usnic_ib_main.c | 10 ++-- drivers/infiniband/ulp/ipoib/ipoib_main.c | 10 +--- drivers/infiniband/ulp/iser/iser_verbs.c | 6 +-- drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c | 7 +-- drivers/infiniband/ulp/srpt/ib_srpt.c | 5 +- include/linux/mlx5/qp.h | 1 - include/rdma/ib_verbs.h | 26 +++++------ 23 files changed, 105 insertions(+), 159 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): 20170731070924.7193-1-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
PATCH 1/14 RDMA/(core,ulp): Convert register/unregister event handler to be void
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The functions ib_register_event_handler() and ib_register_event_handler() always returned success and they can't fail.
Let's convert those functions to be void and remove redundant checks.
--- drivers/infiniband/core/cache.c | 23 ++++++++--------------- drivers/infiniband/core/device.c | 8 ++------ drivers/infiniband/core/sa_query.c | 3 +-- drivers/infiniband/core/uverbs_main.c | 13 +------------ drivers/infiniband/ulp/ipoib/ipoib_main.c | 10 +--------- drivers/infiniband/ulp/iser/iser_verbs.c | 6 ++---- drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c | 7 +------ drivers/infiniband/ulp/srpt/ib_srpt.c | 5 ++--- include/rdma/ib_verbs.h | 4 ++-- 9 files changed, 20 insertions(+), 59 deletions(-)
drivers/infiniband/core/cache.c +8 -15
@@ -1199,30 +1199,23 @@ int ib_cache_setup_one(struct ib_device *device) device->cache.ports =kzalloc (sizeof (*device->cache.ports) * (rdma_end_port (device) -rdma_start_port (device) +1 ),GFP_KERNEL ); -if (!device->cache.ports) { - err = -ENOMEM ; - goto out; - } +if (!device->cache.ports) +return -ENOMEM ; err =gid_table_setup_one (device); -if (err) - goto out; +if (err) { +kfree (device->cache.ports); + device->cache.ports =NULL ; +return err; + }for (p =0 ; p <=rdma_end_port (device) -rdma_start_port (device); ++p)ib_cache_update (device, p +rdma_start_port (device), true);INIT_IB_EVENT_HANDLER (&device->cache.event_handler, device, ib_cache_event); - err =ib_register_event_handler (&device->cache.event_handler); -if (err) - goto err; - +ib_register_event_handler (&device->cache.event_handler);return 0 ; - -err: -gid_table_cleanup_one (device); -out: -return err; }void ib_cache_release_one (struct ib_device *device)
drivers/infiniband/core/device.c +2 -6
@@ -711,7 +711,7 @@ EXPORT_SYMBOL(ib_set_client_data);* chapter 11 of the InfiniBand Architecture Specification). This * callback may occur in interrupt context. */ -int ib_register_event_handler (struct ib_event_handler *event_handler) +void ib_register_event_handler (struct ib_event_handler *event_handler) {unsigned long flags;
@@ -719,8 +719,6 @@ int ib_register_event_handler (struct ib_event_handler *event_handler)list_add_tail (&event_handler->list, &event_handler->device->event_handler_list);spin_unlock_irqrestore (&event_handler->device->event_handler_lock, flags); - -return 0 ; }EXPORT_SYMBOL (ib_register_event_handler);
@@ -731,15 +729,13 @@ EXPORT_SYMBOL(ib_register_event_handler);* Unregister an event handler registered with * ib_register_event_handler(). */ -int ib_unregister_event_handler (struct ib_event_handler *event_handler) +void ib_unregister_event_handler (struct ib_event_handler *event_handler) {unsigned long flags;spin_lock_irqsave (&event_handler->device->event_handler_lock, flags);list_del (&event_handler->list);spin_unlock_irqrestore (&event_handler->device->event_handler_lock, flags); - -return 0 ; }EXPORT_SYMBOL (ib_unregister_event_handler);
drivers/infiniband/core/sa_query.c +1 -2
@@ -2410,8 +2410,7 @@ static void ib_sa_add_one(struct ib_device *device)*/ INIT_IB_EVENT_HANDLER (&sa_dev->event_handler, device, ib_sa_event); -if (ib_register_event_handler (&sa_dev->event_handler)) - goto err; +ib_register_event_handler (&sa_dev->event_handler);for (i =0 ; i <= e - s; ++i) {if (rdma_cap_ib_sa (device, i +1 ))
drivers/infiniband/core/uverbs_main.c +1 -12
@@ -594,7 +594,6 @@ struct file *ib_uverbs_alloc_async_event_file(struct ib_uverbs_file *uverbs_file {struct ib_uverbs_async_event_file *ev_file;struct file *filp; -int ret; ev_file =kzalloc (sizeof (*ev_file),GFP_KERNEL );if (!ev_file)
@@ -620,21 +619,11 @@ struct file *ib_uverbs_alloc_async_event_file(struct ib_uverbs_file *uverbs_fileINIT_IB_EVENT_HANDLER (&uverbs_file->event_handler, ib_dev, ib_uverbs_event_handler); - ret =ib_register_event_handler (&uverbs_file->event_handler); -if (ret) - goto err_put_file; - +ib_register_event_handler (&uverbs_file->event_handler);/* At that point async file stuff was fully set */ return filp; -err_put_file: -fput (filp); -kref_put (&uverbs_file->async_file->ref, - ib_uverbs_release_async_event_file); - uverbs_file->async_file =NULL ; -return ERR_PTR (ret); - err_put_refs:kref_put (&ev_file->uverbs_file->ref, ib_uverbs_release_file);kref_put (&ev_file->ref, ib_uverbs_release_async_event_file);
drivers/infiniband/ulp/ipoib/ipoib_main.c +1 -9
@@ -2228,13 +2228,7 @@ static struct net_device *ipoib_add_port(const char *format,INIT_IB_EVENT_HANDLER (&priv->event_handler, priv->ca, ipoib_event); - result =ib_register_event_handler (&priv->event_handler); -if (result <0 ) { -printk (KERN_WARNING "%s: ib_register_event_handler failed for " -"port %d (ret = %d)\n" , - hca->name, port, result); - goto event_failed; - } +ib_register_event_handler (&priv->event_handler); result =register_netdev (priv->dev);if (result) {
@@ -2267,8 +2261,6 @@ static struct net_device *ipoib_add_port(const char *format,set_bit (IPOIB_STOP_NEIGH_GC , &priv->flags);cancel_delayed_work (&priv->neigh_reap_task);flush_workqueue (priv->wq); - -event_failed:ipoib_dev_cleanup (priv->dev); device_init_failed:
drivers/infiniband/ulp/iser/iser_verbs.c +2 -4
@@ -106,9 +106,7 @@ static int iser_create_device_ib_res(struct iser_device *device)INIT_IB_EVENT_HANDLER (&device->event_handler, ib_dev, iser_event_handler); -if (ib_register_event_handler (&device->event_handler)) - goto cq_err; - +ib_register_event_handler (&device->event_handler);return 0 ; cq_err:
@@ -141,7 +139,7 @@ static void iser_free_device_ib_res(struct iser_device *device) comp->cq =NULL ; } - (void )ib_unregister_event_handler (&device->event_handler); +ib_unregister_event_handler (&device->event_handler);ib_dealloc_pd (device->pd);kfree (device->comps);
drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c +1 -6
@@ -952,12 +952,7 @@ static int vema_register(struct opa_vnic_ctrl_port *cport)INIT_IB_EVENT_HANDLER (&port->event_handler, cport->ibdev, opa_vnic_event); - ret =ib_register_event_handler (&port->event_handler); -if (ret) { -c_err ("port %d: event handler register failed\n" , i); -vema_unregister (cport); -return ret; - } +ib_register_event_handler (&port->event_handler);idr_init (&port->vport_idr);mutex_init (&port->lock);
drivers/infiniband/ulp/srpt/ib_srpt.c +2 -3
@@ -2238,7 +2238,7 @@ static int srpt_write_pending(struct se_cmd *se_cmd) cqe, first_wr); cqe =NULL ; } - + ret =ib_post_send (ch->qp, first_wr, &bad_wr);if (ret) { pr_err("%s: ib_post_send() returned %d for %d (avail: %d)\n",
@@ -2530,8 +2530,7 @@ static void srpt_add_one(struct ib_device *device)INIT_IB_EVENT_HANDLER (&sdev->event_handler, sdev->device, srpt_event_handler); -if (ib_register_event_handler (&sdev->event_handler)) - goto err_cm; +ib_register_event_handler (&sdev->event_handler); sdev->ioctx_ring = (struct srpt_recv_ioctx **) srpt_alloc_ioctx_ring(sdev, sdev->srq_size,
include/rdma/ib_verbs.h +2 -2
@@ -2395,8 +2395,8 @@ int ib_modify_qp_is_ok(enum ib_qp_state cur_state, enum ib_qp_state next_state, enum ib_qp_type type, enum ib_qp_attr_mask mask, enum rdma_link_layer ll); -int ib_register_event_handler (struct ib_event_handler *event_handler); -int ib_unregister_event_handler (struct ib_event_handler *event_handler); +void ib_register_event_handler (struct ib_event_handler *event_handler); +void ib_unregister_event_handler (struct ib_event_handler *event_handler);void ib_dispatch_event (struct ib_event *event); int ib_query_port(struct ib_device *device,
--
2.13.3
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): 20170731070924.7193-2-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
Dennis Dalessandro
Reviewed-byRe: [rdma-rc 01/14] RDMA/(core,ulp): Convert register/unregister event handler to be void
Quoted 8 lines · Leon Romanovsky
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The functions
ib_register_event_handler()andib_register_event_handler()always returned success and they can't fail.Let's convert those functions to be void and remove redundant checks.
Quoted 11 lines
--- drivers/infiniband/core/cache.c | 23 ++++++++--------------- drivers/infiniband/core/device.c | 8 ++------ drivers/infiniband/core/sa_query.c | 3 +-- drivers/infiniband/core/uverbs_main.c | 13 +------------ drivers/infiniband/ulp/ipoib/ipoib_main.c | 10 +--------- drivers/infiniband/ulp/iser/iser_verbs.c | 6 ++---- drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c | 7 +------ drivers/infiniband/ulp/srpt/ib_srpt.c | 5 ++--- include/rdma/ib_verbs.h | 4 ++-- 9 files changed, 20 insertions(+), 59 deletions(-)
I think you mean "... and ib_unregister_event_handler()". Also maybe mention a couple words about this lets us clean up some of the goto stuff.
Otherwise:
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): 67e53715-1346-c969-b59a-ca4494d3b7c4-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org
Re: [rdma-rc 01/14] RDMA/(core,ulp): Convert register/unregister event handler to be void
Quoted 21 lines · Dennis Dalessandro
Quoted 8 lines · Leon Romanovsky
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The functions
ib_register_event_handler()andib_register_event_handler()always returned success and they can't fail.Let's convert those functions to be void and remove redundant checks.
Quoted 11 lines
--- drivers/infiniband/core/cache.c | 23 ++++++++--------------- drivers/infiniband/core/device.c | 8 ++------ drivers/infiniband/core/sa_query.c | 3 +-- drivers/infiniband/core/uverbs_main.c | 13 +------------ drivers/infiniband/ulp/ipoib/ipoib_main.c | 10 +--------- drivers/infiniband/ulp/iser/iser_verbs.c | 6 ++---- drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c | 7 +------ drivers/infiniband/ulp/srpt/ib_srpt.c | 5 ++--- include/rdma/ib_verbs.h | 4 ++-- 9 files changed, 20 insertions(+), 59 deletions(-)
Quoted 2 lines
I think you mean "... and
ib_unregister_event_handler()". Also maybe mention a couple words about this lets us clean up some of the goto stuff.
Right, I'll respin.
Quoted 4 lines
Otherwise:
Reviewed-by Dennis Dalessandro <[email protected]>
Thanks
Re: [rdma-rc 01/14] RDMA/(core,ulp): Convert register/unregister event handler to be void
In reply to a message not in the archiveQuoted 9 lines · Leon Romanovsky
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The functions
ib_register_event_handler()andib_register_event_handler()always returned success and they can't fail.Let's convert those functions to be void and remove redundant checks.
Quoted 12 lines
--- drivers/infiniband/core/cache.c | 23 ++++++++------- -------- drivers/infiniband/core/device.c | 8 ++------ drivers/infiniband/core/sa_query.c | 3 +-- drivers/infiniband/core/uverbs_main.c | 13 +------------ drivers/infiniband/ulp/ipoib/ipoib_main.c | 10 +--------- drivers/infiniband/ulp/iser/iser_verbs.c | 6 ++---- drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c | 7 +------ drivers/infiniband/ulp/srpt/ib_srpt.c | 5 ++--- include/rdma/ib_verbs.h | 4 ++-- 9 files changed, 20 insertions(+), 59 deletions(-)
Unless this fixes a build warning or an actual bug, this is not -rc material. You are putting a lot of stuff in -rc pull requests because "they are simple one line changes" or "it's a cleanup". That's not what -rc is for. It's for fixes, be they build fixes or run time fixes. Please split your pull requests appropriately and resubmit.
Signature · 8 lines
Doug Ledford <[email protected]> GPG KeyID: B826A3330E572FDD Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): [email protected]
Re: [rdma-rc 01/14] RDMA/(core,ulp): Convert register/unregister event handler to be void
Quoted 23 lines · Doug Ledford
Quoted 9 lines · Leon Romanovsky
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The functions
ib_register_event_handler()andib_register_event_handler()always returned success and they can't fail.Let's convert those functions to be void and remove redundant checks.
Quoted 12 lines
--- drivers/infiniband/core/cache.c | 23 ++++++++------- -------- drivers/infiniband/core/device.c | 8 ++------ drivers/infiniband/core/sa_query.c | 3 +-- drivers/infiniband/core/uverbs_main.c | 13 +------------ drivers/infiniband/ulp/ipoib/ipoib_main.c | 10 +--------- drivers/infiniband/ulp/iser/iser_verbs.c | 6 ++---- drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c | 7 +------ drivers/infiniband/ulp/srpt/ib_srpt.c | 5 ++--- include/rdma/ib_verbs.h | 4 ++-- 9 files changed, 20 insertions(+), 59 deletions(-)
Quoted 5 lines
Unless this fixes a build warning or an actual bug, this is not -rc material. You are putting a lot of stuff in -rc pull requests because "they are simple one line changes" or "it's a cleanup". That's not what -rc is for. It's for fixes, be they build fixes or run time fixes. Please split your pull requests appropriately and resubmit.
It is not cleanup, but my attempt to run smatch/sparse tools cleanly on drivers/infiniband.
I'll resubmit.
Thanks
Quoted 6 lines
--
Doug Ledford <[email protected]>GPG KeyID: B826A3330E572FDD Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
PATCH 2/14 RDMA/core: Cleanup device capability enum
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
Cleanup patch prior exporting the ib_device_cap_flags to the user space. In this patch, we are aligning the indentation and removing IB_DEVICE_INIT_TYPE field, because it is not used in the kernel.
--- include/rdma/ib_verbs.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
include/rdma/ib_verbs.h +3 -3
@@ -168,7 +168,7 @@ enum ib_device_cap_flags {IB_DEVICE_UD_AV_PORT_ENFORCE = (1 <<6 ),IB_DEVICE_CURR_QP_STATE_MOD = (1 <<7 ),IB_DEVICE_SHUTDOWN_PORT = (1 <<8 ), -IB_DEVICE_INIT_TYPE = (1 <<9 ), +/* Not in use, former INIT_TYPE = (1 << 9),*/ IB_DEVICE_PORT_ACTIVE_EVENT = (1 <<10 ),IB_DEVICE_SYS_IMAGE_GUID = (1 <<11 ), IB_DEVICE_RC_RNR_NAK_GEN = (1 << 12),
@@ -183,7 +183,7 @@ enum ib_device_cap_flags {* which will always contain a usable lkey. */ IB_DEVICE_LOCAL_DMA_LKEY = (1 <<15 ), -IB_DEVICE_RESERVED /* old SEND_W_INV */ = (1 <<16 ), +/* Reserved, old SEND_W_INV = (1 << 16),*/ IB_DEVICE_MEM_WINDOW = (1 << 17),/* * Devices should set IB_DEVICE_UD_IP_SUM if they support
@@ -218,7 +218,7 @@ enum ib_device_cap_flags {* of I/O operations with single completion queue managed * by hardware. */ -IB_DEVICE_CROSS_CHANNEL = (1 <<27 ), +IB_DEVICE_CROSS_CHANNEL = (1 <<27 ),IB_DEVICE_MANAGED_FLOW_STEERING = (1 <<29 ),IB_DEVICE_SIGNATURE_HANDOVER = (1 <<30 ), IB_DEVICE_ON_DEMAND_PAGING = (1ULL << 31),
--
2.13.3
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): 20170731070924.7193-3-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
Dennis Dalessandro
Reviewed-byRe: [rdma-rc 02/14] RDMA/core: Cleanup device capability enum
Quoted 8 lines · Leon Romanovsky
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
Cleanup patch prior exporting the
ib_device_cap_flagsto the user space. In this patch, we are aligning the indentation and removingIB_DEVICE_INIT_TYPEfield, because it is not used in the kernel.
Mention removing IB_DEVICE_RESERVED?
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 3/14 RDMA/core: Remove unimplemented node_types and node transport
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
There is no need to carry code, which is not implemented in any underlying hardware. This patch removes unimplemented node_types and their respective node transport layers.
--- drivers/infiniband/core/sysfs.c | 3 --- drivers/infiniband/core/verbs.c | 5 ----- include/rdma/ib_verbs.h | 16 ++++++---------- 3 files changed, 6 insertions(+), 18 deletions(-)
drivers/infiniband/core/sysfs.c +0 -3
@@ -1146,10 +1146,7 @@ static ssize_t show_node_type(struct device *device,switch (dev->node_type) {case RDMA_NODE_IB_CA :return sprintf (buf,"%d: CA\n" , dev->node_type);case RDMA_NODE_RNIC :return sprintf (buf,"%d: RNIC\n" , dev->node_type); -case RDMA_NODE_USNIC :return sprintf (buf,"%d: usNIC\n" , dev->node_type);case RDMA_NODE_USNIC_UDP :return sprintf (buf,"%d: usNIC UDP\n" , dev->node_type); -case RDMA_NODE_IB_SWITCH :return sprintf (buf,"%d: switch\n" , dev->node_type); -case RDMA_NODE_IB_ROUTER :return sprintf (buf,"%d: router\n" , dev->node_type);default :return sprintf (buf,"%d: <unknown>\n" , dev->node_type); } }
drivers/infiniband/core/verbs.c +0 -5
@@ -182,13 +182,9 @@ rdma_node_get_transport(enum rdma_node_type node_type) {switch (node_type) {case RDMA_NODE_IB_CA : -case RDMA_NODE_IB_SWITCH : -case RDMA_NODE_IB_ROUTER :return RDMA_TRANSPORT_IB ;case RDMA_NODE_RNIC :return RDMA_TRANSPORT_IWARP ; -case RDMA_NODE_USNIC : -return RDMA_TRANSPORT_USNIC ;case RDMA_NODE_USNIC_UDP :return RDMA_TRANSPORT_USNIC_UDP ;default :
@@ -207,7 +203,6 @@ enum rdma_link_layer rdma_port_get_link_layer(struct ib_device *device, u8 port_case RDMA_TRANSPORT_IB :return IB_LINK_LAYER_INFINIBAND ;case RDMA_TRANSPORT_IWARP : -case RDMA_TRANSPORT_USNIC :case RDMA_TRANSPORT_USNIC_UDP :return IB_LINK_LAYER_ETHERNET ;default :
include/rdma/ib_verbs.h +6 -10
@@ -93,12 +93,9 @@ struct ib_gid_attr {enum rdma_node_type {/* IB values map to NodeInfo:NodeType. */ -RDMA_NODE_IB_CA =1 , -RDMA_NODE_IB_SWITCH , -RDMA_NODE_IB_ROUTER , -RDMA_NODE_RNIC , -RDMA_NODE_USNIC , -RDMA_NODE_USNIC_UDP , +RDMA_NODE_IB_CA =1 , +RDMA_NODE_RNIC =4 , +RDMA_NODE_USNIC_UDP =6 , };enum {
@@ -107,10 +104,9 @@ enum { };enum rdma_transport_type { -RDMA_TRANSPORT_IB , -RDMA_TRANSPORT_IWARP , -RDMA_TRANSPORT_USNIC , -RDMA_TRANSPORT_USNIC_UDP +RDMA_TRANSPORT_IB =0 , +RDMA_TRANSPORT_IWARP =1 , +RDMA_TRANSPORT_USNIC_UDP =3 , };enum rdma_protocol_type {
--
2.13.3
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): 20170731070924.7193-4-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
Re: [rdma-rc 03/14] RDMA/core: Remove unimplemented node_types and node transport
Quoted 4 lines · Leon Romanovsky
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
There is no need to carry code, which is not implemented in any underlying hardware.
AFAIK there are Linux based IB switch implementations and there may be IB router implementation(s) whose drivers have not been pushed upstream so it would be better to leave at least those 2 node types in.
-- Hal
Quoted 88 lines
This patch removes unimplemented
node_typesand
their respective node transport layers.--- drivers/infiniband/core/sysfs.c | 3 --- drivers/infiniband/core/verbs.c | 5 ----- include/rdma/ib_verbs.h | 16 ++++++---------- 3 files changed, 6 insertions(+), 18 deletions(-)drivers/infiniband/core/sysfs.c +0
-3@@ -1146,10 +1146,7 @@ static ssize_t show_node_type(struct device *device,switch (dev->node_type) {case RDMA_NODE_IB_CA :return sprintf (buf,"%d: CA\n" , dev->node_type);case RDMA_NODE_RNIC :return sprintf (buf,"%d: RNIC\n" , dev->node_type); -case RDMA_NODE_USNIC :return sprintf (buf,"%d: usNIC\n" , dev->node_type);case RDMA_NODE_USNIC_UDP :return sprintf (buf,"%d: usNIC UDP\n" , dev->node_type); -case RDMA_NODE_IB_SWITCH :return sprintf (buf,"%d: switch\n" , dev->node_type); -case RDMA_NODE_IB_ROUTER :return sprintf (buf,"%d: router\n" , dev->node_type);default :return sprintf (buf,"%d: <unknown>\n" , dev->node_type); } }drivers/infiniband/core/verbs.c +0
-5@@ -182,13 +182,9 @@ rdma_node_get_transport(enum rdma_node_type node_type) {switch (node_type) {case RDMA_NODE_IB_CA : -case RDMA_NODE_IB_SWITCH : -case RDMA_NODE_IB_ROUTER :return RDMA_TRANSPORT_IB ;case RDMA_NODE_RNIC :return RDMA_TRANSPORT_IWARP ; -case RDMA_NODE_USNIC : -return RDMA_TRANSPORT_USNIC ;case RDMA_NODE_USNIC_UDP :return RDMA_TRANSPORT_USNIC_UDP ;default :@@ -207,7 +203,6 @@ enum rdma_link_layer rdma_port_get_link_layer(struct ib_device *device, u8 port_case RDMA_TRANSPORT_IB :return IB_LINK_LAYER_INFINIBAND ;case RDMA_TRANSPORT_IWARP : -case RDMA_TRANSPORT_USNIC :case RDMA_TRANSPORT_USNIC_UDP :return IB_LINK_LAYER_ETHERNET ;default :include/rdma/ib_verbs.h +6
-11@@ -93,12 +93,9 @@ struct ib_gid_attr {enum rdma_node_type {/* IB values map to NodeInfo:NodeType. */ -RDMA_NODE_IB_CA =1 , -RDMA_NODE_IB_SWITCH , -RDMA_NODE_IB_ROUTER , -RDMA_NODE_RNIC , -RDMA_NODE_USNIC , -RDMA_NODE_USNIC_UDP , +RDMA_NODE_IB_CA =1 , +RDMA_NODE_RNIC =4 , +RDMA_NODE_USNIC_UDP =6 , };enum {@@ -107,10 +104,9 @@ enum { };enum rdma_transport_type { -RDMA_TRANSPORT_IB , -RDMA_TRANSPORT_IWARP , -RDMA_TRANSPORT_USNIC , -RDMA_TRANSPORT_USNIC_UDP +RDMA_TRANSPORT_IB =0 , +RDMA_TRANSPORT_IWARP =1 , +RDMA_TRANSPORT_USNIC_UDP =3 , };enum rdma_protocol_type { --2.13.3
Quoted 5 lines
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): 2e1b38f9-ae7a-4f5e-9cac-168f1f9b56af-LDSdmyG8hGV8YrgS2mwiifqBs+8SCbDb@public.gmane.org
Re: [rdma-rc 03/14] RDMA/core: Remove unimplemented node_types and node transport
Quoted 9 lines · Hal Rosenstock
Quoted 4 lines · Leon Romanovsky
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
There is no need to carry code, which is not implemented in any underlying hardware.
AFAIK there are Linux based IB switch implementations and there may be IB router implementation(s) whose drivers have not been pushed upstream so it would be better to leave at least those 2 node types in.
So if they care, this code will be pushed together with needed fields. Right now, this code is not connected in current linux kernel code, not tested and can't be exposed.
It is dead code from upstream point of view.
Thanks
A message that is not in the archive (sent off-list or not yet received): 20170731121823.GV13672-U/[email protected]
Re: [rdma-rc 03/14] RDMA/core: Remove unimplemented node_types and node transport
Quoted 11 lines · Leon Romanovsky
Quoted 9 lines · Hal Rosenstock
Quoted 4 lines · Leon Romanovsky
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
There is no need to carry code, which is not implemented in any underlying hardware.
AFAIK there are Linux based IB switch implementations and there may be IB router implementation(s) whose drivers have not been pushed upstream so it would be better to leave at least those 2 node types in.
Quoted 8 lines
So if they care, this code will be pushed together with needed fields. Right now, this code is not connected in current linux kernel code, not tested and can't be exposed.
It is dead code from upstream point of view.
Thanks
I would be fine leaving the code in for a driver which was in the works or fairly new. For instance the bnxt patch (11/14), perhaps there are plans to implement that stuff soonish. I'll let that driver's maintainer make that case though.
However these things are not new and unless someone is working on code to implement those things I would lean toward removing.
There is a fine line between dead code and code waiting to be written.
-Denny
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): 63a8961c-7341-80c2-c417-67d4e6fd4702-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org
Re: [rdma-rc 03/14] RDMA/core: Remove unimplemented node_types and node transport
Quoted 26 lines · Dennis Dalessandro
Quoted 11 lines · Leon Romanovsky
Quoted 9 lines · Hal Rosenstock
Quoted 4 lines · Leon Romanovsky
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
There is no need to carry code, which is not implemented in any underlying hardware.
AFAIK there are Linux based IB switch implementations and there may be IB router implementation(s) whose drivers have not been pushed upstream so it would be better to leave at least those 2 node types in.
Quoted 8 lines
So if they care, this code will be pushed together with needed fields. Right now, this code is not connected in current linux kernel code, not tested and can't be exposed.
It is dead code from upstream point of view.
Thanks
I would be fine leaving the code in for a driver which was in the works or fairly new. For instance the bnxt patch (11/14), perhaps there are plans to implement that stuff soonish. I'll let that driver's maintainer make that case though.
Quoted 6 lines
However these things are not new and unless someone is working on code to implement those things I would lean toward removing.
There is a fine line between dead code and code waiting to be written.
-Denny
Neither of which is -rc material though.
Signature · 3 lines
Doug Ledford <[email protected]> GPG Key ID: B826A3330E572FDD Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
PATCH 4/14 RDMA/core: Delete BUG() from unreachable flow
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
Remove call to BUG() in case wrong node_type was provided. This flow is unreachable, because node_types are supplied from specific enum.
--- drivers/infiniband/core/verbs.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-)
drivers/infiniband/core/verbs.c +5 -9
@@ -180,17 +180,13 @@ EXPORT_SYMBOL(ib_rate_to_mbps); __attribute_const__ enum rdma_transport_typerdma_node_get_transport (enum rdma_node_type node_type) { -switch (node_type) { -case RDMA_NODE_IB_CA : +if (node_type ==RDMA_NODE_IB_CA )return RDMA_TRANSPORT_IB ; -case RDMA_NODE_RNIC : + +if (node_type ==RDMA_NODE_RNIC )return RDMA_TRANSPORT_IWARP ; -case RDMA_NODE_USNIC_UDP : -return RDMA_TRANSPORT_USNIC_UDP ; -default : -BUG (); -return 0 ; - } + +return RDMA_TRANSPORT_USNIC_UDP ; }EXPORT_SYMBOL (rdma_node_get_transport);
--
2.13.3
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): 20170731070924.7193-5-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
Dennis Dalessandro
Reviewed-byRe: [rdma-rc 04/14] RDMA/core: Delete BUG() from unreachable flow
Quoted 7 lines · Leon Romanovsky
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
Remove call to
BUG()in case wrongnode_typewas provided. This flow is unreachable, becausenode_typesare supplied from specific enum.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 5/14 RDMA/core: Refactor get link layer wrapper
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The return values from rdma_node_get_transport() are strict and IB_LINK_LAYER_UNSPECIFIED is unreachable in this flow.
--- drivers/infiniband/core/verbs.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-)
drivers/infiniband/core/verbs.c +5 -8
@@ -192,18 +192,15 @@ EXPORT_SYMBOL(rdma_node_get_transport);enum rdma_link_layer rdma_port_get_link_layer (struct ib_device *device,u8 port_num) { +enum rdma_transport_type lt;if (device->get_link_layer)return device->get_link_layer (device, port_num); -switch (rdma_node_get_transport (device->node_type)) { -case RDMA_TRANSPORT_IB : + lt =rdma_node_get_transport (device->node_type); +if (lt ==RDMA_TRANSPORT_IB )return IB_LINK_LAYER_INFINIBAND ; -case RDMA_TRANSPORT_IWARP : -case RDMA_TRANSPORT_USNIC_UDP : -return IB_LINK_LAYER_ETHERNET ; -default : -return IB_LINK_LAYER_UNSPECIFIED ; - } + +return IB_LINK_LAYER_ETHERNET ; }EXPORT_SYMBOL (rdma_port_get_link_layer);
--
2.13.3
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): 20170731070924.7193-6-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
Dennis Dalessandro
Reviewed-byRe: [rdma-rc 05/14] RDMA/core: Refactor get link layer wrapper
Quoted 6 lines · Leon Romanovsky
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The return values from
rdma_node_get_transport()are strict andIB_LINK_LAYER_UNSPECIFIEDis unreachable in this flow.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 6/14 IB/core: Fix race condition in resolving IP to MAC
In reply to a message not in the archiveFrom: Parav Pandit <parav-VPRAkNaXOzVWk0Htik3J/[email protected]>
Currently while resolving IP address to MAC address single delayed work is used for resolving multiple such resolve requests. This singled work is essentially performs two tasks.
(a) any retry needed to resolve and
(b) it executes the callback function for all completed requests
While work is executing callbacks, any new work scheduled on for this workqueue is lost because workqueue has completed looking at all pending requests and now looking at callbacks, but work is still under execution. Any further retry to look at pending requests in process_req() after executing callbacks would lead to similar race condition (may be reduce the probably further but doesn't eliminate it). Retrying to enqueue work that from queue_req() context is not something rest of the kernel modules have followed.
Therefore fix in this patch utilizes kernel facility to enqueue multiple work items to a workqueue. This ensures that no such requests gets lost in synchronization. Request list is still maintained so that rdma_cancel_addr() can unlink the request and get the completion with error sooner. Neighbour update event handling continues to be handled in same way as before.
Additionally process_req() work entry cancels any pending work for a request that gets completed while processing those requests.
Originally ib_addr was ST workqueue, but it became MT work queue with patch of [1]. This patch again makes it similar to ST so that neighbour update events handler work item doesn't race with other work items.
In one such below trace, (though on 4.5 based kernel) it can be seen that process_req() never executed the callback, which is likely for an event that was schedule by queue_req() when previous callback was getting executed by workqueue.
[<ffffffff816b0dde>] schedule+0x3e/0x90 [<ffffffff816b3c45>] schedule_timeout+0x1b5/0x210 [<ffffffff81618c37>] ? ip_route_output_flow+0x27/0x70 [<ffffffffa027f9c9>] ? addr_resolve+0x149/0x1b0 [ib_addr] [<ffffffff816b228f>] wait_for_completion+0x10f/0x170 [<ffffffff810b6140>] ? try_to_wake_up+0x210/0x210 [<ffffffffa027f220>] ? rdma_copy_addr+0xa0/0xa0 [ib_addr] [<ffffffffa0280120>] rdma_addr_find_l2_eth_by_grh+0x1d0/0x278 [ib_addr] [<ffffffff81321297>] ? sub_alloc+0x77/0x1c0 [<ffffffffa02943b7>] ib_init_ah_from_wc+0x3a7/0x5a0 [ib_core] [<ffffffffa0457aba>] cm_req_handler+0xea/0x580 [ib_cm] [<ffffffff81015982>] ? __switch_to+0x212/0x5e0 [<ffffffffa04582fd>] cm_work_handler+0x6d/0x150 [ib_cm] [<ffffffff810a14c1>] process_one_work+0x151/0x4b0 [<ffffffff810a1940>] worker_thread+0x120/0x480 [<ffffffff816b074b>] ? __schedule+0x30b/0x890 [<ffffffff810a1820>] ? process_one_work+0x4b0/0x4b0 [<ffffffff810a1820>] ? process_one_work+0x4b0/0x4b0 [<ffffffff810a6b1e>] kthread+0xce/0xf0 [<ffffffff810a6a50>] ? kthread_freezable_should_stop+0x70/0x70 [<ffffffff816b53a2>] ret_from_fork+0x42/0x70 [<ffffffff810a6a50>] ? kthread_freezable_should_stop+0x70/0x70 INFO: task kworker/u144:1:156520 blocked for more than 120 seconds. "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. kworker/u144:1 D ffff883ffe1d7600 0 156520 2 0x00000080 Workqueue: ib_addr process_req [ib_addr] ffff883f446fbbd8 0000000000000046 ffff881f95280000 ffff881ff24de200 ffff883f66120000 ffff883f446f8008 ffff881f95280000 ffff883f6f9208c4 ffff883f6f9208c8 00000000ffffffff ffff883f446fbbf8 ffffffff816b0dde
[1] http://lkml.iu.edu/hypermail/linux/kernel/1608.1/05834.html
--- drivers/infiniband/core/addr.c | 62 ++++++++++++++++++++++++++++++++---------- 1 file changed, 48 insertions(+), 14 deletions(-)
drivers/infiniband/core/addr.c +48 -14
@@ -61,6 +61,7 @@ struct addr_req {void (*callback)(int status,struct sockaddr *src_addr,struct rdma_dev_addr *addr,void *context);unsigned long timeout; +struct delayed_work work;int status;u32 seq; };
@@ -295,7 +296,7 @@ int rdma_translate_ip(const struct sockaddr *addr, }EXPORT_SYMBOL (rdma_translate_ip); -static void set_timeout (unsigned long time) +static void set_timeout (struct delayed_work *delayed_work,unsigned long time) {unsigned long delay;
@@ -303,7 +304,7 @@ static void set_timeout(unsigned long time)if ((long )delay <0 ) delay =0 ; -mod_delayed_work (addr_wq, &work, delay); +mod_delayed_work (addr_wq, delayed_work, delay); }static void queue_req (struct addr_req *req)
@@ -318,8 +319,7 @@ static void queue_req(struct addr_req *req)list_add (&req->list, &temp_req->list); -if (req_list.next == &req->list) -set_timeout (req->timeout); +set_timeout (&req->work, req->timeout);mutex_unlock (&lock); }
@@ -574,6 +574,37 @@ static int addr_resolve(struct sockaddr *src_in,return ret; } +static void process_one_req (struct work_struct *_work) +{ +struct addr_req *req; +struct sockaddr *src_in, *dst_in; + +mutex_lock (&lock); + req = container_of(_work, struct addr_req, work.work); + +if (req->status == -ENODATA ) { + src_in = (struct sockaddr *)&req->src_addr; + dst_in = (struct sockaddr *)&req->dst_addr; + req->status =addr_resolve (src_in, dst_in, req->addr, + true, req->seq); +if (req->status &&time_after_eq (jiffies, req->timeout)) { + req->status = -ETIMEDOUT ; + }else if (req->status == -ENODATA ) { +/* requeue the work for retrying again */ +set_timeout (&req->work, req->timeout); +mutex_unlock (&lock); +return ; + } + } +list_del (&req->list); +mutex_unlock (&lock); + + req->callback (req->status, (struct sockaddr *)&req->src_addr, + req->addr, req->context); +put_client (req->client); +kfree (req); +} +static void process_req (struct work_struct *work) {struct addr_req *req, *temp_req;
@@ -591,20 +622,23 @@ static void process_req(struct work_struct *work) true, req->seq);if (req->status &&time_after_eq (jiffies, req->timeout)) req->status = -ETIMEDOUT ; -else if (req->status == -ENODATA ) +else if (req->status == -ENODATA ) { +set_timeout (&req->work, req->timeout);continue ; + } }list_move_tail (&req->list, &done_list); } -if (!list_empty (&req_list)) { - req = list_entry(req_list.next, struct addr_req, list); -set_timeout (req->timeout); - }mutex_unlock (&lock);list_for_each_entry_safe (req, temp_req, &done_list, list) {list_del (&req->list); +/* It is safe to cancel other work items from this work item +* because at a time there can be only one work item running +* with this single threaded work queue. +*/ +cancel_delayed_work (&req->work); req->callback (req->status, (struct sockaddr *) &req->src_addr, req->addr, req->context);put_client (req->client);
@@ -647,6 +681,7 @@ int rdma_resolve_ip(struct rdma_addr_client *client, req->context = context; req->client = client;atomic_inc (&client->refcount); +INIT_DELAYED_WORK (&req->work, process_one_req); req->seq = (u32 )atomic_inc_return (&ib_nl_addr_request_seq); req->status =addr_resolve (src_in, dst_in, addr, true, req->seq);
@@ -701,7 +736,7 @@ void rdma_addr_cancel(struct rdma_dev_addr *addr) req->status = -ECANCELED ; req->timeout = jiffies;list_move (&req->list, &req_list); -set_timeout (req->timeout); +set_timeout (&req->work, req->timeout);break ; } }
@@ -807,9 +842,8 @@ static int netevent_callback(struct notifier_block *self, unsigned long event,if (event ==NETEVENT_NEIGH_UPDATE ) {struct neighbour *neigh = ctx; -if (neigh->nud_state &NUD_VALID ) { -set_timeout (jiffies); - } +if (neigh->nud_state &NUD_VALID ) +set_timeout (&work, jiffies); }return 0 ; }
@@ -820,7 +854,7 @@ static struct notifier_block nb = {int addr_init (void ) { - addr_wq =alloc_workqueue ("ib_addr" ,WQ_MEM_RECLAIM ,0 ); + addr_wq =alloc_ordered_workqueue ("ib_addr" ,WQ_MEM_RECLAIM );if (!addr_wq)return -ENOMEM ;
Signature · 6 lines
2.13.3 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 7/14 RDMA/uverbs: Prevent leak of reserved field
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
initialize to zero the response structure to prevent the leakage of "resp.reserved" field.
drivers/infiniband/core/uverbs_cmd.c:1178 ib_uverbs_resize_cq() warn:
check that 'resp.reserved' doesn't leak information
--- drivers/infiniband/core/uverbs_cmd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/infiniband/core/uverbs_cmd.c +1 -1
@@ -1153,7 +1153,7 @@ ssize_t ib_uverbs_resize_cq(struct ib_uverbs_file *file, int out_len) {struct ib_uverbs_resize_cq cmd; -struct ib_uverbs_resize_cq_resp resp; +struct ib_uverbs_resize_cq_resp resp = {};struct ib_udata udata;struct ib_cq *cq;int ret = -EINVAL ;
--
2.13.3
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): 20170731070924.7193-8-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
Dennis Dalessandro
Reviewed-byRe: [rdma-rc 07/14] RDMA/uverbs: Prevent leak of reserved field
Quoted 10 lines · Leon Romanovsky
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
initialize to zero the response structure to prevent the leakage of "resp.reserved" field.
drivers/infiniband/core/uverbs_cmd.c:1178ib_uverbs_resize_cq()warn:
check that 'resp.reserved' doesn't leak informationFixes 33b9b3ee9709 ("IB: Add userspace support for resizing CQs")
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 8/14 RDMA/mlx4: Avoid potential derefence warning
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The smatch complains about possible dereference, it is unclear from source how it can occur, since in case of failure to find/allocate group, the acquire_group() will return error.
Avoid the following error, by changing IS_ERR() to be IS_ERR_OR_NULL().
drivers/infiniband/hw/mlx4/mcg.c:964 mlx4_ib_mcg_multiplex_handler() error:
potential null dereference 'group'. (acquire_group returns null)
--- drivers/infiniband/hw/mlx4/mcg.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/infiniband/hw/mlx4/mcg.c +1 -1
@@ -956,7 +956,7 @@ int mlx4_ib_mcg_multiplex_handler(struct ib_device *ibdev, int port,mutex_lock (&ctx->mcg_table_lock); group =acquire_group (ctx, &rec->mgid, may_create,GFP_KERNEL );mutex_unlock (&ctx->mcg_table_lock); -if (IS_ERR (group)) { +if (IS_ERR_OR_NULL (group)) {kfree (req);return PTR_ERR (group); }
--
2.13.3
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): 20170731070924.7193-9-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
Re: [rdma-rc 08/14] RDMA/mlx4: Avoid potential derefence warning
Quoted 10 lines · Leon Romanovsky
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The smatch complains about possible dereference, it is unclear from source how it can occur, since in case of failure to find/allocate group, the
acquire_group()will return error.Avoid the following error, by changing
IS_ERR()to beIS_ERR_OR_NULL().
drivers/infiniband/hw/mlx4/mcg.c:964mlx4_ib_mcg_multiplex_handler()error:
potential null dereference 'group'. (acquire_groupreturns null)
Hello Leon,
I agree with you that it's not clear from the source code how acquire_group() could return null. My proposal is to drop this patch and to report this as a bug to the smatch author (Dan Carpenter). I don't think it makes sense to modify kernel code that is fine because a tool (smatch) reports a false positive.
Thanks,
Bart.--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [rdma-rc 08/14] RDMA/mlx4: Avoid potential derefence warning
In reply to a message not in the archiveQuoted 8 lines · Leon Romanovsky
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The smatch complains about possible dereference, it is unclear from source how it can occur, since in case of failure to find/allocate group, the
acquire_group()will return error.Avoid the following error, by changing
IS_ERR()to beIS_ERR_OR_NULL().
Quoted 9 lines
drivers/infiniband/hw/mlx4/mcg.c:964mlx4_ib_mcg_multiplex_handler()error:
potential null dereference 'group'. (acquire_groupreturns null)Fixes b9c5d6a64358 ("IB/mlx4: Add multicast group (MCG) paravirtualization for SR-IOV")--- drivers/infiniband/hw/mlx4/mcg.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Hi Dan,
Bart [1] and me both believe that smatch tool is incorrectly reported error. Do you think that the tool should be fixed?
Thanks
[1] http://marc.info/?l=linux-rdma&m=150151394813006&w=2
Quoted 16 lines
drivers/infiniband/hw/mlx4/mcg.c +1
-2@@ -956,7 +956,7 @@ int mlx4_ib_mcg_multiplex_handler(struct ib_device *ibdev, int port,mutex_lock (&ctx->mcg_table_lock); group =acquire_group (ctx, &rec->mgid, may_create,GFP_KERNEL );mutex_unlock (&ctx->mcg_table_lock); -if (IS_ERR (group)) { +if (IS_ERR_OR_NULL (group)) {kfree (req);return PTR_ERR (group); } --2.13.3
A message that is not in the archive (sent off-list or not yet received): 20170731162133.GB13672-U/[email protected]
Re: [rdma-rc 08/14] RDMA/mlx4: Avoid potential derefence warning
Quoted 19 lines · Leon Romanovsky
Quoted 8 lines · Leon Romanovsky
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The smatch complains about possible dereference, it is unclear from source how it can occur, since in case of failure to find/allocate group, the
acquire_group()will return error.Avoid the following error, by changing
IS_ERR()to beIS_ERR_OR_NULL().Quoted 9 lines
drivers/infiniband/hw/mlx4/mcg.c:964mlx4_ib_mcg_multiplex_handler()error:
potential null dereference 'group'. (acquire_groupreturns null)Fixes b9c5d6a64358 ("IB/mlx4: Add multicast group (MCG) paravirtualization for SR-IOV")--- drivers/infiniband/hw/mlx4/mcg.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 5 lines
Hi Dan,
Bart [1] and me both believe that smatch tool is incorrectly reported error. Do you think that the tool should be fixed?
Yeah. There are two functions called acquire_group() and one returns NULL and one returns error pointers. It's getting them mixed up. It's simple enough to silence the warning. I'll push that tomorrow.
I normally run with the cross function database so I don't see this warning on my builds. The cross function DB doesn't get confused when there are two functions with the same name.
regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [rdma-rc 08/14] RDMA/mlx4: Avoid potential derefence warning
Quoted 26 lines · Dan Carpenter
Quoted 19 lines · Leon Romanovsky
Quoted 8 lines · Leon Romanovsky
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The smatch complains about possible dereference, it is unclear from source how it can occur, since in case of failure to find/allocate group, the
acquire_group()will return error.Avoid the following error, by changing
IS_ERR()to beIS_ERR_OR_NULL().Quoted 9 lines
drivers/infiniband/hw/mlx4/mcg.c:964mlx4_ib_mcg_multiplex_handler()error:
potential null dereference 'group'. (acquire_groupreturns null)Fixes b9c5d6a64358 ("IB/mlx4: Add multicast group (MCG) paravirtualization for SR-IOV")--- drivers/infiniband/hw/mlx4/mcg.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)Quoted 5 lines
Hi Dan,
Bart [1] and me both believe that smatch tool is incorrectly reported error. Do you think that the tool should be fixed?
Quoted 7 lines
Yeah. There are two functions called
acquire_group()and one returns NULL and one returns error pointers. It's getting them mixed up. It's simple enough to silence the warning. I'll push that tomorrow.I normally run with the cross function database so I don't see this warning on my builds. The cross function DB doesn't get confused when there are two functions with the same name.
Thanks
Quoted 3 lines
regards,
dan carpenter
PATCH 9/14 RDMA/mlx4: Remove gfp_mask argument from acquire_group call
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
All callers of acquire_group() passed the same gfp_mask to it and it is safe to remove it.
--- drivers/infiniband/hw/mlx4/mcg.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
drivers/infiniband/hw/mlx4/mcg.c +4 -5
@@ -808,8 +808,7 @@ static ssize_t sysfs_show_group(struct device *dev, struct device_attribute *attr, char *buf);static struct mcast_group *acquire_group (struct mlx4_ib_demux_ctx *ctx, -union ib_gid *mgid,int create, -gfp_t gfp_mask) +union ib_gid *mgid,int create) {struct mcast_group *group, *cur_group;int is_mgid0;
@@ -825,7 +824,7 @@ static struct mcast_group *acquire_group(struct mlx4_ib_demux_ctx *ctx,if (!create)return ERR_PTR (-ENOENT ); - group =kzalloc (sizeof *group, gfp_mask); + group =kzalloc (sizeof (*group),GFP_KERNEL );if (!group)return ERR_PTR (-ENOMEM );
@@ -892,7 +891,7 @@ int mlx4_ib_mcg_demux_handler(struct ib_device *ibdev, int port, int slave,case IB_MGMT_METHOD_GET_RESP :case IB_SA_METHOD_DELETE_RESP :mutex_lock (&ctx->mcg_table_lock); - group =acquire_group (ctx, &rec->mgid,0 ,GFP_KERNEL ); + group =acquire_group (ctx, &rec->mgid,0 );mutex_unlock (&ctx->mcg_table_lock);if (IS_ERR (group)) {if (mad->mad_hdr.method ==IB_MGMT_METHOD_GET_RESP ) {
@@ -954,7 +953,7 @@ int mlx4_ib_mcg_multiplex_handler(struct ib_device *ibdev, int port, req->sa_mad = *sa_mad;mutex_lock (&ctx->mcg_table_lock); - group =acquire_group (ctx, &rec->mgid, may_create,GFP_KERNEL ); + group =acquire_group (ctx, &rec->mgid, may_create);mutex_unlock (&ctx->mcg_table_lock);if (IS_ERR_OR_NULL (group)) {kfree (req);
--
2.13.3
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): 20170731070924.7193-10-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
Dennis Dalessandro
Reviewed-byRe: [rdma-rc 09/14] RDMA/mlx4: Remove gfp_mask argument from acquire_group call
Quoted 6 lines · Leon Romanovsky
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
All callers of
acquire_group()passed the samegfp_maskto it and it is safe to remove it.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 10/14 RDMA/mlx5: Fix existence check for extended address vector
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The extended address vector is the highest bit in be32 variable, but it was compared with the lowest. This patch fixes the endianness of that check and removes already declared define.
--- drivers/infiniband/hw/mlx5/odp.c | 2 +- include/linux/mlx5/qp.h | 1 - 2 files changed, 1 insertion(+), 2 deletions(-)
drivers/infiniband/hw/mlx5/odp.c +1 -1
@@ -939,7 +939,7 @@ static int mlx5_ib_mr_initiator_pfault_handler(if (qp->ibqp.qp_type !=IB_QPT_RC ) { av = *wqe; -if (av->dqp_dct &be32_to_cpu (MLX5_WQE_AV_EXT )) +if (av->dqp_dct &cpu_to_be32 (MLX5_EXTENDED_UD_AV )) *wqe +=sizeof (struct mlx5_av );else *wqe +=sizeof (struct mlx5_base_av );
include/linux/mlx5/qp.h +0 -1
@@ -212,7 +212,6 @@ struct mlx5_wqe_ctrl_seg {#define MLX5_WQE_CTRL_OPCODE_MASK 0xff#define MLX5_WQE_CTRL_WQE_INDEX_MASK 0x00ffff00#define MLX5_WQE_CTRL_WQE_INDEX_SHIFT 8 -#define MLX5_WQE_AV_EXT 0x80000000enum {MLX5_ETH_WQE_L3_INNER_CSUM =1 <<4 ,
Signature · 6 lines
2.13.3 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 11/14 RDMA/bnxt_re: Delete unsupported modify_port function
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
There is no need to return always zero for function which is not supported. The IB stack treats uninitialized ib_device->functions as not implemented.
--- drivers/infiniband/hw/bnxt_re/ib_verbs.c | 17 ----------------- drivers/infiniband/hw/bnxt_re/ib_verbs.h | 3 --- drivers/infiniband/hw/bnxt_re/main.c | 1 - 3 files changed, 21 deletions(-)
drivers/infiniband/hw/bnxt_re/ib_verbs.c +0 -17
@@ -313,23 +313,6 @@ int bnxt_re_query_port(struct ib_device *ibdev, u8 port_num,return 0 ; } -int bnxt_re_modify_port (struct ib_device *ibdev,u8 port_num, -int port_modify_mask, -struct ib_port_modify *port_modify) -{ -switch (port_modify_mask) { -case IB_PORT_SHUTDOWN : -break ; -case IB_PORT_INIT_TYPE : -break ; -case IB_PORT_RESET_QKEY_CNTR : -break ; -default : -break ; - } -return 0 ; -} -int bnxt_re_get_port_immutable (struct ib_device *ibdev,u8 port_num,struct ib_port_immutable *immutable) {
drivers/infiniband/hw/bnxt_re/ib_verbs.h +0 -3
@@ -141,9 +141,6 @@ int bnxt_re_modify_device(struct ib_device *ibdev, struct ib_device_modify *device_modify);int bnxt_re_query_port (struct ib_device *ibdev,u8 port_num,struct ib_port_attr *port_attr); -int bnxt_re_modify_port (struct ib_device *ibdev,u8 port_num, -int port_modify_mask, -struct ib_port_modify *port_modify);int bnxt_re_get_port_immutable (struct ib_device *ibdev,u8 port_num,struct ib_port_immutable *immutable); int bnxt_re_query_pkey(struct ib_device *ibdev, u8 port_num,
drivers/infiniband/hw/bnxt_re/main.c +0 -1
@@ -474,7 +474,6 @@ static int bnxt_re_register_ib(struct bnxt_re_dev *rdev) ibdev->modify_device = bnxt_re_modify_device; ibdev->query_port = bnxt_re_query_port; - ibdev->modify_port = bnxt_re_modify_port; ibdev->get_port_immutable = bnxt_re_get_port_immutable; ibdev->query_pkey = bnxt_re_query_pkey; ibdev->query_gid = bnxt_re_query_gid;
Signature · 6 lines
2.13.3 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 12/14 RDMA/usnic: Fix remove address space warning
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
Sparse tool complains with the following error:drivers/infiniband/hw/usnic/usnic_ib_main.c:445:16: warning: cast removes
address space of expression
Fix it by doing casting on correct field and convert function helper which sets ifaddr to be void, because there are no users who are interested in returned value.
--- drivers/infiniband/hw/usnic/usnic_fwd.c | 12 ++---------- drivers/infiniband/hw/usnic/usnic_fwd.h | 2 +- drivers/infiniband/hw/usnic/usnic_ib_main.c | 10 ++++++---- 3 files changed, 9 insertions(+), 15 deletions(-)
drivers/infiniband/hw/usnic/usnic_fwd.c +2 -10
@@ -110,20 +110,12 @@ void usnic_fwd_set_mac(struct usnic_fwd_dev *ufdev, char mac[ETH_ALEN])spin_unlock (&ufdev->lock); } -int usnic_fwd_add_ipaddr (struct usnic_fwd_dev *ufdev,__be32 inaddr) +void usnic_fwd_add_ipaddr (struct usnic_fwd_dev *ufdev,__be32 inaddr) { -int status; -spin_lock (&ufdev->lock); -if (ufdev->inaddr ==0 ) { +if (!ufdev->inaddr) ufdev->inaddr = inaddr; - status =0 ; - }else { - status = -EFAULT ; - }spin_unlock (&ufdev->lock); - -return status; }void usnic_fwd_del_ipaddr (struct usnic_fwd_dev *ufdev)
drivers/infiniband/hw/usnic/usnic_fwd.h +1 -1
@@ -75,7 +75,7 @@ struct usnic_fwd_dev *usnic_fwd_dev_alloc(struct pci_dev *pdev);void usnic_fwd_dev_free (struct usnic_fwd_dev *ufdev);void usnic_fwd_set_mac (struct usnic_fwd_dev *ufdev,char mac[ETH_ALEN ]); -int usnic_fwd_add_ipaddr (struct usnic_fwd_dev *ufdev,__be32 inaddr); +void usnic_fwd_add_ipaddr (struct usnic_fwd_dev *ufdev,__be32 inaddr);void usnic_fwd_del_ipaddr (struct usnic_fwd_dev *ufdev);void usnic_fwd_carrier_up (struct usnic_fwd_dev *ufdev);void usnic_fwd_carrier_down (struct usnic_fwd_dev *ufdev);
drivers/infiniband/hw/usnic/usnic_ib_main.c +6 -4
@@ -353,7 +353,7 @@ static void *usnic_ib_device_add(struct pci_dev *dev) {struct usnic_ib_dev *us_ibdev;union ib_gid gid; -struct in_ifaddr *in; +struct in_device *ind;struct net_device *netdev;usnic_dbg ("\n" );
@@ -442,9 +442,11 @@ static void *usnic_ib_device_add(struct pci_dev *dev)if (netif_carrier_ok (us_ibdev->netdev))usnic_fwd_carrier_up (us_ibdev->ufdev); - in = ((struct in_device *)(netdev->ip_ptr))->ifa_list; -if (in !=NULL ) -usnic_fwd_add_ipaddr (us_ibdev->ufdev, in->ifa_address); + ind =in_dev_get (netdev); +if (ind->ifa_list) +usnic_fwd_add_ipaddr (us_ibdev->ufdev, + ind->ifa_list->ifa_address); +in_dev_put (ind);usnic_mac_ip_to_gid (us_ibdev->netdev->perm_addr, us_ibdev->ufdev->inaddr, &gid.raw[0 ]);
Signature · 6 lines
2.13.3 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 13/14 RDMA/mthca: Make explicit conversion to 64bit value
In reply to a message not in the archiveFrom: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The "lg" variable is declared as int so in all places where this variable is used as a shift operand, the output will be int too.
This produces the following smatch warning:drivers/infiniband/hw/mthca/mthca_cmd.c:701 mthca_map_cmd() warn:
should '1 << lg' be a 64 bit type?
Simple declaration of "1" to be "1ULL" will fix the issue.
--- drivers/infiniband/hw/mthca/mthca_cmd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/infiniband/hw/mthca/mthca_cmd.c +1 -1
@@ -698,7 +698,7 @@ static int mthca_map_cmd(struct mthca_dev *dev, u16 op, struct mthca_icm *icm,for (i =0 ; i <mthca_icm_size (&iter) >> lg; ++i) {if (virt !=-1 ) { pages[nent *2 ] =cpu_to_be64 (virt); - virt +=1 << lg; + virt +=1ULL << lg; } pages[nent * 2 + 1] =
Signature · 6 lines
2.13.3 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): 20170731070924.7193-14-leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
Re: [rdma-rc 13/14] RDMA/mthca: Make explicit conversion to 64bit value
Quoted 23 lines · Leon Romanovsky
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The "lg" variable is declared as int so in all places where this variable is used as a shift operand, the output will be int too.
This produces the following smatch warning:
drivers/infiniband/hw/mthca/mthca_cmd.c:701mthca_map_cmd()warn:
should '1 << lg' be a 64 bit type?Simple declaration of "1" to be "1ULL" will fix the issue.
Signed-off-by Leon Romanovsky <[email protected]>--- drivers/infiniband/hw/mthca/mthca_cmd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/infiniband/hw/mthca/mthca_cmd.c +0
-0
drivers/infiniband/hw/mthca/mthca_cmd.c static int mthca_map_cmd(struct mthca_dev *dev, u16 op, struct mthca_icm *icm, · line 704
Quoted 8 lines
drivers/infiniband/hw/mthca/mthca_cmd.c +1
-1@@ -698,7 +698,7 @@ static int mthca_map_cmd(struct mthca_dev *dev, u16 op, struct mthca_icm *icm,for (i =0 ; i <mthca_icm_size (&iter) >> lg; ++i) {if (virt !=-1 ) { pages[nent *2 ] =cpu_to_be64 (virt); - virt +=1 << lg; + virt +=1ULL << lg; } pages[nent * 2 + 1] =
Hello Leon,
Is my analysis correct that lg can be equal to or larger than 32? If so, isn't this patch a bug fix that needs a "Fixes:" tag?
Thanks,
Bart.--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): [email protected]
Re: [rdma-rc 13/14] RDMA/mthca: Make explicit conversion to 64bit value
Quoted 31 lines · Bart Van Assche
Quoted 30 lines · Leon Romanovsky
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The "lg" variable is declared as int so in all places where this variable is used as a shift operand, the output will be int too.
This produces the following smatch warning:
drivers/infiniband/hw/mthca/mthca_cmd.c:701mthca_map_cmd()warn:
should '1 << lg' be a 64 bit type?Simple declaration of "1" to be "1ULL" will fix the issue.
Signed-off-by Leon Romanovsky <[email protected]>--- drivers/infiniband/hw/mthca/mthca_cmd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/infiniband/hw/mthca/mthca_cmd.c +1
-1@@ -698,7 +698,7 @@ static int mthca_map_cmd(struct mthca_dev *dev, u16 op, struct mthca_icm *icm,for (i =0 ; i <mthca_icm_size (&iter) >> lg; ++i) {if (virt !=-1 ) { pages[nent *2 ] =cpu_to_be64 (virt); - virt +=1 << lg; + virt +=1ULL << lg; }
Quoted 6 lines
drivers/infiniband/hw/mthca/mthca_cmd.c static int mthca_map_cmd(struct mthca_dev *dev, u16 op, struct mthca_icm *icm, · line 704
Quoted 1 line
drivers/infiniband/hw/mthca/mthca_cmd.c +0
-0pages[nent * 2 + 1] =Hello Leon,
Is my analysis correct that lg can be equal to or larger than 32? If so, isn't this patch a bug fix that needs a "Fixes:" tag?
This is the fix to commit introduced in pre-git era and I don't have adequate Fixes tag. The fact that no one complained about the problem suggests to me that no one is really important to have it in stable trees.
Tariq and me did the analysis for the same code issue with mlx4 (he is planning to forward the fix to Dave) and our conclusion that it is not bug.
The reason to it in line 689:drivers/infiniband/hw/mthca/mthca_cmd.c
689 lg = ffs(mthca_icm_addr(&iter) | mthca_icm_size(&iter)) - 1;
The "lg" for sure will be less than 31, because ffs returns values from 0 to 31 and minus 1. The situation that ffs will return 0 is unlikely to happen.
Thanks
Quoted 4 lines
Thanks,
Bart.
A message that is not in the archive (sent off-list or not yet received): 20170731161636.GA13672-U/[email protected]
Re: [rdma-rc 13/14] RDMA/mthca: Make explicit conversion to 64bit value
Quoted 47 lines · Leon Romanovsky
Quoted 31 lines · Bart Van Assche
Quoted 30 lines · Leon Romanovsky
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The "lg" variable is declared as int so in all places where this variable is used as a shift operand, the output will be int too.
This produces the following smatch warning:
drivers/infiniband/hw/mthca/mthca_cmd.c:701mthca_map_cmd()warn:
should '1 << lg' be a 64 bit type?Simple declaration of "1" to be "1ULL" will fix the issue.
Signed-off-by Leon Romanovsky <[email protected]>--- drivers/infiniband/hw/mthca/mthca_cmd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/infiniband/hw/mthca/mthca_cmd.c +1
-1@@ -698,7 +698,7 @@ static int mthca_map_cmd(struct mthca_dev *dev, u16 op, struct mthca_icm *icm,for (i =0 ; i <mthca_icm_size (&iter) >> lg; ++i) {if (virt !=-1 ) { pages[nent *2 ] =cpu_to_be64 (virt); - virt +=1 << lg; + virt +=1ULL << lg; }Quoted 6 lines
drivers/infiniband/hw/mthca/mthca_cmd.c static int mthca_map_cmd(struct mthca_dev *dev, u16 op, struct mthca_icm *icm, · line 704
Quoted 1 line
drivers/infiniband/hw/mthca/mthca_cmd.c +0
-0pages[nent * 2 + 1] =Hello Leon,
Is my analysis correct that lg can be equal to or larger than 32? If so, isn't this patch a bug fix that needs a "Fixes:" tag?
This is the fix to commit introduced in pre-git era and I don't have adequate Fixes tag. The fact that no one complained about the problem suggests to me that no one is really important to have it in stable trees.
Tariq and me did the analysis for the same code issue with mlx4 (he is planning to forward the fix to Dave) and our conclusion that it is not bug.
Quoted 5 lines
The reason to it in line 689:drivers/infiniband/hw/mthca/mthca_cmd.c
689 lg =ffs(mthca_icm_addr(&iter) | mthca_icm_size(&iter))- 1;The "lg" for sure will be less than 31, because ffs returns values from 0 to 31 and minus 1. The situation that ffs will return 0 is unlikely to happen.
Hello Leon,
mthca_icm_addr() returns a DMA address (dma_addr_t). Does that mean that that function can return a 64-bit value on 64-bit systems? Shouldn't ffs() be changed into a function that supports 64-bit values?
Thanks,
Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): [email protected]
Re: [rdma-rc 13/14] RDMA/mthca: Make explicit conversion to 64bit value
Quoted 51 lines · Bart Van Assche
Quoted 50 lines · Leon Romanovsky
Quoted 31 lines · Bart Van Assche
Quoted 30 lines · Leon Romanovsky
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The "lg" variable is declared as int so in all places where this variable is used as a shift operand, the output will be int too.
This produces the following smatch warning:
drivers/infiniband/hw/mthca/mthca_cmd.c:701mthca_map_cmd()warn:
should '1 << lg' be a 64 bit type?Simple declaration of "1" to be "1ULL" will fix the issue.
Signed-off-by Leon Romanovsky <[email protected]>--- drivers/infiniband/hw/mthca/mthca_cmd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/infiniband/hw/mthca/mthca_cmd.c +1
-1@@ -698,7 +698,7 @@ static int mthca_map_cmd(struct mthca_dev *dev, u16 op, struct mthca_icm *icm,for (i =0 ; i <mthca_icm_size (&iter) >> lg; ++i) {if (virt !=-1 ) { pages[nent *2 ] =cpu_to_be64 (virt); - virt +=1 << lg; + virt +=1ULL << lg; }Quoted 6 lines
drivers/infiniband/hw/mthca/mthca_cmd.c static int mthca_map_cmd(struct mthca_dev *dev, u16 op, struct mthca_icm *icm, · line 704
Quoted 1 line
drivers/infiniband/hw/mthca/mthca_cmd.c +0
-0pages[nent * 2 + 1] =Hello Leon,
Is my analysis correct that lg can be equal to or larger than 32? If so, isn't this patch a bug fix that needs a "Fixes:" tag?
This is the fix to commit introduced in pre-git era and I don't have adequate Fixes tag. The fact that no one complained about the problem suggests to me that no one is really important to have it in stable trees.
Tariq and me did the analysis for the same code issue with mlx4 (he is planning to forward the fix to Dave) and our conclusion that it is not bug.
The reason to it in line 689:drivers/infiniband/hw/mthca/mthca_cmd.c
689 lg =ffs(mthca_icm_addr(&iter) | mthca_icm_size(&iter))- 1;
Quoted 8 lines
Quoted 2 lines
The "lg" for sure will be less than 31, because ffs returns values from 0 to 31 and minus 1. The situation that ffs will return 0 is unlikely to happen.
Hello Leon,
mthca_icm_addr()returns a DMA address (dma_addr_t). Does that mean that that function can return a 64-bit value on 64-bit systems? Shouldn'tffs()be changed into a function that supports 64-bit values?
Don't dismiss the second part of that ffs: "| mthca_icm_size(&iter)", to overflow ffs, the icm size should be more than 2^32. Is it real scenario?
Quoted 5 lines
Thanks,
Bart.
A message that is not in the archive (sent off-list or not yet received): 20170731165010.GC13672-U/[email protected]
Re: [rdma-rc 13/14] RDMA/mthca: Make explicit conversion to 64bit value
Quoted 55 lines · Leon Romanovsky
Quoted 54 lines · Bart Van Assche
Quoted 52 lines · Leon Romanovsky
Quoted 31 lines · Bart Van Assche
Quoted 30 lines · Leon Romanovsky
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The "lg" variable is declared as int so in all places where this variable is used as a shift operand, the output will be int too.
This produces the following smatch warning:
drivers/infiniband/hw/mthca/mthca_cmd.c:701mthca_map_cmd()warn:
should '1 << lg' be a 64 bit type?Simple declaration of "1" to be "1ULL" will fix the issue.
Signed-off-by Leon Romanovsky <[email protected]>--- drivers/infiniband/hw/mthca/mthca_cmd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/infiniband/hw/mthca/mthca_cmd.c +1
-1@@ -698,7 +698,7 @@ static int mthca_map_cmd(struct mthca_dev *dev, u16 op, struct mthca_icm *icm,for (i =0 ; i <mthca_icm_size (&iter) >> lg; ++i) {if (virt !=-1 ) { pages[nent *2 ] =cpu_to_be64 (virt); - virt +=1 << lg; + virt +=1ULL << lg; }Quoted 6 lines
drivers/infiniband/hw/mthca/mthca_cmd.c static int mthca_map_cmd(struct mthca_dev *dev, u16 op, struct mthca_icm *icm, · line 704
Quoted 1 line
drivers/infiniband/hw/mthca/mthca_cmd.c +0
-0pages[nent * 2 + 1] =Hello Leon,
Is my analysis correct that lg can be equal to or larger than 32? If so, isn't this patch a bug fix that needs a "Fixes:" tag?
This is the fix to commit introduced in pre-git era and I don't have adequate Fixes tag. The fact that no one complained about the problem suggests to me that no one is really important to have it in stable trees.
Tariq and me did the analysis for the same code issue with mlx4 (he is planning to forward the fix to Dave) and our conclusion that it is not bug.
The reason to it in line 689:drivers/infiniband/hw/mthca/mthca_cmd.c
689 lg =ffs(mthca_icm_addr(&iter) | mthca_icm_size(&iter))- 1;The "lg" for sure will be less than 31, because ffs returns values from 0 to 31 and minus 1. The situation that ffs will return 0 is unlikely to happen.
Quoted 8 lines
Quoted 5 lines
Hello Leon,
mthca_icm_addr()returns a DMA address (dma_addr_t). Does that mean that that function can return a 64-bit value on 64-bit systems? Shouldn'tffs()be changed into a function that supports 64-bit values?Don't dismiss the second part of that ffs: "|
mthca_icm_size(&iter)", to overflow ffs, the icm size should be more than 2^32. Is it real scenario?
Hello Leon,
The second part of the logical or expression is not relevant if the first part can be equal to or larger than 2^32. Has the mthca driver ever been tested on a 64-bit system or should it perhaps be marked as not supporting 64-bit systems?
Thanks,
Bart.--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
A message that is not in the archive (sent off-list or not yet received): [email protected]
Re: [rdma-rc 13/14] RDMA/mthca: Make explicit conversion to 64bit value
Quoted 62 lines · Bart Van Assche
Quoted 61 lines · Leon Romanovsky
Quoted 59 lines · Bart Van Assche
Quoted 47 lines · Leon Romanovsky
Quoted 31 lines · Bart Van Assche
Quoted 30 lines · Leon Romanovsky
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The "lg" variable is declared as int so in all places where this variable is used as a shift operand, the output will be int too.
This produces the following smatch warning:
drivers/infiniband/hw/mthca/mthca_cmd.c:701mthca_map_cmd()warn:
should '1 << lg' be a 64 bit type?Simple declaration of "1" to be "1ULL" will fix the issue.
Signed-off-by Leon Romanovsky <[email protected]>--- drivers/infiniband/hw/mthca/mthca_cmd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/infiniband/hw/mthca/mthca_cmd.c +1
-1@@ -698,7 +698,7 @@ static int mthca_map_cmd(struct mthca_dev *dev, u16 op, struct mthca_icm *icm,for (i =0 ; i <mthca_icm_size (&iter) >> lg; ++i) {if (virt !=-1 ) { pages[nent *2 ] =cpu_to_be64 (virt); - virt +=1 << lg; + virt +=1ULL << lg; }Quoted 6 lines
drivers/infiniband/hw/mthca/mthca_cmd.c static int mthca_map_cmd(struct mthca_dev *dev, u16 op, struct mthca_icm *icm, · line 704
Quoted 1 line
drivers/infiniband/hw/mthca/mthca_cmd.c +0
-0pages[nent * 2 + 1] =Hello Leon,
Is my analysis correct that lg can be equal to or larger than 32? If so, isn't this patch a bug fix that needs a "Fixes:" tag?
This is the fix to commit introduced in pre-git era and I don't have adequate Fixes tag. The fact that no one complained about the problem suggests to me that no one is really important to have it in stable trees.
Tariq and me did the analysis for the same code issue with mlx4 (he is planning to forward the fix to Dave) and our conclusion that it is not bug.
Quoted 5 lines
The reason to it in line 689:drivers/infiniband/hw/mthca/mthca_cmd.c
689 lg =ffs(mthca_icm_addr(&iter) | mthca_icm_size(&iter))- 1;The "lg" for sure will be less than 31, because ffs returns values from 0 to 31 and minus 1. The situation that ffs will return 0 is unlikely to happen.
Hello Leon,
mthca_icm_addr()returns a DMA address (dma_addr_t). Does that mean that that function can return a 64-bit value on 64-bit systems? Shouldn'tffs()be changed into a function that supports 64-bit values?
Quoted 6 lines
Quoted 2 lines
Don't dismiss the second part of that ffs: "|
mthca_icm_size(&iter)", to overflow ffs, the icm size should be more than 2^32. Is it real scenario?The second part of the logical or expression is not relevant if the first part can be equal to or larger than 2^32. Has the mthca driver ever been tested on a 64-bit system or should it perhaps be marked as not supporting 64-bit systems?
Sorry - I confused ffs() with another instruction. Please ignore my reply.
Bart.--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [rdma-rc 13/14] RDMA/mthca: Make explicit conversion to 64bit value
In reply to a message not in the archiveQuoted 62 lines · Bart Van Assche
Quoted 61 lines · Leon Romanovsky
Quoted 59 lines · Bart Van Assche
Quoted 47 lines · Leon Romanovsky
Quoted 31 lines · Bart Van Assche
Quoted 30 lines · Leon Romanovsky
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The "lg" variable is declared as int so in all places where this variable is used as a shift operand, the output will be int too.
This produces the following smatch warning:
drivers/infiniband/hw/mthca/mthca_cmd.c:701mthca_map_cmd()warn:
should '1 << lg' be a 64 bit type?Simple declaration of "1" to be "1ULL" will fix the issue.
Signed-off-by Leon Romanovsky <[email protected]>--- drivers/infiniband/hw/mthca/mthca_cmd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/infiniband/hw/mthca/mthca_cmd.c +1
-1@@ -698,7 +698,7 @@ static int mthca_map_cmd(struct mthca_dev *dev, u16 op, struct mthca_icm *icm,for (i =0 ; i <mthca_icm_size (&iter) >> lg; ++i) {if (virt !=-1 ) { pages[nent *2 ] =cpu_to_be64 (virt); - virt +=1 << lg; + virt +=1ULL << lg; }Quoted 6 lines
drivers/infiniband/hw/mthca/mthca_cmd.c static int mthca_map_cmd(struct mthca_dev *dev, u16 op, struct mthca_icm *icm, · line 704
Quoted 1 line
drivers/infiniband/hw/mthca/mthca_cmd.c +0
-0pages[nent * 2 + 1] =Hello Leon,
Is my analysis correct that lg can be equal to or larger than 32? If so, isn't this patch a bug fix that needs a "Fixes:" tag?
This is the fix to commit introduced in pre-git era and I don't have adequate Fixes tag. The fact that no one complained about the problem suggests to me that no one is really important to have it in stable trees.
Tariq and me did the analysis for the same code issue with mlx4 (he is planning to forward the fix to Dave) and our conclusion that it is not bug.
Quoted 5 lines
The reason to it in line 689:drivers/infiniband/hw/mthca/mthca_cmd.c
689 lg =ffs(mthca_icm_addr(&iter) | mthca_icm_size(&iter))- 1;The "lg" for sure will be less than 31, because ffs returns values from 0 to 31 and minus 1. The situation that ffs will return 0 is unlikely to happen.
Hello Leon,
mthca_icm_addr()returns a DMA address (dma_addr_t). Does that mean that that function can return a 64-bit value on 64-bit systems? Shouldn'tffs()be changed into a function that supports 64-bit values?
Quoted 7 lines
Quoted 2 lines
Don't dismiss the second part of that ffs: "|
mthca_icm_size(&iter)", to overflow ffs, the icm size should be more than 2^32. Is it real scenario?Hello Leon,
The second part of the logical or expression is not relevant if the first part can be equal to or larger than 2^32.
Why? ffs(100000000000000000 | 1) == 1. The second part of the expression provides first set bit.
Quoted 3 lines
Has the mthca driver ever been tested on
a 64-bit system or should it perhaps be marked as not supporting 64-bit systems?
I guess that Doug has relevant HW.
Quoted 3 lines
Thanks,
Bart.
Bart Van Assche
Reviewed-byRe: [rdma-rc 13/14] RDMA/mthca: Make explicit conversion to 64bit value
In reply to a message not in the archiveQuoted 23 lines · Leon Romanovsky
From: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/[email protected]>
The "lg" variable is declared as int so in all places where this variable is used as a shift operand, the output will be int too.
This produces the following smatch warning:
drivers/infiniband/hw/mthca/mthca_cmd.c:701mthca_map_cmd()warn:
should '1 << lg' be a 64 bit type?Simple declaration of "1" to be "1ULL" will fix the issue.
Signed-off-by Leon Romanovsky <[email protected]>--- drivers/infiniband/hw/mthca/mthca_cmd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/infiniband/hw/mthca/mthca_cmd.c +0
-0
drivers/infiniband/hw/mthca/mthca_cmd.c static int mthca_map_cmd(struct mthca_dev *dev, u16 op, struct mthca_icm *icm, · line 704
Quoted 8 lines
drivers/infiniband/hw/mthca/mthca_cmd.c +1
-1@@ -698,7 +698,7 @@ static int mthca_map_cmd(struct mthca_dev *dev, u16 op, struct mthca_icm *icm,for (i =0 ; i <mthca_icm_size (&iter) >> lg; ++i) {if (virt !=-1 ) { pages[nent *2 ] =cpu_to_be64 (virt); - virt +=1 << lg; + virt +=1ULL << lg; } pages[nent * 2 + 1] =
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 14/14 IB/uverbs: Fix device cleanup
In reply to a message not in the archiveFrom: Yishai Hadas <yishaih-VPRAkNaXOzVWk0Htik3J/[email protected]>
Uverbs device should be cleaned up only when there is no potential usage of.
As part of ib_uverbs_remove_one which might be triggered upon reset flow the device reference count is decreased as expected and leave the final cleanup to the FDs that were opened.
Current code increases reference count upon opening a new command FD and decreases it upon closing the file. The event FD is opened internally and rely on the command FD by taking on it a reference count.
In case that the command FD was closed and just later the event FD we may ensure that the device resources as of srcu are still alive as they are still in use.
Fixing the above by moving the reference count decreasing to the place where the command FD is really freed instead of doing that when it was just closed.
--- drivers/infiniband/core/uverbs_main.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/infiniband/core/uverbs_main.c +1 -2
@@ -250,6 +250,7 @@ void ib_uverbs_release_file(struct kref *ref)if (atomic_dec_and_test (&file->device->refcount))ib_uverbs_comp_dev (file->device); +kobject_put (&file->device->kobj);kfree (file); }
@@ -906,7 +907,6 @@ static int ib_uverbs_open(struct inode *inode, struct file *filp)static int ib_uverbs_close (struct inode *inode,struct file *filp) {struct ib_uverbs_file *file = filp->private_data; -struct ib_uverbs_device *dev = file->device;mutex_lock (&file->cleanup_mutex);if (file->ucontext) {
@@ -928,7 +928,6 @@ static int ib_uverbs_close(struct inode *inode, struct file *filp) ib_uverbs_release_async_event_file);kref_put (&file->ref, ib_uverbs_release_file); -kobject_put (&dev->kobj);return 0 ; }
Signature · 6 lines
2.13.3 -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 33 lines · Leon Romanovsky
The following changes since commit
5dc78ad1904db597bdb4427f3ead437aae86f54c:IB/ipoib: Notify on modify QP failure only when relevant (2017-07-23 10:52:00 +0300)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/leon/linux-rdma.gittags/rdma-rc-2017-07-31for you to fetch changes up to
38a974d578451dbbde0c40fc2d81fba44027a338:IB/uverbs: Fix device cleanup (2017-07-30 10:43:15 +0300)
----------------------------------------------------------------
RDMA fixes for 4.13This is cleanup series to various static checkers errors along with two major kernel panics fixes from from Parav and Yishai.
----------------------------------------------------------------
Leon Romanovsky (12):
RDMA/(core,ulp): Convert register/unregister event handler to be void RDMA/core: Cleanup device capability enum
RDMA/core: Remove unimplementednode_typesand node transport RDMA/core: DeleteBUG()from unreachable flow
RDMA/core: Refactor get link layer wrapper
RDMA/uverbs: Prevent leak of reserved field
RDMA/mlx4: Avoid potential derefence warning
RDMA/mlx4: Removegfp_maskargument fromacquire_groupcall RDMA/mlx5: Fix existence check for extended address vector RDMA/bnxt_re: Delete unsupportedmodify_portfunction RDMA/usnic: Fix remove address space warning
RDMA/mthca: Make explicit conversion to 64bit value
Quoted 5 lines
Parav Pandit (1):
IB/core: Fix race condition in resolving IP to MACYishai Hadas (1):
IB/uverbs: Fix device cleanup
I'll resubmit as a fixes for -rc the following patches:
RDMA/uverbs: Prevent leak of reserved field RDMA/mlx5: Fix existence check for extended address vector IB/core: Fix race condition in resolving IP to MAC IB/uverbs: Fix device cleanup
And I will move other patches to -next.
Thanks