Skip to content

IPoIB: Fix multiple race conditions

Doug Ledford linux-rdma 1 version In review

Get series with b4
b4 am [email protected]

Versions newest first

  1. v1 Latest8 patches 28 repliesIn review

Patches in v1 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/8IPoIB: Consolidate rtnl_lock tasks in workqueuedrivers/infiniband/ulp/ipoib/ipoib_multicast.c+2 -62 replies
2/8IPoIB: Make the carrier_on_task race awaredrivers/infiniband/ulp/ipoib/ipoib_multicast.c+15 -65 replies
3/8IPoIB: fix MCAST_FLAG_BUSY usagedrivers/infiniband/ulp/ipoib/ipoib.h drivers/infiniband/ulp/ipoib/ipoib_multicast.c+101 -575 replies
4/8IPoIB: fix mcast_dev_flush/mcast_restart_task racedrivers/infiniband/ulp/ipoib/ipoib_multicast.c+32 -55 replies
5/8IPoIB: change init sequence orderingdrivers/infiniband/ulp/ipoib/ipoib_main.c+17 -71 reply
6/8IPoIB: Use dedicated workqueues per interfacedrivers/infiniband/ulp/ipoib/ipoib.h drivers/infiniband/ulp/ipoib/ipoib_cm.c drivers/infiniband/ulp/ipoib/ipoib_ib.c +3 more+58 -344 replies
7/8IPoIB: Make ipoib_mcast_stop_thread flush the workqueuedrivers/infiniband/ulp/ipoib/ipoib.h drivers/infiniband/ulp/ipoib/ipoib_ib.c drivers/infiniband/ulp/ipoib/ipoib_multicast.c+12 -151 reply
8/8IPoIB: No longer use flush as a parameterdrivers/infiniband/ulp/ipoib/ipoib.h drivers/infiniband/ulp/ipoib/ipoib_ib.c drivers/infiniband/ulp/ipoib/ipoib_main.c+16 -171 reply

Reviews trailers per patch

7 of 8 patches have a Reviewed-by or Acked-by.

PatchWendy ChengAlex EstrinErez Shitrit
1/8A-b A-b
2/8A-b
3/8A-b
4/8
5/8A-b
6/8A-b
7/8A-b
8/8A-b

Trailers given in replies are filled; those carried in the patches from earlier versions are outlined with dashes. Scroll the table sideways for all 3 reviewers.

Read the v1 thread

Cover letter [PATCH 0/8] IPoIB: Fix multiple race conditions

Locking of multicast joins/leaves in the IPoIB layer have been problematic for a while. There have been recent changes to try and make things better, including these changes:

bea1e22 IPoIB: Fix use-after-free of multicast object a9c8ba5 IPoIB: Fix usage of uninitialized multicast objects

Unfortunately, the following test still fails (miserably) on a plain upstream kernel:

pass=0
ifdown ib0
while true; do
ifconfig ib0 up
ifconfig ib0 down
echo "Pass $pass"
let pass++
done

This usually fails within 10 to 20 passes, although I did have a lucky run make it to 300 or so. If you happen to have a P_Key child interface, it fails even quicker.

In tracking down the rtnl deadlock in the multicast code, I ended up re-designing the flag usage and clean up just the race conditions in the various tasks. Even that wasn't enough to resolve the issue entirely though, as if you ran the test above on multiple interfaces simultaneously, it could still deadlock. So in the end I re-did the workqueue usage too so that we now use a workqueue per device (and that includes P_Key devices have dedicated workqueues) as well as one global workqueue that does nothing but flush tasks. This turns out to be a much more elegant way of handling the workqueues and in fact enabled me to remove all of the klunky passing around of flush parameters to tell various functions not to flush the workqueue if it would deadlock the workqueue we are running from.

Here is my test setup:

2 InfiniBand physical fabrics: ib0 and ib1
2 P_Keys on each fabric: default and 0x8002 on ib0 and 0x8003 on ib1 4 total IPoIB devices that I have named mlx4_ib0, mlx4_ib0.8002,
mlx4_ib1, and mlx4_ib1.8003

In order to test my final patch set, I logged into my test machine on four different virtual terminals, I used ifdown on all of the above interfaces to get things in a consistent state, and then I ran the above loop, one per terminal per interface simultaneously.

It's worth noting here that when you ifconfig a base interface up, it automatically brings up the child interface too, so the ifconfig mlx4_ib0 up is in fact racing with both ups and downs of mlx4_ib0.8002. The same is true for the mlx4_ib1 interface and its child. With my patch set in place, these loops are currently running without a problem and have passed 15,000 up/down cycles per interface.

Doug Ledford (8):
IPoIB: Consolidate rtnl_lock tasks in workqueue
IPoIB: Make the carrier_on_task race aware
IPoIB: fix MCAST_FLAG_BUSY usage
IPoIB: fix mcast_dev_flush/mcast_restart_task race
IPoIB: change init sequence ordering
IPoIB: Use dedicated workqueues per interface
IPoIB: Make ipoib_mcast_stop_thread flush the workqueue
IPoIB: No longer use flush as a parameter

 drivers/infiniband/ulp/ipoib/ipoib.h           |  19 +-
 drivers/infiniband/ulp/ipoib/ipoib_cm.c        |  18 +-
 drivers/infiniband/ulp/ipoib/ipoib_ib.c        |  27 ++-
 drivers/infiniband/ulp/ipoib/ipoib_main.c      |  49 +++--
 drivers/infiniband/ulp/ipoib/ipoib_multicast.c | 239 ++++++++++++++++---------
 drivers/infiniband/ulp/ipoib/ipoib_verbs.c     |  22 ++-
 6 files changed, 240 insertions(+), 134 deletions(-)
Signature · 6 lines
1.9.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