wireguard-linux/drivers/infiniband/core/cm.c, branch jd/unified-crypt-queue

RDMA/cm: Make sure the cm_id is in the IB_CM_IDLE state in destroy

2020-03-17T20:05:54Z

The first switch statement in cm_destroy_id() tries to move the ID to either IB_CM_IDLE or IB_CM_TIMEWAIT. Both states will block concurrent MAD handlers from progressing. Previous patches removed the unreliably lock/unlock sequences in this flow, this patch removes the extra locking steps and adds the missing parts to guarantee that destroy reaches IB_CM_IDLE. There is no point in leaving the ID in the IB_CM_TIMEWAIT state the memory about to be kfreed. Rework things to hold the lock across all the state transitions and directly assert when done that it ended up in IB_CM_IDLE as expected. This was accompanied by a careful audit of all the state transitions here, which generally did end up in IDLE on their success and non-racy paths. Link: https://lore.kernel.org/r/20200310092545.251365-16-leon@kernel.org Signed-off-by: Leon Romanovsky Signed-off-by: Jason Gunthorpe

RDMA/cm: Allow ib_send_cm_sidr_rep() to be done under lock

2020-03-17T20:05:54Z

The first thing ib_send_cm_sidr_rep() does is obtain the lock, so use the usual unlocked wrapper, locked actor pattern here. Get rid of the cm_reject_sidr_req() wrapper so each call site can call the locked or unlocked version as required. This avoids a sketchy lock/unlock sequence (which could allow state to change) during cm_destroy_id(). Link: https://lore.kernel.org/r/20200310092545.251365-15-leon@kernel.org Signed-off-by: Leon Romanovsky Signed-off-by: Jason Gunthorpe

RDMA/cm: Allow ib_send_cm_rej() to be done under lock

2020-03-17T20:05:54Z

The first thing ib_send_cm_rej() does is obtain the lock, so use the usual unlocked wrapper, locked actor pattern here. This avoids a sketchy lock/unlock sequence (which could allow state to change) during cm_destroy_id(). While here simplify some of the logic in the implementation. Link: https://lore.kernel.org/r/20200310092545.251365-14-leon@kernel.org Signed-off-by: Leon Romanovsky Signed-off-by: Jason Gunthorpe

RDMA/cm: Allow ib_send_cm_drep() to be done under lock

2020-03-17T20:05:53Z

The first thing ib_send_cm_drep() does is obtain the lock, so use the usual unlocked wrapper, locked actor pattern here. This avoids a sketchy lock/unlock sequence (which could allow state to change) during cm_destroy_id(). Link: https://lore.kernel.org/r/20200310092545.251365-13-leon@kernel.org Signed-off-by: Leon Romanovsky Signed-off-by: Jason Gunthorpe

RDMA/cm: Allow ib_send_cm_dreq() to be done under lock

2020-03-17T20:05:53Z

The first thing ib_send_cm_dreq() does is obtain the lock, so use the usual unlocked wrapper, locked actor pattern here. This avoids a sketchy lock/unlock sequence (which could allow state to change) during cm_destroy_id(). Link: https://lore.kernel.org/r/20200310092545.251365-12-leon@kernel.org Signed-off-by: Leon Romanovsky Signed-off-by: Jason Gunthorpe

RDMA/cm: Add some lockdep assertions for cm_id_priv->lock

2020-03-17T20:05:53Z

These functions all touch state, so must be called under the lock. Inspection shows this is currently true. Link: https://lore.kernel.org/r/20200310092545.251365-11-leon@kernel.org Signed-off-by: Leon Romanovsky Signed-off-by: Jason Gunthorpe

RDMA/cm: Add missing locking around id.state in cm_dup_req_handler

2020-03-17T20:05:53Z

All accesses to id.state must be done under the spinlock. Fixes: a977049dacde ("[PATCH] IB: Add the kernel CM implementation") Link: https://lore.kernel.org/r/20200310092545.251365-10-leon@kernel.org Signed-off-by: Leon Romanovsky Signed-off-by: Jason Gunthorpe

RDMA/cm: Make it clearer how concurrency works in cm_req_handler()

2020-03-17T20:05:52Z

ib_crate_cm_id() immediately places the id in the xarray, and publishes it into the remote_id and remote_qpn rbtrees. This makes it visible to other threads before it is fully set up. It appears the thinking here was that the states IB_CM_IDLE and IB_CM_REQ_RCVD do not allow any MAD handler or lookup in the remote_id and remote_qpn rbtrees to advance. However, cm_rej_handler() does take an action on IB_CM_REQ_RCVD, which is not really expected by the design. Make the whole thing clearer: - Keep the new cm_id out of the xarray until it is completely set up. This directly prevents MAD handlers and all rbtree lookups from seeing the pointer. - Move all the trivial setup right to the top so it is obviously done before any concurrency begins - Move the mutation of the cm_id_priv out of cm_match_id() and into the caller so the state transition is obvious - Place the manipulation of the work_list at the end, under lock, after the cm_id is placed in the xarray. The work_count cannot change on an ID outside the xarray. - Add some comments Link: https://lore.kernel.org/r/20200310092545.251365-9-leon@kernel.org Signed-off-by: Leon Romanovsky Signed-off-by: Jason Gunthorpe

RDMA/cm: Make it clear that there is no concurrency in cm_sidr_req_handler()

2020-03-17T20:05:52Z

ib_create_cm_id() immediately places the id in the xarray, so it is visible to network traffic. The state is initially set to IB_CM_IDLE and all the MAD handlers will test this state under lock and refuse to advance from IDLE, so adding to the xarray is harmless. Further, the set to IB_CM_SIDR_REQ_RCVD also excludes all MAD handlers. However, the local_id isn't even used for SIDR mode, and there will be no input MADs related to the newly created ID. So, make the whole flow simpler so it can be understood: - Do not put the SIDR cm_id in the xarray. This directly shows that there is no concurrency - Delete the confusing work_count and pending_list manipulations. This mechanism is only used by MAD handlers and timewait, neither of which apply to SIDR. - Add a few comments and rename 'cur_cm_id_priv' to 'listen_cm_id_priv' - Move other loose sets up to immediately after cm_id creation so that the cm_id is fully configured right away. This fixes an oversight where the service_id will not be returned back on a IB_SIDR_UNSUPPORTED reject. Link: https://lore.kernel.org/r/20200310092545.251365-8-leon@kernel.org Signed-off-by: Leon Romanovsky Signed-off-by: Jason Gunthorpe

RDMA/cm: Read id.state under lock when doing pr_debug()

2020-03-17T20:05:52Z

The lock should not be dropped before doing the pr_debug() print as it is accessing data protected by the lock, such as id.state. Fixes: 119bf81793ea ("IB/cm: Add debug prints to ib_cm") Link: https://lore.kernel.org/r/20200310092545.251365-7-leon@kernel.org Signed-off-by: Leon Romanovsky Signed-off-by: Jason Gunthorpe