Commit 681dbd710779e8b8d5bae926f6b11f30df70638b
Committed by
David S. Miller
1 parent
c5a8895082
Exists in
master
and in
20 other branches
cnic: Fix locking in start/stop calls.
The slow path ulp_start and ulp_stop calls to the bnx2i driver are sleepable calls and therefore should not be protected using rcu_read_lock. Fix it by using mutex and setting a bit during these calls. cnic_unregister_device() will now wait for the bit to clear before completing the call. Signed-off-by: Michael Chan <mchan@broadcom.com> Reviewed-by: Benjamin Li <benli@broadcom.com> Signed-off-by: David S. Miller <davem@davemloft.net>
Showing 2 changed files with 29 additions and 16 deletions Side-by-side Diff
drivers/net/cnic.c
... | ... | @@ -466,6 +466,7 @@ |
466 | 466 | static int cnic_unregister_device(struct cnic_dev *dev, int ulp_type) |
467 | 467 | { |
468 | 468 | struct cnic_local *cp = dev->cnic_priv; |
469 | + int i = 0; | |
469 | 470 | |
470 | 471 | if (ulp_type >= MAX_CNIC_ULP_TYPE) { |
471 | 472 | printk(KERN_ERR PFX "cnic_unregister_device: Bad type %d\n", |
... | ... | @@ -486,6 +487,15 @@ |
486 | 487 | |
487 | 488 | synchronize_rcu(); |
488 | 489 | |
490 | + while (test_bit(ULP_F_CALL_PENDING, &cp->ulp_flags[ulp_type]) && | |
491 | + i < 20) { | |
492 | + msleep(100); | |
493 | + i++; | |
494 | + } | |
495 | + if (test_bit(ULP_F_CALL_PENDING, &cp->ulp_flags[ulp_type])) | |
496 | + printk(KERN_WARNING PFX "%s: Failed waiting for ULP up call" | |
497 | + " to complete.\n", dev->netdev->name); | |
498 | + | |
489 | 499 | return 0; |
490 | 500 | } |
491 | 501 | EXPORT_SYMBOL(cnic_unregister_driver); |
492 | 502 | |
493 | 503 | |
494 | 504 | |
495 | 505 | |
... | ... | @@ -1076,18 +1086,23 @@ |
1076 | 1086 | if (cp->cnic_uinfo) |
1077 | 1087 | cnic_send_nlmsg(cp, ISCSI_KEVENT_IF_DOWN, NULL); |
1078 | 1088 | |
1079 | - rcu_read_lock(); | |
1080 | 1089 | for (if_type = 0; if_type < MAX_CNIC_ULP_TYPE; if_type++) { |
1081 | 1090 | struct cnic_ulp_ops *ulp_ops; |
1082 | 1091 | |
1083 | - ulp_ops = rcu_dereference(cp->ulp_ops[if_type]); | |
1084 | - if (!ulp_ops) | |
1092 | + mutex_lock(&cnic_lock); | |
1093 | + ulp_ops = cp->ulp_ops[if_type]; | |
1094 | + if (!ulp_ops) { | |
1095 | + mutex_unlock(&cnic_lock); | |
1085 | 1096 | continue; |
1097 | + } | |
1098 | + set_bit(ULP_F_CALL_PENDING, &cp->ulp_flags[if_type]); | |
1099 | + mutex_unlock(&cnic_lock); | |
1086 | 1100 | |
1087 | 1101 | if (test_and_clear_bit(ULP_F_START, &cp->ulp_flags[if_type])) |
1088 | 1102 | ulp_ops->cnic_stop(cp->ulp_handle[if_type]); |
1103 | + | |
1104 | + clear_bit(ULP_F_CALL_PENDING, &cp->ulp_flags[if_type]); | |
1089 | 1105 | } |
1090 | - rcu_read_unlock(); | |
1091 | 1106 | } |
1092 | 1107 | |
1093 | 1108 | static void cnic_ulp_start(struct cnic_dev *dev) |
1094 | 1109 | |
1095 | 1110 | |
1096 | 1111 | |
1097 | 1112 | |
... | ... | @@ -1095,18 +1110,23 @@ |
1095 | 1110 | struct cnic_local *cp = dev->cnic_priv; |
1096 | 1111 | int if_type; |
1097 | 1112 | |
1098 | - rcu_read_lock(); | |
1099 | 1113 | for (if_type = 0; if_type < MAX_CNIC_ULP_TYPE; if_type++) { |
1100 | 1114 | struct cnic_ulp_ops *ulp_ops; |
1101 | 1115 | |
1102 | - ulp_ops = rcu_dereference(cp->ulp_ops[if_type]); | |
1103 | - if (!ulp_ops || !ulp_ops->cnic_start) | |
1116 | + mutex_lock(&cnic_lock); | |
1117 | + ulp_ops = cp->ulp_ops[if_type]; | |
1118 | + if (!ulp_ops || !ulp_ops->cnic_start) { | |
1119 | + mutex_unlock(&cnic_lock); | |
1104 | 1120 | continue; |
1121 | + } | |
1122 | + set_bit(ULP_F_CALL_PENDING, &cp->ulp_flags[if_type]); | |
1123 | + mutex_unlock(&cnic_lock); | |
1105 | 1124 | |
1106 | 1125 | if (!test_and_set_bit(ULP_F_START, &cp->ulp_flags[if_type])) |
1107 | 1126 | ulp_ops->cnic_start(cp->ulp_handle[if_type]); |
1127 | + | |
1128 | + clear_bit(ULP_F_CALL_PENDING, &cp->ulp_flags[if_type]); | |
1108 | 1129 | } |
1109 | - rcu_read_unlock(); | |
1110 | 1130 | } |
1111 | 1131 | |
1112 | 1132 | static int cnic_ctl(void *data, struct cnic_ctl_info *info) |
1113 | 1133 | |
1114 | 1134 | |
1115 | 1135 | |
... | ... | @@ -1116,22 +1136,18 @@ |
1116 | 1136 | switch (info->cmd) { |
1117 | 1137 | case CNIC_CTL_STOP_CMD: |
1118 | 1138 | cnic_hold(dev); |
1119 | - mutex_lock(&cnic_lock); | |
1120 | 1139 | |
1121 | 1140 | cnic_ulp_stop(dev); |
1122 | 1141 | cnic_stop_hw(dev); |
1123 | 1142 | |
1124 | - mutex_unlock(&cnic_lock); | |
1125 | 1143 | cnic_put(dev); |
1126 | 1144 | break; |
1127 | 1145 | case CNIC_CTL_START_CMD: |
1128 | 1146 | cnic_hold(dev); |
1129 | - mutex_lock(&cnic_lock); | |
1130 | 1147 | |
1131 | 1148 | if (!cnic_start_hw(dev)) |
1132 | 1149 | cnic_ulp_start(dev); |
1133 | 1150 | |
1134 | - mutex_unlock(&cnic_lock); | |
1135 | 1151 | cnic_put(dev); |
1136 | 1152 | break; |
1137 | 1153 | default: |
1138 | 1154 | |
... | ... | @@ -2667,10 +2683,8 @@ |
2667 | 2683 | cnic_put(dev); |
2668 | 2684 | goto done; |
2669 | 2685 | } |
2670 | - mutex_lock(&cnic_lock); | |
2671 | 2686 | if (!cnic_start_hw(dev)) |
2672 | 2687 | cnic_ulp_start(dev); |
2673 | - mutex_unlock(&cnic_lock); | |
2674 | 2688 | } |
2675 | 2689 | |
2676 | 2690 | rcu_read_lock(); |
2677 | 2691 | |
... | ... | @@ -2689,10 +2703,8 @@ |
2689 | 2703 | rcu_read_unlock(); |
2690 | 2704 | |
2691 | 2705 | if (event == NETDEV_GOING_DOWN) { |
2692 | - mutex_lock(&cnic_lock); | |
2693 | 2706 | cnic_ulp_stop(dev); |
2694 | 2707 | cnic_stop_hw(dev); |
2695 | - mutex_unlock(&cnic_lock); | |
2696 | 2708 | cnic_unregister_netdev(dev); |
2697 | 2709 | } else if (event == NETDEV_UNREGISTER) { |
2698 | 2710 | write_lock(&cnic_dev_lock); |
drivers/net/cnic.h