[PATCH net-next 0/3] bonding: remove RTNL from three sysfs files

[PATCH net-next 0/3] bonding: remove RTNL from three sysfs files

[Eric Dumazet]

First patch might fix a potential deadlock.
sysfs handlers should use rtnl_trylock() instead of rtnl_lock().

Following files can be read without acquiring RTNL :

- /sys/class/net/bonding_masters
- /sys/class/net/<name>/bonding/slaves
- /sys/class/net/<name>/bonding/queue_id

Eric Dumazet (3):
  bonding: no longer use RTNL in bonding_show_bonds()
  bonding: no longer use RTNL in bonding_show_slaves()
  bonding: no longer use RTNL in bonding_show_queue_id()

 drivers/net/bonding/bond_main.c        |  6 +++---
 drivers/net/bonding/bond_netlink.c     |  3 ++-
 drivers/net/bonding/bond_options.c     |  2 +-
 drivers/net/bonding/bond_procfs.c      |  2 +-
 drivers/net/bonding/bond_sysfs.c       | 25 ++++++++++++-------------
 drivers/net/bonding/bond_sysfs_slave.c |  2 +-
 6 files changed, 20 insertions(+), 20 deletions(-)

-- 
2.44.0.478.gd926399ef9-goog

[PATCH net-next 1/3] bonding: no longer use RTNL in bonding_show_bonds()

[Eric Dumazet]

netdev structures are already RCU protected.

Change bond_init() and bond_uninit() to use RCU
enabled list_add_tail_rcu() and list_del_rcu().

Then bonding_show_bonds() can use rcu_read_lock()
while iterating through bn->dev_list.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 drivers/net/bonding/bond_main.c  | 4 ++--
 drivers/net/bonding/bond_sysfs.c | 8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index c9f0415f780ab0a9ecb26424795695eff951421a..08e9bdbf450afdc103931249259c58a08665dc02 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -5933,7 +5933,7 @@ static void bond_uninit(struct net_device *bond_dev)
 
 	bond_set_slave_arr(bond, NULL, NULL);
 
-	list_del(&bond->bond_list);
+	list_del_rcu(&bond->bond_list);
 
 	bond_debug_unregister(bond);
 }
@@ -6347,7 +6347,7 @@ static int bond_init(struct net_device *bond_dev)
 	spin_lock_init(&bond->stats_lock);
 	netdev_lockdep_set_classes(bond_dev);
 
-	list_add_tail(&bond->bond_list, &bn->dev_list);
+	list_add_tail_rcu(&bond->bond_list, &bn->dev_list);
 
 	bond_prepare_sysfs_group(bond);
 
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 2805135a7205ba444ccaf412df33f621f55a729a..9132033f85fb0e33093e97c55f885a997c95cb4a 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -37,12 +37,12 @@ static ssize_t bonding_show_bonds(const struct class *cls,
 {
 	const struct bond_net *bn =
 		container_of_const(attr, struct bond_net, class_attr_bonding_masters);
-	int res = 0;
 	struct bonding *bond;
+	int res = 0;
 
-	rtnl_lock();
+	rcu_read_lock();
 
-	list_for_each_entry(bond, &bn->dev_list, bond_list) {
+	list_for_each_entry_rcu(bond, &bn->dev_list, bond_list) {
 		if (res > (PAGE_SIZE - IFNAMSIZ)) {
 			/* not enough space for another interface name */
 			if ((PAGE_SIZE - res) > 10)
@@ -55,7 +55,7 @@ static ssize_t bonding_show_bonds(const struct class *cls,
 	if (res)
 		buf[res-1] = '\n'; /* eat the leftover space */
 
-	rtnl_unlock();
+	rcu_read_unlock();
 	return res;
 }
 
-- 
2.44.0.478.gd926399ef9-goog

Re: [PATCH net-next 1/3] bonding: no longer use RTNL in bonding_show_bonds()

[Jay Vosburgh]

Eric Dumazet <edumazet@google.com> wrote:

>netdev structures are already RCU protected.
>
>Change bond_init() and bond_uninit() to use RCU
>enabled list_add_tail_rcu() and list_del_rcu().
>
>Then bonding_show_bonds() can use rcu_read_lock()
>while iterating through bn->dev_list.
>
>Signed-off-by: Eric Dumazet <edumazet@google.com>

Acked-by: Jay Vosburgh <jay.vosburgh@canonical.com>


>---
> drivers/net/bonding/bond_main.c  | 4 ++--
> drivers/net/bonding/bond_sysfs.c | 8 ++++----
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index c9f0415f780ab0a9ecb26424795695eff951421a..08e9bdbf450afdc103931249259c58a08665dc02 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -5933,7 +5933,7 @@ static void bond_uninit(struct net_device *bond_dev)
> 
> 	bond_set_slave_arr(bond, NULL, NULL);
> 
>-	list_del(&bond->bond_list);
>+	list_del_rcu(&bond->bond_list);
> 
> 	bond_debug_unregister(bond);
> }
>@@ -6347,7 +6347,7 @@ static int bond_init(struct net_device *bond_dev)
> 	spin_lock_init(&bond->stats_lock);
> 	netdev_lockdep_set_classes(bond_dev);
> 
>-	list_add_tail(&bond->bond_list, &bn->dev_list);
>+	list_add_tail_rcu(&bond->bond_list, &bn->dev_list);
> 
> 	bond_prepare_sysfs_group(bond);
> 
>diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
>index 2805135a7205ba444ccaf412df33f621f55a729a..9132033f85fb0e33093e97c55f885a997c95cb4a 100644
>--- a/drivers/net/bonding/bond_sysfs.c
>+++ b/drivers/net/bonding/bond_sysfs.c
>@@ -37,12 +37,12 @@ static ssize_t bonding_show_bonds(const struct class *cls,
> {
> 	const struct bond_net *bn =
> 		container_of_const(attr, struct bond_net, class_attr_bonding_masters);
>-	int res = 0;
> 	struct bonding *bond;
>+	int res = 0;
> 
>-	rtnl_lock();
>+	rcu_read_lock();
> 
>-	list_for_each_entry(bond, &bn->dev_list, bond_list) {
>+	list_for_each_entry_rcu(bond, &bn->dev_list, bond_list) {
> 		if (res > (PAGE_SIZE - IFNAMSIZ)) {
> 			/* not enough space for another interface name */
> 			if ((PAGE_SIZE - res) > 10)
>@@ -55,7 +55,7 @@ static ssize_t bonding_show_bonds(const struct class *cls,
> 	if (res)
> 		buf[res-1] = '\n'; /* eat the leftover space */
> 
>-	rtnl_unlock();
>+	rcu_read_unlock();
> 	return res;
> }
> 
>-- 
>2.44.0.478.gd926399ef9-goog
>
>

[PATCH net-next 2/3] bonding: no longer use RTNL in bonding_show_slaves()

[Eric Dumazet]

Slave devices are already RCU protected, simply
switch to bond_for_each_slave_rcu(),

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 drivers/net/bonding/bond_sysfs.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 9132033f85fb0e33093e97c55f885a997c95cb4a..75ee7ca369034ef6fa58fc9399b566dd7044fedc 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -170,10 +170,9 @@ static ssize_t bonding_show_slaves(struct device *d,
 	struct slave *slave;
 	int res = 0;
 
-	if (!rtnl_trylock())
-		return restart_syscall();
+	rcu_read_lock();
 
-	bond_for_each_slave(bond, slave, iter) {
+	bond_for_each_slave_rcu(bond, slave, iter) {
 		if (res > (PAGE_SIZE - IFNAMSIZ)) {
 			/* not enough space for another interface name */
 			if ((PAGE_SIZE - res) > 10)
@@ -184,7 +183,7 @@ static ssize_t bonding_show_slaves(struct device *d,
 		res += sysfs_emit_at(buf, res, "%s ", slave->dev->name);
 	}
 
-	rtnl_unlock();
+	rcu_read_unlock();
 
 	if (res)
 		buf[res-1] = '\n'; /* eat the leftover space */
-- 
2.44.0.478.gd926399ef9-goog

Re: [PATCH net-next 2/3] bonding: no longer use RTNL in bonding_show_slaves()

[Jay Vosburgh]

Eric Dumazet <edumazet@google.com> wrote:

>Slave devices are already RCU protected, simply
>switch to bond_for_each_slave_rcu(),
>
>Signed-off-by: Eric Dumazet <edumazet@google.com>

Acked-by: Jay Vosburgh <jay.vosburgh@canonical.com>


>---
> drivers/net/bonding/bond_sysfs.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
>index 9132033f85fb0e33093e97c55f885a997c95cb4a..75ee7ca369034ef6fa58fc9399b566dd7044fedc 100644
>--- a/drivers/net/bonding/bond_sysfs.c
>+++ b/drivers/net/bonding/bond_sysfs.c
>@@ -170,10 +170,9 @@ static ssize_t bonding_show_slaves(struct device *d,
> 	struct slave *slave;
> 	int res = 0;
> 
>-	if (!rtnl_trylock())
>-		return restart_syscall();
>+	rcu_read_lock();
> 
>-	bond_for_each_slave(bond, slave, iter) {
>+	bond_for_each_slave_rcu(bond, slave, iter) {
> 		if (res > (PAGE_SIZE - IFNAMSIZ)) {
> 			/* not enough space for another interface name */
> 			if ((PAGE_SIZE - res) > 10)
>@@ -184,7 +183,7 @@ static ssize_t bonding_show_slaves(struct device *d,
> 		res += sysfs_emit_at(buf, res, "%s ", slave->dev->name);
> 	}
> 
>-	rtnl_unlock();
>+	rcu_read_unlock();
> 
> 	if (res)
> 		buf[res-1] = '\n'; /* eat the leftover space */
>-- 
>2.44.0.478.gd926399ef9-goog
>
>

[PATCH net-next 3/3] bonding: no longer use RTNL in bonding_show_queue_id()

[Eric Dumazet]

Annotate lockless reads of slave->queue_id.

Annotate writes of slave->queue_id.

Switch bonding_show_queue_id() to rcu_read_lock()
and bond_for_each_slave_rcu().

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 drivers/net/bonding/bond_main.c        |  2 +-
 drivers/net/bonding/bond_netlink.c     |  3 ++-
 drivers/net/bonding/bond_options.c     |  2 +-
 drivers/net/bonding/bond_procfs.c      |  2 +-
 drivers/net/bonding/bond_sysfs.c       | 10 +++++-----
 drivers/net/bonding/bond_sysfs_slave.c |  2 +-
 6 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 08e9bdbf450afdc103931249259c58a08665dc02..b3a7d60c3a5ca60be1d9eed184ec1dad593a182b 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -5245,7 +5245,7 @@ static inline int bond_slave_override(struct bonding *bond,
 
 	/* Find out if any slaves have the same mapping as this skb. */
 	bond_for_each_slave_rcu(bond, slave, iter) {
-		if (slave->queue_id == skb_get_queue_mapping(skb)) {
+		if (READ_ONCE(slave->queue_id) == skb_get_queue_mapping(skb)) {
 			if (bond_slave_is_up(slave) &&
 			    slave->link == BOND_LINK_UP) {
 				bond_dev_queue_xmit(bond, skb, slave->dev);
diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
index 29b4c3d1b9b6ff873fe067e80bedf7cb681d18f1..2a6a424806aa603ad8a00ca797e9e22d38bd0435 100644
--- a/drivers/net/bonding/bond_netlink.c
+++ b/drivers/net/bonding/bond_netlink.c
@@ -51,7 +51,8 @@ static int bond_fill_slave_info(struct sk_buff *skb,
 		    slave_dev->addr_len, slave->perm_hwaddr))
 		goto nla_put_failure;
 
-	if (nla_put_u16(skb, IFLA_BOND_SLAVE_QUEUE_ID, slave->queue_id))
+	if (nla_put_u16(skb, IFLA_BOND_SLAVE_QUEUE_ID,
+			READ_ONCE(slave->queue_id)))
 		goto nla_put_failure;
 
 	if (nla_put_s32(skb, IFLA_BOND_SLAVE_PRIO, slave->prio))
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index 4cdbc7e084f4b4cb3b150656aa765531806d8ad9..0cacd7027e352dbf3204d82b7ce1672469a186de 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -1589,7 +1589,7 @@ static int bond_option_queue_id_set(struct bonding *bond,
 		goto err_no_cmd;
 
 	/* Actually set the qids for the slave */
-	update_slave->queue_id = qid;
+	WRITE_ONCE(update_slave->queue_id, qid);
 
 out:
 	return ret;
diff --git a/drivers/net/bonding/bond_procfs.c b/drivers/net/bonding/bond_procfs.c
index 43be458422b3f9448d96383b0fb140837562f446..7edf72ec816abd8b66917bdecd2c93d237629ffa 100644
--- a/drivers/net/bonding/bond_procfs.c
+++ b/drivers/net/bonding/bond_procfs.c
@@ -209,7 +209,7 @@ static void bond_info_show_slave(struct seq_file *seq,
 
 	seq_printf(seq, "Permanent HW addr: %*phC\n",
 		   slave->dev->addr_len, slave->perm_hwaddr);
-	seq_printf(seq, "Slave queue ID: %d\n", slave->queue_id);
+	seq_printf(seq, "Slave queue ID: %d\n", READ_ONCE(slave->queue_id));
 
 	if (BOND_MODE(bond) == BOND_MODE_8023AD) {
 		const struct port *port = &SLAVE_AD_INFO(slave)->port;
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 75ee7ca369034ef6fa58fc9399b566dd7044fedc..1e13bb17051567e2b5d9451ceef47f2cf1a588ec 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -625,10 +625,9 @@ static ssize_t bonding_show_queue_id(struct device *d,
 	struct slave *slave;
 	int res = 0;
 
-	if (!rtnl_trylock())
-		return restart_syscall();
+	rcu_read_lock();
 
-	bond_for_each_slave(bond, slave, iter) {
+	bond_for_each_slave_rcu(bond, slave, iter) {
 		if (res > (PAGE_SIZE - IFNAMSIZ - 6)) {
 			/* not enough space for another interface_name:queue_id pair */
 			if ((PAGE_SIZE - res) > 10)
@@ -637,12 +636,13 @@ static ssize_t bonding_show_queue_id(struct device *d,
 			break;
 		}
 		res += sysfs_emit_at(buf, res, "%s:%d ",
-				     slave->dev->name, slave->queue_id);
+				     slave->dev->name,
+				     READ_ONCE(slave->queue_id));
 	}
 	if (res)
 		buf[res-1] = '\n'; /* eat the leftover space */
 
-	rtnl_unlock();
+	rcu_read_unlock();
 
 	return res;
 }
diff --git a/drivers/net/bonding/bond_sysfs_slave.c b/drivers/net/bonding/bond_sysfs_slave.c
index 313866f2c0e49ac96299ffea307b1613955713ec..36d0e8440b5b94464b3226ce1a04f32361de5aa6 100644
--- a/drivers/net/bonding/bond_sysfs_slave.c
+++ b/drivers/net/bonding/bond_sysfs_slave.c
@@ -53,7 +53,7 @@ static SLAVE_ATTR_RO(perm_hwaddr);
 
 static ssize_t queue_id_show(struct slave *slave, char *buf)
 {
-	return sysfs_emit(buf, "%d\n", slave->queue_id);
+	return sysfs_emit(buf, "%d\n", READ_ONCE(slave->queue_id));
 }
 static SLAVE_ATTR_RO(queue_id);
 
-- 
2.44.0.478.gd926399ef9-goog

Re: [PATCH net-next 3/3] bonding: no longer use RTNL in bonding_show_queue_id()

[Jay Vosburgh]

Eric Dumazet <edumazet@google.com> wrote:

>Annotate lockless reads of slave->queue_id.
>
>Annotate writes of slave->queue_id.
>
>Switch bonding_show_queue_id() to rcu_read_lock()
>and bond_for_each_slave_rcu().

	This is combining two logical changes into one patch, isn't it?
The annotation change isn't part of what's stated in the Subject.

	-J

>Signed-off-by: Eric Dumazet <edumazet@google.com>
>---
> drivers/net/bonding/bond_main.c        |  2 +-
> drivers/net/bonding/bond_netlink.c     |  3 ++-
> drivers/net/bonding/bond_options.c     |  2 +-
> drivers/net/bonding/bond_procfs.c      |  2 +-
> drivers/net/bonding/bond_sysfs.c       | 10 +++++-----
> drivers/net/bonding/bond_sysfs_slave.c |  2 +-
> 6 files changed, 11 insertions(+), 10 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index 08e9bdbf450afdc103931249259c58a08665dc02..b3a7d60c3a5ca60be1d9eed184ec1dad593a182b 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -5245,7 +5245,7 @@ static inline int bond_slave_override(struct bonding *bond,
> 
> 	/* Find out if any slaves have the same mapping as this skb. */
> 	bond_for_each_slave_rcu(bond, slave, iter) {
>-		if (slave->queue_id == skb_get_queue_mapping(skb)) {
>+		if (READ_ONCE(slave->queue_id) == skb_get_queue_mapping(skb)) {
> 			if (bond_slave_is_up(slave) &&
> 			    slave->link == BOND_LINK_UP) {
> 				bond_dev_queue_xmit(bond, skb, slave->dev);
>diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
>index 29b4c3d1b9b6ff873fe067e80bedf7cb681d18f1..2a6a424806aa603ad8a00ca797e9e22d38bd0435 100644
>--- a/drivers/net/bonding/bond_netlink.c
>+++ b/drivers/net/bonding/bond_netlink.c
>@@ -51,7 +51,8 @@ static int bond_fill_slave_info(struct sk_buff *skb,
> 		    slave_dev->addr_len, slave->perm_hwaddr))
> 		goto nla_put_failure;
> 
>-	if (nla_put_u16(skb, IFLA_BOND_SLAVE_QUEUE_ID, slave->queue_id))
>+	if (nla_put_u16(skb, IFLA_BOND_SLAVE_QUEUE_ID,
>+			READ_ONCE(slave->queue_id)))
> 		goto nla_put_failure;
> 
> 	if (nla_put_s32(skb, IFLA_BOND_SLAVE_PRIO, slave->prio))
>diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
>index 4cdbc7e084f4b4cb3b150656aa765531806d8ad9..0cacd7027e352dbf3204d82b7ce1672469a186de 100644
>--- a/drivers/net/bonding/bond_options.c
>+++ b/drivers/net/bonding/bond_options.c
>@@ -1589,7 +1589,7 @@ static int bond_option_queue_id_set(struct bonding *bond,
> 		goto err_no_cmd;
> 
> 	/* Actually set the qids for the slave */
>-	update_slave->queue_id = qid;
>+	WRITE_ONCE(update_slave->queue_id, qid);
> 
> out:
> 	return ret;
>diff --git a/drivers/net/bonding/bond_procfs.c b/drivers/net/bonding/bond_procfs.c
>index 43be458422b3f9448d96383b0fb140837562f446..7edf72ec816abd8b66917bdecd2c93d237629ffa 100644
>--- a/drivers/net/bonding/bond_procfs.c
>+++ b/drivers/net/bonding/bond_procfs.c
>@@ -209,7 +209,7 @@ static void bond_info_show_slave(struct seq_file *seq,
> 
> 	seq_printf(seq, "Permanent HW addr: %*phC\n",
> 		   slave->dev->addr_len, slave->perm_hwaddr);
>-	seq_printf(seq, "Slave queue ID: %d\n", slave->queue_id);
>+	seq_printf(seq, "Slave queue ID: %d\n", READ_ONCE(slave->queue_id));
> 
> 	if (BOND_MODE(bond) == BOND_MODE_8023AD) {
> 		const struct port *port = &SLAVE_AD_INFO(slave)->port;
>diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
>index 75ee7ca369034ef6fa58fc9399b566dd7044fedc..1e13bb17051567e2b5d9451ceef47f2cf1a588ec 100644
>--- a/drivers/net/bonding/bond_sysfs.c
>+++ b/drivers/net/bonding/bond_sysfs.c
>@@ -625,10 +625,9 @@ static ssize_t bonding_show_queue_id(struct device *d,
> 	struct slave *slave;
> 	int res = 0;
> 
>-	if (!rtnl_trylock())
>-		return restart_syscall();
>+	rcu_read_lock();
> 
>-	bond_for_each_slave(bond, slave, iter) {
>+	bond_for_each_slave_rcu(bond, slave, iter) {
> 		if (res > (PAGE_SIZE - IFNAMSIZ - 6)) {
> 			/* not enough space for another interface_name:queue_id pair */
> 			if ((PAGE_SIZE - res) > 10)
>@@ -637,12 +636,13 @@ static ssize_t bonding_show_queue_id(struct device *d,
> 			break;
> 		}
> 		res += sysfs_emit_at(buf, res, "%s:%d ",
>-				     slave->dev->name, slave->queue_id);
>+				     slave->dev->name,
>+				     READ_ONCE(slave->queue_id));
> 	}
> 	if (res)
> 		buf[res-1] = '\n'; /* eat the leftover space */
> 
>-	rtnl_unlock();
>+	rcu_read_unlock();
> 
> 	return res;
> }
>diff --git a/drivers/net/bonding/bond_sysfs_slave.c b/drivers/net/bonding/bond_sysfs_slave.c
>index 313866f2c0e49ac96299ffea307b1613955713ec..36d0e8440b5b94464b3226ce1a04f32361de5aa6 100644
>--- a/drivers/net/bonding/bond_sysfs_slave.c
>+++ b/drivers/net/bonding/bond_sysfs_slave.c
>@@ -53,7 +53,7 @@ static SLAVE_ATTR_RO(perm_hwaddr);
> 
> static ssize_t queue_id_show(struct slave *slave, char *buf)
> {
>-	return sysfs_emit(buf, "%d\n", slave->queue_id);
>+	return sysfs_emit(buf, "%d\n", READ_ONCE(slave->queue_id));
> }
> static SLAVE_ATTR_RO(queue_id);
> 
>-- 
>2.44.0.478.gd926399ef9-goog
>
>

---
	-Jay Vosburgh, jay.vosburgh@canonical.com

Re: [PATCH net-next 3/3] bonding: no longer use RTNL in bonding_show_queue_id()

[Eric Dumazet]

On Tue, Apr 9, 2024 at 10:39 PM Jay Vosburgh <jay.vosburgh@canonical.com> wrote:
>
> Eric Dumazet <edumazet@google.com> wrote:
>
> >Annotate lockless reads of slave->queue_id.
> >
> >Annotate writes of slave->queue_id.
> >
> >Switch bonding_show_queue_id() to rcu_read_lock()
> >and bond_for_each_slave_rcu().
>
>         This is combining two logical changes into one patch, isn't it?
> The annotation change isn't part of what's stated in the Subject.

The annotations are really part of this change, otherwise KCSAN might
find races.

Re: [PATCH net-next 3/3] bonding: no longer use RTNL in bonding_show_queue_id()

[Jay Vosburgh]

Eric Dumazet <edumazet@google.com> wrote:

>On Tue, Apr 9, 2024 at 10:39 PM Jay Vosburgh <jay.vosburgh@canonical.com> wrote:
>>
>> Eric Dumazet <edumazet@google.com> wrote:
>>
>> >Annotate lockless reads of slave->queue_id.
>> >
>> >Annotate writes of slave->queue_id.
>> >
>> >Switch bonding_show_queue_id() to rcu_read_lock()
>> >and bond_for_each_slave_rcu().
>>
>>         This is combining two logical changes into one patch, isn't it?
>> The annotation change isn't part of what's stated in the Subject.
>
>The annotations are really part of this change, otherwise KCSAN might
>find races.

	Works for me.

Acked-by: Jay Vosburgh <jay.vosburgh@canonical.com>

	-J

---
	-Jay Vosburgh, jay.vosburgh@canonical.com

Re: [PATCH net-next 0/3] bonding: remove RTNL from three sysfs files

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Mon,  8 Apr 2024 19:04:34 +0000 you wrote:
> First patch might fix a potential deadlock.
> sysfs handlers should use rtnl_trylock() instead of rtnl_lock().
> 
> Following files can be read without acquiring RTNL :
> 
> - /sys/class/net/bonding_masters
> - /sys/class/net/<name>/bonding/slaves
> - /sys/class/net/<name>/bonding/queue_id
> 
> [...]

Here is the summary with links:
  - [net-next,1/3] bonding: no longer use RTNL in bonding_show_bonds()
    https://git.kernel.org/netdev/net-next/c/6c5d17143fa4
  - [net-next,2/3] bonding: no longer use RTNL in bonding_show_slaves()
    https://git.kernel.org/netdev/net-next/c/d67fed98caa1
  - [net-next,3/3] bonding: no longer use RTNL in bonding_show_queue_id()
    https://git.kernel.org/netdev/net-next/c/662e451d9a62

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