[PATCH net-next,v2 0/5] bonding: do some cleanups in bond driver

[PATCH net-next,v2 0/5] bonding: do some cleanups in bond driver

[Zhengchao Shao]

Do some cleanups in bond driver.

---
v2: use IS_ERR instead of NULL check in patch 2/5, update commit 
    information in patch 3/5, remove inline modifier in patch 4/5
---
Zhengchao Shao (5):
  bonding: add modifier to initialization function and exit function
  bonding: use IS_ERR instead of NULL check in bond_create_debugfs
  bonding: remove redundant NULL check in debugfs function
  bonding: use bond_set_slave_arr to simplify code
  bonding: remove unnecessary NULL check in bond_destructor

 drivers/net/bonding/bond_debugfs.c | 15 +++-----------
 drivers/net/bonding/bond_main.c    | 32 ++++--------------------------
 drivers/net/bonding/bond_sysfs.c   |  4 ++--
 3 files changed, 9 insertions(+), 42 deletions(-)

-- 
2.34.1

[PATCH net-next,v2 1/5] bonding: add modifier to initialization function and exit function

[Zhengchao Shao]

Some functions are only used in initialization and exit functions, so add
the __init/__net_init and __net_exit modifiers to these functions.

Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com>
---
 drivers/net/bonding/bond_debugfs.c | 4 ++--
 drivers/net/bonding/bond_main.c    | 2 +-
 drivers/net/bonding/bond_sysfs.c   | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/bonding/bond_debugfs.c b/drivers/net/bonding/bond_debugfs.c
index 594094526648..94c2f35e3bfc 100644
--- a/drivers/net/bonding/bond_debugfs.c
+++ b/drivers/net/bonding/bond_debugfs.c
@@ -84,7 +84,7 @@ void bond_debug_reregister(struct bonding *bond)
 	}
 }
 
-void bond_create_debugfs(void)
+void __init bond_create_debugfs(void)
 {
 	bonding_debug_root = debugfs_create_dir("bonding", NULL);
 
@@ -113,7 +113,7 @@ void bond_debug_reregister(struct bonding *bond)
 {
 }
 
-void bond_create_debugfs(void)
+void __init bond_create_debugfs(void)
 {
 }
 
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index d26c69d84c1e..6636638f5d97 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -5981,7 +5981,7 @@ static void bond_uninit(struct net_device *bond_dev)
 
 /*------------------------- Module initialization ---------------------------*/
 
-static int bond_check_params(struct bond_params *params)
+static int __init bond_check_params(struct bond_params *params)
 {
 	int arp_validate_value, fail_over_mac_value, primary_reselect_value, i;
 	struct bond_opt_value newval;
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 0bb59da24922..2805135a7205 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -803,7 +803,7 @@ static const struct attribute_group bonding_group = {
 /* Initialize sysfs.  This sets up the bonding_masters file in
  * /sys/class/net.
  */
-int bond_create_sysfs(struct bond_net *bn)
+int __net_init bond_create_sysfs(struct bond_net *bn)
 {
 	int ret;
 
@@ -836,7 +836,7 @@ int bond_create_sysfs(struct bond_net *bn)
 }
 
 /* Remove /sys/class/net/bonding_masters. */
-void bond_destroy_sysfs(struct bond_net *bn)
+void __net_exit bond_destroy_sysfs(struct bond_net *bn)
 {
 	netdev_class_remove_file_ns(&bn->class_attr_bonding_masters, bn->net);
 }
-- 
2.34.1

[PATCH net-next,v2 2/5] bonding: use IS_ERR instead of NULL check in bond_create_debugfs

[Zhengchao Shao]

Because debugfs_create_dir returns ERR_PTR, so IS_ERR should be used to
check whether the directory is successfully created.

Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com>
---
 drivers/net/bonding/bond_debugfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/bonding/bond_debugfs.c b/drivers/net/bonding/bond_debugfs.c
index 94c2f35e3bfc..50e48136c697 100644
--- a/drivers/net/bonding/bond_debugfs.c
+++ b/drivers/net/bonding/bond_debugfs.c
@@ -88,7 +88,7 @@ void __init bond_create_debugfs(void)
 {
 	bonding_debug_root = debugfs_create_dir("bonding", NULL);
 
-	if (!bonding_debug_root)
+	if (IS_ERR(bonding_debug_root))
 		pr_warn("Warning: Cannot create bonding directory in debugfs\n");
 }
 
-- 
2.34.1

[PATCH net-next,v2 3/5] bonding: remove redundant NULL check in debugfs function

[Zhengchao Shao]

Because debugfs_create_dir returns ERR_PTR, so bonding_debug_root will
never be NULL. Remove redundant NULL check for bonding_debug_root in
debugfs function. The later debugfs_create_dir/debugfs_remove_recursive
/debugfs_remove_recursive functions will check the dentry with IS_ERR().

Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com>
---
 drivers/net/bonding/bond_debugfs.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/net/bonding/bond_debugfs.c b/drivers/net/bonding/bond_debugfs.c
index 50e48136c697..b19492a7f6ad 100644
--- a/drivers/net/bonding/bond_debugfs.c
+++ b/drivers/net/bonding/bond_debugfs.c
@@ -49,9 +49,6 @@ DEFINE_SHOW_ATTRIBUTE(bond_debug_rlb_hash);
 
 void bond_debug_register(struct bonding *bond)
 {
-	if (!bonding_debug_root)
-		return;
-
 	bond->debug_dir =
 		debugfs_create_dir(bond->dev->name, bonding_debug_root);
 
@@ -61,9 +58,6 @@ void bond_debug_register(struct bonding *bond)
 
 void bond_debug_unregister(struct bonding *bond)
 {
-	if (!bonding_debug_root)
-		return;
-
 	debugfs_remove_recursive(bond->debug_dir);
 }
 
@@ -71,9 +65,6 @@ void bond_debug_reregister(struct bonding *bond)
 {
 	struct dentry *d;
 
-	if (!bonding_debug_root)
-		return;
-
 	d = debugfs_rename(bonding_debug_root, bond->debug_dir,
 			   bonding_debug_root, bond->dev->name);
 	if (!IS_ERR(d)) {
-- 
2.34.1

[PATCH net-next,v2 4/5] bonding: use bond_set_slave_arr to simplify code

[Zhengchao Shao]

In bond_reset_slave_arr(), values are assigned and memory is released only
when the variables "usable" and "all" are not NULL. But even if the
"usable" and "all" variables are NULL, they can still work, because value
will be checked in kfree_rcu. Therefore, use bond_set_slave_arr() and set
the input parameters "usable_slaves" and "all_slaves" to NULL to simplify
the code in bond_reset_slave_arr(). And the same to bond_uninit().

Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com>
---
 drivers/net/bonding/bond_main.c | 27 ++-------------------------
 1 file changed, 2 insertions(+), 25 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 6636638f5d97..de3ae9c57da0 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -5046,19 +5046,7 @@ static void bond_set_slave_arr(struct bonding *bond,
 
 static void bond_reset_slave_arr(struct bonding *bond)
 {
-	struct bond_up_slave *usable, *all;
-
-	usable = rtnl_dereference(bond->usable_slaves);
-	if (usable) {
-		RCU_INIT_POINTER(bond->usable_slaves, NULL);
-		kfree_rcu(usable, rcu);
-	}
-
-	all = rtnl_dereference(bond->all_slaves);
-	if (all) {
-		RCU_INIT_POINTER(bond->all_slaves, NULL);
-		kfree_rcu(all, rcu);
-	}
+	bond_set_slave_arr(bond, NULL, NULL);
 }
 
 /* Build the usable slaves array in control path for modes that use xmit-hash
@@ -5951,7 +5939,6 @@ void bond_setup(struct net_device *bond_dev)
 static void bond_uninit(struct net_device *bond_dev)
 {
 	struct bonding *bond = netdev_priv(bond_dev);
-	struct bond_up_slave *usable, *all;
 	struct list_head *iter;
 	struct slave *slave;
 
@@ -5962,17 +5949,7 @@ static void bond_uninit(struct net_device *bond_dev)
 		__bond_release_one(bond_dev, slave->dev, true, true);
 	netdev_info(bond_dev, "Released all slaves\n");
 
-	usable = rtnl_dereference(bond->usable_slaves);
-	if (usable) {
-		RCU_INIT_POINTER(bond->usable_slaves, NULL);
-		kfree_rcu(usable, rcu);
-	}
-
-	all = rtnl_dereference(bond->all_slaves);
-	if (all) {
-		RCU_INIT_POINTER(bond->all_slaves, NULL);
-		kfree_rcu(all, rcu);
-	}
+	bond_set_slave_arr(bond, NULL, NULL);
 
 	list_del(&bond->bond_list);
 
-- 
2.34.1

[PATCH net-next,v2 5/5] bonding: remove unnecessary NULL check in bond_destructor

[Zhengchao Shao]

The free_percpu function also could check whether "rr_tx_counter"
parameter is NULL. Therefore, remove NULL check in bond_destructor.

Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com>
---
 drivers/net/bonding/bond_main.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index de3ae9c57da0..f398bec78457 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -5863,8 +5863,7 @@ static void bond_destructor(struct net_device *bond_dev)
 	if (bond->wq)
 		destroy_workqueue(bond->wq);
 
-	if (bond->rr_tx_counter)
-		free_percpu(bond->rr_tx_counter);
+	free_percpu(bond->rr_tx_counter);
 }
 
 void bond_setup(struct net_device *bond_dev)
-- 
2.34.1

Re: [PATCH net-next,v2 0/5] bonding: do some cleanups in bond driver

[Hangbin Liu]

On Thu, Aug 10, 2023 at 09:50:02PM +0800, Zhengchao Shao wrote:
> Do some cleanups in bond driver.

Reviewed-by: Hangbin Liu <liuhangbin@gmail.com>
> 
> ---
> v2: use IS_ERR instead of NULL check in patch 2/5, update commit 
>     information in patch 3/5, remove inline modifier in patch 4/5
> ---
> Zhengchao Shao (5):
>   bonding: add modifier to initialization function and exit function
>   bonding: use IS_ERR instead of NULL check in bond_create_debugfs
>   bonding: remove redundant NULL check in debugfs function
>   bonding: use bond_set_slave_arr to simplify code
>   bonding: remove unnecessary NULL check in bond_destructor
> 
>  drivers/net/bonding/bond_debugfs.c | 15 +++-----------
>  drivers/net/bonding/bond_main.c    | 32 ++++--------------------------
>  drivers/net/bonding/bond_sysfs.c   |  4 ++--
>  3 files changed, 9 insertions(+), 42 deletions(-)
> 
> -- 
> 2.34.1
> 

Re: [PATCH net-next,v2 0/5] bonding: do some cleanups in bond driver

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by David S. Miller <davem@davemloft.net>:

On Thu, 10 Aug 2023 21:50:02 +0800 you wrote:
> Do some cleanups in bond driver.
> 
> ---
> v2: use IS_ERR instead of NULL check in patch 2/5, update commit
>     information in patch 3/5, remove inline modifier in patch 4/5
> ---
> Zhengchao Shao (5):
>   bonding: add modifier to initialization function and exit function
>   bonding: use IS_ERR instead of NULL check in bond_create_debugfs
>   bonding: remove redundant NULL check in debugfs function
>   bonding: use bond_set_slave_arr to simplify code
>   bonding: remove unnecessary NULL check in bond_destructor
> 
> [...]

Here is the summary with links:
  - [net-next,v2,1/5] bonding: add modifier to initialization function and exit function
    https://git.kernel.org/netdev/net-next/c/e08190ef514f
  - [net-next,v2,2/5] bonding: use IS_ERR instead of NULL check in bond_create_debugfs
    https://git.kernel.org/netdev/net-next/c/57647e6fdf17
  - [net-next,v2,3/5] bonding: remove redundant NULL check in debugfs function
    https://git.kernel.org/netdev/net-next/c/cc317ea3d927
  - [net-next,v2,4/5] bonding: use bond_set_slave_arr to simplify code
    https://git.kernel.org/netdev/net-next/c/a8f3f4b44845
  - [net-next,v2,5/5] bonding: remove unnecessary NULL check in bond_destructor
    https://git.kernel.org/netdev/net-next/c/f5370ba3590d

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html