* [PATCH net-next v2 1/2] net: sysfs: factor out link settings read
2026-09-03 13:33 [PATCH net-next v2 0/2] net: sysfs: use ops lock for speed and duplex Wang Zhan
@ 2026-09-03 13:33 ` Wang Zhan
2026-09-03 13:33 ` [PATCH net-next v2 2/2] net: sysfs: use ops lock for speed and duplex Wang Zhan
1 sibling, 0 replies; 3+ messages in thread
From: Wang Zhan @ 2026-09-03 13:33 UTC (permalink / raw)
To: netdev; +Cc: davem, edumazet, kuba, pabeni, horms, keyong.sun, 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
^ permalink raw reply related [flat|nested] 3+ messages in thread* [PATCH net-next v2 2/2] net: sysfs: use ops lock for speed and duplex
2026-09-03 13:33 [PATCH net-next v2 0/2] net: sysfs: use ops lock for speed and duplex Wang Zhan
2026-09-03 13:33 ` [PATCH net-next v2 1/2] net: sysfs: factor out link settings read Wang Zhan
@ 2026-09-03 13:33 ` Wang Zhan
1 sibling, 0 replies; 3+ messages in thread
From: Wang Zhan @ 2026-09-03 13:33 UTC (permalink / raw)
To: netdev; +Cc: davem, edumazet, kuba, pabeni, horms, keyong.sun, 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
^ permalink raw reply related [flat|nested] 3+ messages in thread