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 --- 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 Reading /sys/class/net//{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 --- 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