RDMA/hns: Simplify the slave uninit logic of RoCE bonding operations

driver inclusion
category: bugfix
bugzilla: https://gitee.com/openeuler/kernel/issues/I6Z4E9

---------------------------------------------------------------

Currently, bond group resource is always associated to a main_hr_dev
whether bond device has been set or not. When setting bond, all slaves,
including the main_hr_dev, will be uninited. Normally, during main_hr_dev
uninit, the bond group resource is also released. However, the bond group
resource should not be released in the case of setting bond.

To avoid releasing bond group resource when setting bond, the code logic
is like:
    1. uninit a not-main_hr_dev slave
    2. register bond device, with the slave in step 1 becoming the new
       main_hr_dev
    3. uninit the original main_hr_dev.
In step 3, the slave is not current main_hr_dev any more, so during its
uninit, bond group resource will not be released. But in general, the
logic is not simple enough to understand.

This patch adds an argument 'bond_cleanup' to
__hns_roce_hw_v2_uninit_instance() in order to indicate whether bond
group resources should be released during RoCE driver uninit. During
driver uninit when setting bond or clearing bond, 'bond_cleanup' is false;
when RoCE driver kernel object is being removed , 'bond_cleanup' is true.

With this change and the previous patch, bonding operations including
bond setting, bond clearing and decreasing slave do not have to concern
about avoiding releasing bond_grp when uniniting hr_dev. The uninit
order is simplified to a more intuitive one: simply uninit all slaves and
then register the bond device.
Signed-off-by: NJunxian Huang <huangjunxian6@hisilicon.com>

drivers/infiniband/hw/hns/hns_roce_bond.c

@@ -178,44 +178,16 @@ static void hns_roce_bond_get_active_slave(struct hns_roce_bond_group *bond_grp)
 	bond_grp->active_slave_map = active_slave_map;
 }
 
-static struct hns_roce_dev
-		*hns_roce_bond_init_client(struct hns_roce_bond_group *bond_grp,
-					   int func_idx)
-{
-	struct hnae3_handle *handle = bond_grp->bond_func_info[func_idx].handle;
-	int ret = hns_roce_hw_v2_init_instance(handle);
-
-	if (ret)
-		return NULL;
-
-	return handle->priv;
-}
-
-static void hns_roce_bond_uninit_client(struct hns_roce_bond_group *bond_grp,
-					int func_idx)
-{
-	struct hnae3_handle *handle = bond_grp->bond_func_info[func_idx].handle;
-
-	hns_roce_hw_v2_uninit_instance(handle, 0);
-}
-
 static void hns_roce_set_bond(struct hns_roce_bond_group *bond_grp)
 {
-	u8 main_func_idx = PCI_FUNC(bond_grp->main_hr_dev->pci_dev->devfn);
-	struct net_device *main_net_dev = bond_grp->main_net_dev;
 	struct hns_roce_dev *hr_dev = NULL;
 	struct net_device *net_dev;
 	int ret;
 	int i;
 
-	/*
-	 * bond_grp will be kfree during uninit_instance of main_hr_dev.
-	 * Thus the main_hr_dev is switched before the uninit_instance
-	 * of the previous main_hr_dev.
-	 */
 	for (i = 0; i < ROCE_BOND_FUNC_MAX; i++) {
 		net_dev = bond_grp->bond_func_info[i].net_dev;
-		if (net_dev && net_dev != main_net_dev)
+		if (net_dev)
 			hns_roce_bond_uninit_client(bond_grp, i);
 	}
 
@@ -223,11 +195,10 @@ static void hns_roce_set_bond(struct hns_roce_bond_group *bond_grp)
 
 	for (i = 0; i < ROCE_BOND_FUNC_MAX; i++) {
 		net_dev = bond_grp->bond_func_info[i].net_dev;
-		if (net_dev && net_dev != main_net_dev) {
+		if (net_dev) {
 			hr_dev = hns_roce_bond_init_client(bond_grp, i);
 			if (hr_dev) {
 				bond_grp->main_hr_dev = hr_dev;
-				bond_grp->main_net_dev = net_dev;
 				break;
 			}
 		}
@@ -235,7 +206,6 @@ static void hns_roce_set_bond(struct hns_roce_bond_group *bond_grp)
 	if (!hr_dev)
 		return;
 
-	hns_roce_bond_uninit_client(bond_grp, main_func_idx);
 	hns_roce_bond_get_active_slave(bond_grp);
 	ret = hns_roce_cmd_bond(bond_grp, HNS_ROCE_SET_BOND);
 	if (ret) {
@@ -250,37 +220,24 @@ static void hns_roce_set_bond(struct hns_roce_bond_group *bond_grp)
 static void hns_roce_clear_bond(struct hns_roce_bond_group *bond_grp)
 {
 	u8 main_func_idx = PCI_FUNC(bond_grp->main_hr_dev->pci_dev->devfn);
-	struct net_device *main_net_dev = bond_grp->main_net_dev;
-	struct hnae3_handle *handle;
 	struct hns_roce_dev *hr_dev;
 	struct net_device *net_dev;
-	int ret;
 	int i;
 
 	bond_grp->bond_state = HNS_ROCE_BOND_NOT_BONDED;
 
+	hns_roce_bond_uninit_client(bond_grp, main_func_idx);
+
 	for (i = 0; i < ROCE_BOND_FUNC_MAX; i++) {
 		net_dev = bond_grp->bond_func_info[i].net_dev;
-		if (net_dev && net_dev != main_net_dev)
-			hns_roce_bond_init_client(bond_grp, i);
+		if (net_dev)
+			hr_dev = hns_roce_bond_init_client(bond_grp, i);
 	}
-
-	ret = hns_roce_cmd_bond(bond_grp, HNS_ROCE_CLEAR_BOND);
-	if (ret)
-		return;
-	handle = bond_grp->bond_func_info[main_func_idx].handle;
-
-	/* bond_grp will be freed in uninit_instance(main_net_dev) */
-	hns_roce_bond_uninit_client(bond_grp, main_func_idx);
-
-	ret = hns_roce_hw_v2_init_instance(handle);
-	if (ret) {
-		ibdev_err(&hr_dev->ib_dev, "failed to clear RoCE bond!\n");
+	if (!hr_dev)
 		return;
-	}
-
-	hr_dev = handle->priv;
 
+	bond_grp->main_hr_dev = hr_dev;
+	hns_roce_cleanup_bond(bond_grp);
 	ibdev_info(&hr_dev->ib_dev, "RoCE clear bond finished!\n");
 }
 
@@ -339,7 +296,7 @@ static void hns_roce_slave_dec(struct hns_roce_bond_group *bond_grp)
 	int i;
 
 	if (dec_slave_map & (1 << main_func_idx)) {
-		hns_roce_cmd_bond(bond_grp, HNS_ROCE_CLEAR_BOND);
+		hns_roce_bond_uninit_client(bond_grp, main_func_idx);
 		for (i = 0; i < ROCE_BOND_FUNC_MAX; i++) {
 			net_dev = bond_grp->bond_func_info[i].net_dev;
 			if (!(dec_slave_map & (1 << i)) && net_dev) {
@@ -347,28 +304,23 @@ static void hns_roce_slave_dec(struct hns_roce_bond_group *bond_grp)
 				hr_dev = hns_roce_bond_init_client(bond_grp, i);
 				if (hr_dev) {
 					bond_grp->main_hr_dev = hr_dev;
-					bond_grp->main_net_dev = net_dev;
 					break;
 				}
 			}
 		}
-		hns_roce_bond_uninit_client(bond_grp, main_func_idx);
 	}
 
 	while (dec_slave_map > 0) {
 		if (dec_slave_map & 1) {
-			hns_roce_bond_init_client(bond_grp, dec_func_idx);
 			bond_grp->bond_func_info[dec_func_idx].net_dev = NULL;
+			hns_roce_bond_init_client(bond_grp, dec_func_idx);
 		}
 		dec_slave_map >>= 1;
 		dec_func_idx++;
 	}
 
 	hns_roce_bond_get_active_slave(bond_grp);
-	if (bond_grp->slave_map_diff & (1 << main_func_idx))
-		ret = hns_roce_cmd_bond(bond_grp, HNS_ROCE_SET_BOND);
-	else
-		ret = hns_roce_cmd_bond(bond_grp, HNS_ROCE_CHANGE_BOND);
+	ret = hns_roce_cmd_bond(bond_grp, HNS_ROCE_CHANGE_BOND);
 	if (ret) {
 		ibdev_err(&bond_grp->main_hr_dev->ib_dev,
 			  "failed to decrease RoCE bond slave!\n");
@@ -502,23 +454,23 @@ static int remove_bond_id(int bus_num, u8 bond_id)
 	return 0;
 }
 
-void hns_roce_cleanup_bond(struct hns_roce_dev *hr_dev,
-			   struct hns_roce_bond_group *bond_grp)
+void hns_roce_cleanup_bond(struct hns_roce_bond_group *bond_grp)
 {
 	int ret;
 
-	unregister_netdevice_notifier(&hr_dev->bond_nb);
+	ret = hns_roce_cmd_bond(bond_grp, HNS_ROCE_CLEAR_BOND);
+	if (ret)
+		ibdev_err(&bond_grp->main_hr_dev->ib_dev,
+			  "failed to clear RoCE bond!\n");
 
-	if (bond_grp && hr_dev == bond_grp->main_hr_dev) {
-		cancel_delayed_work(&bond_grp->bond_work);
-		ret = remove_bond_id(hr_dev->pci_dev->bus->number,
-				     bond_grp->bond_id);
-		if (ret)
-			ibdev_err(&hr_dev->ib_dev,
-				  "failed to remove bond ID %d, ret = %d.\n",
-				  bond_grp->bond_id, ret);
-		kfree(bond_grp);
-	}
+	cancel_delayed_work(&bond_grp->bond_work);
+	ret = remove_bond_id(bond_grp->main_hr_dev->pci_dev->bus->number,
+			     bond_grp->bond_id);
+	if (ret)
+		ibdev_err(&bond_grp->main_hr_dev->ib_dev,
+			  "failed to remove bond ID %d, ret = %d.\n",
+			  bond_grp->bond_id, ret);
+	kfree(bond_grp);
 }
 
 static bool hns_roce_bond_lowerstate_event(struct hns_roce_dev *hr_dev,
@@ -664,7 +616,6 @@ static struct hns_roce_bond_group *hns_roce_alloc_bond_grp(struct hns_roce_dev *
 
 	bond_grp->upper_dev = upper_dev;
 	bond_grp->main_hr_dev = main_hr_dev;
-	bond_grp->main_net_dev = main_hr_dev->iboe.netdevs[0];
 	bond_grp->bond_ready = false;
 	bond_grp->bond_state = HNS_ROCE_BOND_NOT_BONDED;
 

drivers/infiniband/hw/hns/hns_roce_bond.h

@@ -52,7 +52,6 @@ struct hns_roce_func_info {
 
 struct hns_roce_bond_group {
 	struct net_device *upper_dev;
-	struct net_device *main_net_dev;
 	struct hns_roce_dev *main_hr_dev;
 	u8 slave_num;
 	u8 active_slave_num;
@@ -80,8 +79,7 @@ struct hns_roce_die_info {
 int hns_roce_bond_init(struct hns_roce_dev *hr_dev);
 int hns_roce_bond_event(struct notifier_block *self,
 			unsigned long event, void *ptr);
-void hns_roce_cleanup_bond(struct hns_roce_dev *hr_dev,
-			   struct hns_roce_bond_group *bond_grp);
+void hns_roce_cleanup_bond(struct hns_roce_bond_group *bond_grp);
 bool hns_roce_bond_is_active(struct hns_roce_dev *hr_dev);
 struct net_device *hns_roce_get_bond_netdev(struct hns_roce_dev *hr_dev);
 struct hns_roce_bond_group *hns_roce_get_bond_grp(struct hns_roce_dev *hr_dev);

drivers/infiniband/hw/hns/hns_roce_device.h

@@ -1400,7 +1400,7 @@ void hns_roce_srq_event(struct hns_roce_dev *hr_dev, u32 srqn, int event_type);
 u8 hns_get_gid_index(struct hns_roce_dev *hr_dev, u8 port, int gid_index);
 void hns_roce_handle_device_err(struct hns_roce_dev *hr_dev);
 int hns_roce_init(struct hns_roce_dev *hr_dev);
-void hns_roce_exit(struct hns_roce_dev *hr_dev);
+void hns_roce_exit(struct hns_roce_dev *hr_dev, bool bond_cleanup);
 int hns_roce_fill_res_cq_entry(struct sk_buff *msg, struct ib_cq *ib_cq);
 int hns_roce_fill_res_cq_entry_raw(struct sk_buff *msg, struct ib_cq *ib_cq);
 int hns_roce_fill_res_qp_entry(struct sk_buff *msg, struct ib_qp *ib_qp);

drivers/infiniband/hw/hns/hns_roce_hw_v2.c

@@ -7341,7 +7341,7 @@ static int __hns_roce_hw_v2_init_instance(struct hnae3_handle *handle)
 	return 0;
 
 error_failed_roce_init:
-	hns_roce_exit(hr_dev);
+	hns_roce_exit(hr_dev, true);
 
 error_failed_cfg:
 	kfree(hr_dev->priv);
@@ -7353,7 +7353,7 @@ static int __hns_roce_hw_v2_init_instance(struct hnae3_handle *handle)
 }
 
 static void __hns_roce_hw_v2_uninit_instance(struct hnae3_handle *handle,
-					   bool reset)
+					   bool reset, bool bond_cleanup)
 {
 	struct hns_roce_dev *hr_dev = handle->priv;
 
@@ -7368,12 +7368,12 @@ static void __hns_roce_hw_v2_uninit_instance(struct hnae3_handle *handle,
 	if (hr_dev->pci_dev->revision == PCI_REVISION_ID_HIP08)
 		free_mr_exit(hr_dev);
 
-	hns_roce_exit(hr_dev);
+	hns_roce_exit(hr_dev, bond_cleanup);
 	kfree(hr_dev->priv);
 	ib_dealloc_device(&hr_dev->ib_dev);
 }
 
-int hns_roce_hw_v2_init_instance(struct hnae3_handle *handle)
+static int hns_roce_hw_v2_init_instance(struct hnae3_handle *handle)
 {
 	const struct hnae3_ae_ops *ops = handle->ae_algo->ops;
 	const struct pci_device_id *id;
@@ -7420,14 +7420,45 @@ int hns_roce_hw_v2_init_instance(struct hnae3_handle *handle)
 	return -EBUSY;
 }
 
-void hns_roce_hw_v2_uninit_instance(struct hnae3_handle *handle, bool reset)
+static void hns_roce_hw_v2_uninit_instance(struct hnae3_handle *handle,
+					   bool reset)
+{
+	if (handle->rinfo.instance_state != HNS_ROCE_STATE_INITED)
+		return;
+
+	handle->rinfo.instance_state = HNS_ROCE_STATE_UNINIT;
+
+	__hns_roce_hw_v2_uninit_instance(handle, reset, true);
+
+	handle->rinfo.instance_state = HNS_ROCE_STATE_NON_INIT;
+}
+
+struct hns_roce_dev
+	*hns_roce_bond_init_client(struct hns_roce_bond_group *bond_grp,
+				   int func_idx)
 {
+	struct hnae3_handle *handle;
+	int ret;
+
+	handle = bond_grp->bond_func_info[func_idx].handle;
+	ret = hns_roce_hw_v2_init_instance(handle);
+	if (ret)
+		return NULL;
+
+	return handle->priv;
+}
+
+void hns_roce_bond_uninit_client(struct hns_roce_bond_group *bond_grp,
+				 int func_idx)
+{
+	struct hnae3_handle *handle = bond_grp->bond_func_info[func_idx].handle;
+
 	if (handle->rinfo.instance_state != HNS_ROCE_STATE_INITED)
 		return;
 
 	handle->rinfo.instance_state = HNS_ROCE_STATE_UNINIT;
 
-	__hns_roce_hw_v2_uninit_instance(handle, reset);
+	__hns_roce_hw_v2_uninit_instance(handle, false, false);
 
 	handle->rinfo.instance_state = HNS_ROCE_STATE_NON_INIT;
 }
@@ -7507,7 +7538,7 @@ static int hns_roce_hw_v2_reset_notify_uninit(struct hnae3_handle *handle)
 	handle->rinfo.reset_state = HNS_ROCE_STATE_RST_UNINIT;
 	dev_info(&handle->pdev->dev, "In reset process RoCE client uninit.\n");
 	msleep(HNS_ROCE_V2_HW_RST_UNINT_DELAY);
-	__hns_roce_hw_v2_uninit_instance(handle, false);
+	__hns_roce_hw_v2_uninit_instance(handle, false, false);
 
 	return 0;
 }

drivers/infiniband/hw/hns/hns_roce_hw_v2.h

@@ -1594,9 +1594,11 @@ struct hns_roce_bond_info {
 	__le32 hash_policy;
 };
 
-int hns_roce_hw_v2_init_instance(struct hnae3_handle *handle);
-void hns_roce_hw_v2_uninit_instance(struct hnae3_handle *handle, bool reset);
-
+struct hns_roce_dev
+	*hns_roce_bond_init_client(struct hns_roce_bond_group *bond_grp,
+				   int func_idx);
+void hns_roce_bond_uninit_client(struct hns_roce_bond_group *bond_grp,
+				 int func_idx);
 int hns_roce_v2_destroy_qp(struct ib_qp *ibqp, struct ib_udata *udata);
 
 int hns_roce_v2_destroy_qp_common(struct hns_roce_dev *hr_dev,

drivers/infiniband/hw/hns/hns_roce_main.c

@@ -822,15 +822,17 @@ static int hns_roce_get_hw_stats(struct ib_device *device,
 	return hw_counters + HNS_ROCE_DFX_CNT_TOTAL;
 }
 
-static void hns_roce_unregister_device(struct hns_roce_dev *hr_dev)
+static void hns_roce_unregister_device(struct hns_roce_dev *hr_dev,
+				       bool bond_cleanup)
 {
 	struct hns_roce_ib_iboe *iboe = &hr_dev->iboe;
 	struct hns_roce_bond_group *bond_grp;
 
 	if (hr_dev->caps.flags & HNS_ROCE_CAP_FLAG_BOND) {
+		unregister_netdevice_notifier(&hr_dev->bond_nb);
 		bond_grp = hns_roce_get_bond_grp(hr_dev);
-		if (bond_grp)
-			hns_roce_cleanup_bond(hr_dev, bond_grp);
+		if (bond_grp && bond_cleanup)
+			hns_roce_cleanup_bond(bond_grp);
 	}
 
 	hr_dev->active = false;
@@ -1448,10 +1450,10 @@ int hns_roce_init(struct hns_roce_dev *hr_dev)
 	return ret;
 }
 
-void hns_roce_exit(struct hns_roce_dev *hr_dev)
+void hns_roce_exit(struct hns_roce_dev *hr_dev, bool bond_cleanup)
 {
 	hns_roce_unregister_sysfs(hr_dev);
-	hns_roce_unregister_device(hr_dev);
+	hns_roce_unregister_device(hr_dev, bond_cleanup);
 	hns_roce_unregister_debugfs(hr_dev);
 
 	if (hr_dev->hw->hw_exit)