[PATCH net-next v2 0/2] net: sysfs: use ops lock for speed and duplex

[PATCH net-next v2 0/2] net: sysfs: use ops lock for speed and duplex

[Wang Zhan]

Reading speed and duplex from sysfs currently holds RTNL across the
get_link_ksettings callback. This unnecessarily serializes monitoring
reads with unrelated rtnetlink operations.

On CPU-throttled hosts, a periodic reader such as node-exporter can hold
RTNL for hundreds of milliseconds while an mlx5 callback runs, delaying
unrelated rtnetlink operations.

Factor the shared link settings read first, then change only the helper's
locking. Legacy devices and callbacks which request
ETHTOOL_OP_NEEDS_RTNL_LINKSETTINGS retain the existing RTNL path.

Changes in v2:
- Split the common-code refactoring from the locking change.
- Keep speed_show() and duplex_show() focused on formatting the result.

v1: https://lore.kernel.org/netdev/20260831080623.1064001-1-wang.zhan@smartx.com/

Checks:
- checkpatch.pl --strict
- git diff --check
- x86_64 defconfig W=1 net/core/net-sysfs.o

Wang Zhan (2):
  net: sysfs: factor out link settings read
  net: sysfs: use ops lock for speed and duplex

 net/core/net-sysfs.c | 112 ++++++++++++++++++++++++-------------------
 1 file changed, 63 insertions(+), 49 deletions(-)


base-commit: b35d3d2fae3058265ba937544a3895cedce18d08
-- 
2.47.3

[PATCH net-next v2 1/2] net: sysfs: factor out link settings read

[Wang Zhan]

speed_show() and duplex_show() duplicate device validation, RTNL
locking and the link settings query. Move this common work to
sysfs_get_link_ksettings() so later locking changes stay in one place.

RTNL is released before calling sysfs_emit(). The lock only protects the
link settings query; formatting uses the local cmd copy, so the sysfs
output and error handling remain unchanged.

No functional changes.

Assisted-by: LLM
Signed-off-by: Wang Zhan <wang.zhan@smartx.com>
---
 net/core/net-sysfs.c | 97 ++++++++++++++++++++++----------------------
 1 file changed, 48 insertions(+), 49 deletions(-)

diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 352173df75785..7bb8bbc1f71ea 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -118,6 +118,34 @@ static int sysfs_rtnl_lock(struct kobject *kobj, struct attribute *attr,
 	return ret;
 }
 
+static int sysfs_get_link_ksettings(struct device *dev,
+				    struct device_attribute *attr,
+				    struct ethtool_link_ksettings *cmd)
+{
+	struct net_device *netdev = to_net_dev(dev);
+	int ret;
+
+	/*
+	 * The check is also done in __ethtool_get_link_ksettings; this helps
+	 * returning early without hitting the locking section below.
+	 */
+	if (!netdev->ethtool_ops->get_link_ksettings)
+		return -EINVAL;
+
+	ret = sysfs_rtnl_lock(&dev->kobj, &attr->attr, netdev);
+	if (ret)
+		return ret;
+
+	ret = -EINVAL;
+	if (netif_running(netdev)) {
+		if (!__ethtool_get_link_ksettings(netdev, cmd))
+			ret = 0;
+	}
+
+	rtnl_unlock();
+	return ret;
+}
+
 /* use same locking rules as GIF* ioctl's */
 static ssize_t netdev_show(const struct device *dev,
 			   struct device_attribute *attr, char *buf,
@@ -332,70 +360,41 @@ static DEVICE_ATTR_RW(carrier);
 static ssize_t speed_show(struct device *dev,
 			  struct device_attribute *attr, char *buf)
 {
-	struct net_device *netdev = to_net_dev(dev);
-	int ret = -EINVAL;
-
-	/* The check is also done in __ethtool_get_link_ksettings; this helps
-	 * returning early without hitting the locking section below.
-	 */
-	if (!netdev->ethtool_ops->get_link_ksettings)
-		return ret;
+	struct ethtool_link_ksettings cmd;
+	int ret;
 
-	ret = sysfs_rtnl_lock(&dev->kobj, &attr->attr, netdev);
+	ret = sysfs_get_link_ksettings(dev, attr, &cmd);
 	if (ret)
 		return ret;
 
-	ret = -EINVAL;
-	if (netif_running(netdev)) {
-		struct ethtool_link_ksettings cmd;
-
-		if (!__ethtool_get_link_ksettings(netdev, &cmd))
-			ret = sysfs_emit(buf, fmt_dec, cmd.base.speed);
-	}
-	rtnl_unlock();
-	return ret;
+	return sysfs_emit(buf, fmt_dec, cmd.base.speed);
 }
 static DEVICE_ATTR_RO(speed);
 
 static ssize_t duplex_show(struct device *dev,
 			   struct device_attribute *attr, char *buf)
 {
-	struct net_device *netdev = to_net_dev(dev);
-	int ret = -EINVAL;
-
-	/* The check is also done in __ethtool_get_link_ksettings; this helps
-	 * returning early without hitting the locking section below.
-	 */
-	if (!netdev->ethtool_ops->get_link_ksettings)
-		return ret;
+	struct ethtool_link_ksettings cmd;
+	const char *duplex;
+	int ret;
 
-	ret = sysfs_rtnl_lock(&dev->kobj, &attr->attr, netdev);
+	ret = sysfs_get_link_ksettings(dev, attr, &cmd);
 	if (ret)
 		return ret;
 
-	ret = -EINVAL;
-	if (netif_running(netdev)) {
-		struct ethtool_link_ksettings cmd;
-
-		if (!__ethtool_get_link_ksettings(netdev, &cmd)) {
-			const char *duplex;
-
-			switch (cmd.base.duplex) {
-			case DUPLEX_HALF:
-				duplex = "half";
-				break;
-			case DUPLEX_FULL:
-				duplex = "full";
-				break;
-			default:
-				duplex = "unknown";
-				break;
-			}
-			ret = sysfs_emit(buf, "%s\n", duplex);
-		}
+	switch (cmd.base.duplex) {
+	case DUPLEX_HALF:
+		duplex = "half";
+		break;
+	case DUPLEX_FULL:
+		duplex = "full";
+		break;
+	default:
+		duplex = "unknown";
+		break;
 	}
-	rtnl_unlock();
-	return ret;
+
+	return sysfs_emit(buf, "%s\n", duplex);
 }
 static DEVICE_ATTR_RO(duplex);
 
-- 
2.47.3

[PATCH net-next v2 2/2] net: sysfs: use ops lock for speed and duplex

[Wang Zhan]

Reading /sys/class/net/<dev>/{speed,duplex} takes rtnl_lock() even for
ops-locked devices whose get_link_ksettings callback does not require it.
This unnecessarily serializes monitoring reads with unrelated rtnetlink
operations.

On CPU-throttled hosts, a periodic reader such as node-exporter can hold
RTNL for hundreds of milliseconds while an mlx5 callback runs, delaying
unrelated rtnetlink operations.

Use the netdev instance lock for these devices. Retain sysfs_rtnl_lock()
for legacy devices and callbacks that request
ETHTOOL_OP_NEEDS_RTNL_LINKSETTINGS.

Preserve the existing speed and duplex sysfs ABI, including -EINVAL for
devices that are down or callbacks that fail.

Assisted-by: LLM
Signed-off-by: Wang Zhan <wang.zhan@smartx.com>
---
 net/core/net-sysfs.c | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 7bb8bbc1f71ea..fa790e4425704 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -123,26 +123,41 @@ static int sysfs_get_link_ksettings(struct device *dev,
 				    struct ethtool_link_ksettings *cmd)
 {
 	struct net_device *netdev = to_net_dev(dev);
+	bool need_rtnl;
 	int ret;
 
 	/*
-	 * The check is also done in __ethtool_get_link_ksettings; this helps
+	 * The check is also done in netif_get_link_ksettings; this helps
 	 * returning early without hitting the locking section below.
 	 */
 	if (!netdev->ethtool_ops->get_link_ksettings)
 		return -EINVAL;
 
-	ret = sysfs_rtnl_lock(&dev->kobj, &attr->attr, netdev);
-	if (ret)
-		return ret;
+	need_rtnl = !netdev_need_ops_lock(netdev) ||
+		    (netdev->ethtool_ops->op_needs_rtnl &
+		     ETHTOOL_OP_NEEDS_RTNL_LINKSETTINGS);
+	if (need_rtnl) {
+		ret = sysfs_rtnl_lock(&dev->kobj, &attr->attr, netdev);
+		if (ret)
+			return ret;
+	}
+	netdev_lock_ops(netdev);
+
+	if (!dev_isalive(netdev)) {
+		ret = -ENODEV;
+		goto unlock;
+	}
 
 	ret = -EINVAL;
 	if (netif_running(netdev)) {
-		if (!__ethtool_get_link_ksettings(netdev, cmd))
+		if (!netif_get_link_ksettings(netdev, cmd))
 			ret = 0;
 	}
 
-	rtnl_unlock();
+unlock:
+	netdev_unlock_ops(netdev);
+	if (need_rtnl)
+		rtnl_unlock();
 	return ret;
 }
 
-- 
2.47.3