Skip to content

Commit 8524501

Browse files
atenartgregkh
authored andcommitted
net-sysfs: try not to restart the syscall if it will fail eventually
[ Upstream commit 146e5e7 ] Due to deadlocks in the networking subsystem spotted 12 years ago[1], a workaround was put in place[2] to avoid taking the rtnl lock when it was not available and restarting the syscall (back to VFS, letting userspace spin). The following construction is found a lot in the net sysfs and sysctl code: if (!rtnl_trylock()) return restart_syscall(); This can be problematic when multiple userspace threads use such interfaces in a short period, making them to spin a lot. This happens for example when adding and moving virtual interfaces: userspace programs listening on events, such as systemd-udevd and NetworkManager, do trigger actions reading files in sysfs. It gets worse when a lot of virtual interfaces are created concurrently, say when creating containers at boot time. Returning early without hitting the above pattern when the syscall will fail eventually does make things better. While it is not a fix for the issue, it does ease things. [1] https://lore.kernel.org/netdev/49A4D5D5.5090602@trash.net/ https://lore.kernel.org/netdev/m14oyhis31.fsf@fess.ebiederm.org/ and https://lore.kernel.org/netdev/20090226084924.16cb3e08@nehalam/ [2] Rightfully, those deadlocks are *hard* to solve. Signed-off-by: Antoine Tenart <atenart@kernel.org> Reviewed-by: Paolo Abeni <pabeni@redhat.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: Sasha Levin <sashal@kernel.org>
1 parent b94e5bd commit 8524501

1 file changed

Lines changed: 55 additions & 0 deletions

File tree

net/core/net-sysfs.c

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,14 @@ static int change_carrier(struct net_device *dev, unsigned long new_carrier)
175175
static ssize_t carrier_store(struct device *dev, struct device_attribute *attr,
176176
const char *buf, size_t len)
177177
{
178+
struct net_device *netdev = to_net_dev(dev);
179+
180+
/* The check is also done in change_carrier; this helps returning early
181+
* without hitting the trylock/restart in netdev_store.
182+
*/
183+
if (!netdev->netdev_ops->ndo_change_carrier)
184+
return -EOPNOTSUPP;
185+
178186
return netdev_store(dev, attr, buf, len, change_carrier);
179187
}
180188

@@ -196,6 +204,12 @@ static ssize_t speed_show(struct device *dev,
196204
struct net_device *netdev = to_net_dev(dev);
197205
int ret = -EINVAL;
198206

207+
/* The check is also done in __ethtool_get_link_ksettings; this helps
208+
* returning early without hitting the trylock/restart below.
209+
*/
210+
if (!netdev->ethtool_ops->get_link_ksettings)
211+
return ret;
212+
199213
if (!rtnl_trylock())
200214
return restart_syscall();
201215

@@ -216,6 +230,12 @@ static ssize_t duplex_show(struct device *dev,
216230
struct net_device *netdev = to_net_dev(dev);
217231
int ret = -EINVAL;
218232

233+
/* The check is also done in __ethtool_get_link_ksettings; this helps
234+
* returning early without hitting the trylock/restart below.
235+
*/
236+
if (!netdev->ethtool_ops->get_link_ksettings)
237+
return ret;
238+
219239
if (!rtnl_trylock())
220240
return restart_syscall();
221241

@@ -468,6 +488,14 @@ static ssize_t proto_down_store(struct device *dev,
468488
struct device_attribute *attr,
469489
const char *buf, size_t len)
470490
{
491+
struct net_device *netdev = to_net_dev(dev);
492+
493+
/* The check is also done in change_proto_down; this helps returning
494+
* early without hitting the trylock/restart in netdev_store.
495+
*/
496+
if (!netdev->netdev_ops->ndo_change_proto_down)
497+
return -EOPNOTSUPP;
498+
471499
return netdev_store(dev, attr, buf, len, change_proto_down);
472500
}
473501
NETDEVICE_SHOW_RW(proto_down, fmt_dec);
@@ -478,6 +506,12 @@ static ssize_t phys_port_id_show(struct device *dev,
478506
struct net_device *netdev = to_net_dev(dev);
479507
ssize_t ret = -EINVAL;
480508

509+
/* The check is also done in dev_get_phys_port_id; this helps returning
510+
* early without hitting the trylock/restart below.
511+
*/
512+
if (!netdev->netdev_ops->ndo_get_phys_port_id)
513+
return -EOPNOTSUPP;
514+
481515
if (!rtnl_trylock())
482516
return restart_syscall();
483517

@@ -500,6 +534,13 @@ static ssize_t phys_port_name_show(struct device *dev,
500534
struct net_device *netdev = to_net_dev(dev);
501535
ssize_t ret = -EINVAL;
502536

537+
/* The checks are also done in dev_get_phys_port_name; this helps
538+
* returning early without hitting the trylock/restart below.
539+
*/
540+
if (!netdev->netdev_ops->ndo_get_phys_port_name &&
541+
!netdev->netdev_ops->ndo_get_devlink_port)
542+
return -EOPNOTSUPP;
543+
503544
if (!rtnl_trylock())
504545
return restart_syscall();
505546

@@ -522,6 +563,14 @@ static ssize_t phys_switch_id_show(struct device *dev,
522563
struct net_device *netdev = to_net_dev(dev);
523564
ssize_t ret = -EINVAL;
524565

566+
/* The checks are also done in dev_get_phys_port_name; this helps
567+
* returning early without hitting the trylock/restart below. This works
568+
* because recurse is false when calling dev_get_port_parent_id.
569+
*/
570+
if (!netdev->netdev_ops->ndo_get_port_parent_id &&
571+
!netdev->netdev_ops->ndo_get_devlink_port)
572+
return -EOPNOTSUPP;
573+
525574
if (!rtnl_trylock())
526575
return restart_syscall();
527576

@@ -1179,6 +1228,12 @@ static ssize_t tx_maxrate_store(struct netdev_queue *queue,
11791228
if (!capable(CAP_NET_ADMIN))
11801229
return -EPERM;
11811230

1231+
/* The check is also done later; this helps returning early without
1232+
* hitting the trylock/restart below.
1233+
*/
1234+
if (!dev->netdev_ops->ndo_set_tx_maxrate)
1235+
return -EOPNOTSUPP;
1236+
11821237
err = kstrtou32(buf, 10, &rate);
11831238
if (err < 0)
11841239
return err;

0 commit comments

Comments
 (0)