All of lore.kernel.org
 help / color / mirror / Atom feed
From: Antoine Tenart <atenart@kernel.org>
To: davem@davemloft.net, kuba@kernel.org
Cc: Antoine Tenart <atenart@kernel.org>,
	pabeni@redhat.com, gregkh@linuxfoundation.org,
	ebiederm@xmission.com, stephen@networkplumber.org,
	herbert@gondor.apana.org.au, juri.lelli@redhat.com,
	netdev@vger.kernel.org
Subject: [RFC PATCH net-next 9/9] net-sysfs: remove the use of rtnl_trylock/restart_syscall
Date: Tue, 28 Sep 2021 14:55:00 +0200	[thread overview]
Message-ID: <20210928125500.167943-10-atenart@kernel.org> (raw)
In-Reply-To: <20210928125500.167943-1-atenart@kernel.org>

The ABBA deadlock avoided by using rtnl_trylock and restart_syscall was
fixed in previous commits, we can now remove the use of this
trylock/restart logic and have net-sysfs operations not spinning when
rtnl is already taken.

Signed-off-by: Antoine Tenart <atenart@kernel.org>
---
 net/core/net-sysfs.c | 95 +++++++-------------------------------------
 1 file changed, 14 insertions(+), 81 deletions(-)

diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index e754f00c117b..987b32fd8604 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -90,9 +90,7 @@ static ssize_t netdev_store(struct device *dev, struct device_attribute *attr,
 	if (ret)
 		goto err;
 
-	if (!rtnl_trylock())
-		return restart_syscall();
-
+	rtnl_lock();
 	if (dev_isalive(netdev)) {
 		ret = (*set)(netdev, new);
 		if (ret == 0)
@@ -196,15 +194,7 @@ static ssize_t speed_show(struct device *dev,
 	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 trylock/restart below.
-	 */
-	if (!netdev->ethtool_ops->get_link_ksettings)
-		return -EOPNOTSUPP;
-
-	if (!rtnl_trylock())
-		return restart_syscall();
-
+	rtnl_lock();
 	if (netif_running(netdev)) {
 		struct ethtool_link_ksettings cmd;
 
@@ -222,15 +212,7 @@ static ssize_t duplex_show(struct device *dev,
 	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 trylock/restart below.
-	 */
-	if (!netdev->ethtool_ops->get_link_ksettings)
-		return -EOPNOTSUPP;
-
-	if (!rtnl_trylock())
-		return restart_syscall();
-
+	rtnl_lock();
 	if (netif_running(netdev)) {
 		struct ethtool_link_ksettings cmd;
 
@@ -427,9 +409,7 @@ static ssize_t ifalias_store(struct device *dev, struct device_attribute *attr,
 	if (len >  0 && buf[len - 1] == '\n')
 		--count;
 
-	if (!rtnl_trylock())
-		return restart_syscall();
-
+	rtnl_lock();
 	if (dev_isalive(netdev)) {
 		ret = dev_set_alias(netdev, buf, count);
 		if (ret < 0)
@@ -490,15 +470,7 @@ static ssize_t phys_port_id_show(struct device *dev,
 	struct net_device *netdev = to_net_dev(dev);
 	ssize_t ret = -EINVAL;
 
-	/* The check is also done in dev_get_phys_port_id; this helps returning
-	 * early without hitting the trylock/restart below.
-	 */
-	if (!netdev->netdev_ops->ndo_get_phys_port_id)
-		return -EOPNOTSUPP;
-
-	if (!rtnl_trylock())
-		return restart_syscall();
-
+	rtnl_lock();
 	if (dev_isalive(netdev)) {
 		struct netdev_phys_item_id ppid;
 
@@ -518,16 +490,7 @@ static ssize_t phys_port_name_show(struct device *dev,
 	struct net_device *netdev = to_net_dev(dev);
 	ssize_t ret = -EINVAL;
 
-	/* The checks are also done in dev_get_phys_port_name; this helps
-	 * returning early without hitting the trylock/restart below.
-	 */
-	if (!netdev->netdev_ops->ndo_get_phys_port_name &&
-	    !netdev->netdev_ops->ndo_get_devlink_port)
-		return -EOPNOTSUPP;
-
-	if (!rtnl_trylock())
-		return restart_syscall();
-
+	rtnl_lock();
 	if (dev_isalive(netdev)) {
 		char name[IFNAMSIZ];
 
@@ -547,16 +510,7 @@ static ssize_t phys_switch_id_show(struct device *dev,
 	struct net_device *netdev = to_net_dev(dev);
 	ssize_t ret = -EINVAL;
 
-	/* The checks are also done in dev_get_phys_port_name; this helps
-	 * returning early without hitting the trylock/restart below.
-	 */
-	if (!netdev->netdev_ops->ndo_get_port_parent_id &&
-	    !netdev->netdev_ops->ndo_get_devlink_port)
-		return -EOPNOTSUPP;
-
-	if (!rtnl_trylock())
-		return restart_syscall();
-
+	rtnl_lock();
 	if (dev_isalive(netdev)) {
 		struct netdev_phys_item_id ppid = { };
 
@@ -576,9 +530,7 @@ static ssize_t threaded_show(struct device *dev,
 	struct net_device *netdev = to_net_dev(dev);
 	ssize_t ret = -EINVAL;
 
-	if (!rtnl_trylock())
-		return restart_syscall();
-
+	rtnl_lock();
 	if (dev_isalive(netdev))
 		ret = sprintf(buf, fmt_dec, netdev->threaded);
 
@@ -1214,9 +1166,7 @@ static ssize_t traffic_class_show(struct netdev_queue *queue,
 	if (!netif_is_multiqueue(dev))
 		return -ENOENT;
 
-	if (!rtnl_trylock())
-		return restart_syscall();
-
+	rtnl_lock();
 	index = get_netdev_queue_index(queue);
 
 	/* If queue belongs to subordinate dev use its TC mapping */
@@ -1258,18 +1208,11 @@ static ssize_t tx_maxrate_store(struct netdev_queue *queue,
 	if (!capable(CAP_NET_ADMIN))
 		return -EPERM;
 
-	/* The check is also done later; this helps returning early without
-	 * hitting the trylock/restart below.
-	 */
-	if (!dev->netdev_ops->ndo_set_tx_maxrate)
-		return -EOPNOTSUPP;
-
 	err = kstrtou32(buf, 10, &rate);
 	if (err < 0)
 		return err;
 
-	if (!rtnl_trylock())
-		return restart_syscall();
+	rtnl_lock();
 
 	err = -EOPNOTSUPP;
 	if (dev->netdev_ops->ndo_set_tx_maxrate)
@@ -1460,8 +1403,7 @@ static ssize_t xps_cpus_show(struct netdev_queue *queue, char *buf)
 
 	index = get_netdev_queue_index(queue);
 
-	if (!rtnl_trylock())
-		return restart_syscall();
+	rtnl_lock();
 
 	/* If queue belongs to subordinate dev use its map */
 	dev = netdev_get_tx_queue(dev, index)->sb_dev ? : dev;
@@ -1507,11 +1449,7 @@ static ssize_t xps_cpus_store(struct netdev_queue *queue,
 		return err;
 	}
 
-	if (!rtnl_trylock()) {
-		free_cpumask_var(mask);
-		return restart_syscall();
-	}
-
+	rtnl_lock();
 	err = netif_set_xps_queue(dev, mask, index);
 	rtnl_unlock();
 
@@ -1531,9 +1469,7 @@ static ssize_t xps_rxqs_show(struct netdev_queue *queue, char *buf)
 
 	index = get_netdev_queue_index(queue);
 
-	if (!rtnl_trylock())
-		return restart_syscall();
-
+	rtnl_lock();
 	tc = netdev_txq_to_tc(dev, index);
 	rtnl_unlock();
 	if (tc < 0)
@@ -1566,10 +1502,7 @@ static ssize_t xps_rxqs_store(struct netdev_queue *queue, const char *buf,
 		return err;
 	}
 
-	if (!rtnl_trylock()) {
-		bitmap_free(mask);
-		return restart_syscall();
-	}
+	rtnl_lock();
 
 	cpus_read_lock();
 	err = __netif_set_xps_queue(dev, mask, index, XPS_RXQS);
-- 
2.31.1


  parent reply	other threads:[~2021-09-28 12:55 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-28 12:54 [RFC PATCH net-next 0/9] Userspace spinning on net-sysfs access Antoine Tenart
2021-09-28 12:54 ` [RFC PATCH net-next 1/9] net-sysfs: try not to restart the syscall if it will fail eventually Antoine Tenart
2021-09-28 12:54 ` [RFC PATCH net-next 2/9] net: split unlisting the net device from unlisting its node name Antoine Tenart
2021-09-28 12:54 ` [RFC PATCH net-next 3/9] net: export netdev_name_node_lookup Antoine Tenart
2021-09-28 12:54 ` [RFC PATCH net-next 4/9] bonding: use the correct function to check for netdev name collision Antoine Tenart
2021-09-28 12:54 ` [RFC PATCH net-next 5/9] ppp: " Antoine Tenart
2021-09-28 12:54 ` [RFC PATCH net-next 6/9] net: " Antoine Tenart
2021-09-28 12:54 ` [RFC PATCH net-next 7/9] net: delay the removal of the name nodes until run_todo Antoine Tenart
2021-09-28 12:54 ` [RFC PATCH net-next 8/9] net: delay device_del " Antoine Tenart
2021-09-29  0:02   ` Jakub Kicinski
2021-09-29  8:26     ` Antoine Tenart
2021-09-29 13:31       ` Jakub Kicinski
2021-09-29 17:31         ` Antoine Tenart
2021-10-29  9:04           ` Antoine Tenart
2021-10-05 15:21         ` Antoine Tenart
2021-10-05 18:34           ` Jakub Kicinski
2021-09-28 12:55 ` Antoine Tenart [this message]
2021-10-06  6:45   ` [RFC PATCH net-next 9/9] net-sysfs: remove the use of rtnl_trylock/restart_syscall Michal Hocko
2021-10-06  8:03     ` Antoine Tenart
2021-10-06  8:55       ` Michal Hocko
2021-10-06  6:37 ` [RFC PATCH net-next 0/9] Userspace spinning on net-sysfs access Michal Hocko
2021-10-06  7:59   ` Antoine Tenart
2021-10-06  8:35     ` Michal Hocko
2021-10-29 14:33 ` Antoine Tenart
2021-10-29 15:45   ` Stephen Hemminger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210928125500.167943-10-atenart@kernel.org \
    --to=atenart@kernel.org \
    --cc=davem@davemloft.net \
    --cc=ebiederm@xmission.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=herbert@gondor.apana.org.au \
    --cc=juri.lelli@redhat.com \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=stephen@networkplumber.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.