net-sysfs: remove rtnl_trylock from queue attributes
authorAntoine Tenart <atenart@kernel.org>
Tue, 4 Feb 2025 17:03:13 +0000 (18:03 +0100)
committerJakub Kicinski <kuba@kernel.org>
Thu, 6 Feb 2025 01:49:08 +0000 (17:49 -0800)
Similar to the commit removing remove rtnl_trylock from device
attributes we here apply the same technique to networking queues.

Signed-off-by: Antoine Tenart <atenart@kernel.org>
Link: https://patch.msgid.link/20250204170314.146022-5-atenart@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
net/core/net-sysfs.c

index 027af27517fa31b3cf8c695e325c9b5e0a175187..3fe2c521e5740436687f09c572754c5d071038f4 100644 (file)
@@ -1348,9 +1348,11 @@ static int net_rx_queue_change_owner(struct net_device *dev, int num,
  */
 struct netdev_queue_attribute {
        struct attribute attr;
-       ssize_t (*show)(struct netdev_queue *queue, char *buf);
-       ssize_t (*store)(struct netdev_queue *queue,
-                        const char *buf, size_t len);
+       ssize_t (*show)(struct kobject *kobj, struct attribute *attr,
+                       struct netdev_queue *queue, char *buf);
+       ssize_t (*store)(struct kobject *kobj, struct attribute *attr,
+                        struct netdev_queue *queue, const char *buf,
+                        size_t len);
 };
 #define to_netdev_queue_attr(_attr) \
        container_of(_attr, struct netdev_queue_attribute, attr)
@@ -1367,7 +1369,7 @@ static ssize_t netdev_queue_attr_show(struct kobject *kobj,
        if (!attribute->show)
                return -EIO;
 
-       return attribute->show(queue, buf);
+       return attribute->show(kobj, attr, queue, buf);
 }
 
 static ssize_t netdev_queue_attr_store(struct kobject *kobj,
@@ -1381,7 +1383,7 @@ static ssize_t netdev_queue_attr_store(struct kobject *kobj,
        if (!attribute->store)
                return -EIO;
 
-       return attribute->store(queue, buf, count);
+       return attribute->store(kobj, attr, queue, buf, count);
 }
 
 static const struct sysfs_ops netdev_queue_sysfs_ops = {
@@ -1389,7 +1391,8 @@ static const struct sysfs_ops netdev_queue_sysfs_ops = {
        .store = netdev_queue_attr_store,
 };
 
-static ssize_t tx_timeout_show(struct netdev_queue *queue, char *buf)
+static ssize_t tx_timeout_show(struct kobject *kobj, struct attribute *attr,
+                              struct netdev_queue *queue, char *buf)
 {
        unsigned long trans_timeout = atomic_long_read(&queue->trans_timeout);
 
@@ -1407,18 +1410,18 @@ static unsigned int get_netdev_queue_index(struct netdev_queue *queue)
        return i;
 }
 
-static ssize_t traffic_class_show(struct netdev_queue *queue,
-                                 char *buf)
+static ssize_t traffic_class_show(struct kobject *kobj, struct attribute *attr,
+                                 struct netdev_queue *queue, char *buf)
 {
        struct net_device *dev = queue->dev;
-       int num_tc, tc;
-       int index;
+       int num_tc, tc, index, ret;
 
        if (!netif_is_multiqueue(dev))
                return -ENOENT;
 
-       if (!rtnl_trylock())
-               return restart_syscall();
+       ret = sysfs_rtnl_lock(kobj, attr, queue->dev);
+       if (ret)
+               return ret;
 
        index = get_netdev_queue_index(queue);
 
@@ -1445,24 +1448,25 @@ static ssize_t traffic_class_show(struct netdev_queue *queue,
 }
 
 #ifdef CONFIG_XPS
-static ssize_t tx_maxrate_show(struct netdev_queue *queue,
-                              char *buf)
+static ssize_t tx_maxrate_show(struct kobject *kobj, struct attribute *attr,
+                              struct netdev_queue *queue, char *buf)
 {
        return sysfs_emit(buf, "%lu\n", queue->tx_maxrate);
 }
 
-static ssize_t tx_maxrate_store(struct netdev_queue *queue,
-                               const char *buf, size_t len)
+static ssize_t tx_maxrate_store(struct kobject *kobj, struct attribute *attr,
+                               struct netdev_queue *queue, const char *buf,
+                               size_t len)
 {
-       struct net_device *dev = queue->dev;
        int err, index = get_netdev_queue_index(queue);
+       struct net_device *dev = queue->dev;
        u32 rate = 0;
 
        if (!capable(CAP_NET_ADMIN))
                return -EPERM;
 
        /* The check is also done later; this helps returning early without
-        * hitting the trylock/restart below.
+        * hitting the locking section below.
         */
        if (!dev->netdev_ops->ndo_set_tx_maxrate)
                return -EOPNOTSUPP;
@@ -1471,18 +1475,21 @@ static ssize_t tx_maxrate_store(struct netdev_queue *queue,
        if (err < 0)
                return err;
 
-       if (!rtnl_trylock())
-               return restart_syscall();
+       err = sysfs_rtnl_lock(kobj, attr, dev);
+       if (err)
+               return err;
 
        err = -EOPNOTSUPP;
        if (dev->netdev_ops->ndo_set_tx_maxrate)
                err = dev->netdev_ops->ndo_set_tx_maxrate(dev, index, rate);
 
-       rtnl_unlock();
        if (!err) {
                queue->tx_maxrate = rate;
+               rtnl_unlock();
                return len;
        }
+
+       rtnl_unlock();
        return err;
 }
 
@@ -1526,16 +1533,17 @@ static ssize_t bql_set(const char *buf, const size_t count,
        return count;
 }
 
-static ssize_t bql_show_hold_time(struct netdev_queue *queue,
-                                 char *buf)
+static ssize_t bql_show_hold_time(struct kobject *kobj, struct attribute *attr,
+                                 struct netdev_queue *queue, char *buf)
 {
        struct dql *dql = &queue->dql;
 
        return sysfs_emit(buf, "%u\n", jiffies_to_msecs(dql->slack_hold_time));
 }
 
-static ssize_t bql_set_hold_time(struct netdev_queue *queue,
-                                const char *buf, size_t len)
+static ssize_t bql_set_hold_time(struct kobject *kobj, struct attribute *attr,
+                                struct netdev_queue *queue, const char *buf,
+                                size_t len)
 {
        struct dql *dql = &queue->dql;
        unsigned int value;
@@ -1554,15 +1562,17 @@ static struct netdev_queue_attribute bql_hold_time_attribute __ro_after_init
        = __ATTR(hold_time, 0644,
                 bql_show_hold_time, bql_set_hold_time);
 
-static ssize_t bql_show_stall_thrs(struct netdev_queue *queue, char *buf)
+static ssize_t bql_show_stall_thrs(struct kobject *kobj, struct attribute *attr,
+                                  struct netdev_queue *queue, char *buf)
 {
        struct dql *dql = &queue->dql;
 
        return sysfs_emit(buf, "%u\n", jiffies_to_msecs(dql->stall_thrs));
 }
 
-static ssize_t bql_set_stall_thrs(struct netdev_queue *queue,
-                                 const char *buf, size_t len)
+static ssize_t bql_set_stall_thrs(struct kobject *kobj, struct attribute *attr,
+                                 struct netdev_queue *queue, const char *buf,
+                                 size_t len)
 {
        struct dql *dql = &queue->dql;
        unsigned int value;
@@ -1588,13 +1598,15 @@ static ssize_t bql_set_stall_thrs(struct netdev_queue *queue,
 static struct netdev_queue_attribute bql_stall_thrs_attribute __ro_after_init =
        __ATTR(stall_thrs, 0644, bql_show_stall_thrs, bql_set_stall_thrs);
 
-static ssize_t bql_show_stall_max(struct netdev_queue *queue, char *buf)
+static ssize_t bql_show_stall_max(struct kobject *kobj, struct attribute *attr,
+                                 struct netdev_queue *queue, char *buf)
 {
        return sysfs_emit(buf, "%u\n", READ_ONCE(queue->dql.stall_max));
 }
 
-static ssize_t bql_set_stall_max(struct netdev_queue *queue,
-                                const char *buf, size_t len)
+static ssize_t bql_set_stall_max(struct kobject *kobj, struct attribute *attr,
+                                struct netdev_queue *queue, const char *buf,
+                                size_t len)
 {
        WRITE_ONCE(queue->dql.stall_max, 0);
        return len;
@@ -1603,7 +1615,8 @@ static ssize_t bql_set_stall_max(struct netdev_queue *queue,
 static struct netdev_queue_attribute bql_stall_max_attribute __ro_after_init =
        __ATTR(stall_max, 0644, bql_show_stall_max, bql_set_stall_max);
 
-static ssize_t bql_show_stall_cnt(struct netdev_queue *queue, char *buf)
+static ssize_t bql_show_stall_cnt(struct kobject *kobj, struct attribute *attr,
+                                 struct netdev_queue *queue, char *buf)
 {
        struct dql *dql = &queue->dql;
 
@@ -1613,8 +1626,8 @@ static ssize_t bql_show_stall_cnt(struct netdev_queue *queue, char *buf)
 static struct netdev_queue_attribute bql_stall_cnt_attribute __ro_after_init =
        __ATTR(stall_cnt, 0444, bql_show_stall_cnt, NULL);
 
-static ssize_t bql_show_inflight(struct netdev_queue *queue,
-                                char *buf)
+static ssize_t bql_show_inflight(struct kobject *kobj, struct attribute *attr,
+                                struct netdev_queue *queue, char *buf)
 {
        struct dql *dql = &queue->dql;
 
@@ -1625,13 +1638,16 @@ static struct netdev_queue_attribute bql_inflight_attribute __ro_after_init =
        __ATTR(inflight, 0444, bql_show_inflight, NULL);
 
 #define BQL_ATTR(NAME, FIELD)                                          \
-static ssize_t bql_show_ ## NAME(struct netdev_queue *queue,           \
-                                char *buf)                             \
+static ssize_t bql_show_ ## NAME(struct kobject *kobj,                 \
+                                struct attribute *attr,                \
+                                struct netdev_queue *queue, char *buf) \
 {                                                                      \
        return bql_show(buf, queue->dql.FIELD);                         \
 }                                                                      \
                                                                        \
-static ssize_t bql_set_ ## NAME(struct netdev_queue *queue,            \
+static ssize_t bql_set_ ## NAME(struct kobject *kobj,                  \
+                               struct attribute *attr,                 \
+                               struct netdev_queue *queue,             \
                                const char *buf, size_t len)            \
 {                                                                      \
        return bql_set(buf, len, &queue->dql.FIELD);                    \
@@ -1717,19 +1733,21 @@ out_no_maps:
        return len < PAGE_SIZE ? len : -EINVAL;
 }
 
-static ssize_t xps_cpus_show(struct netdev_queue *queue, char *buf)
+static ssize_t xps_cpus_show(struct kobject *kobj, struct attribute *attr,
+                            struct netdev_queue *queue, char *buf)
 {
        struct net_device *dev = queue->dev;
        unsigned int index;
-       int len, tc;
+       int len, tc, ret;
 
        if (!netif_is_multiqueue(dev))
                return -ENOENT;
 
        index = get_netdev_queue_index(queue);
 
-       if (!rtnl_trylock())
-               return restart_syscall();
+       ret = sysfs_rtnl_lock(kobj, attr, queue->dev);
+       if (ret)
+               return ret;
 
        /* If queue belongs to subordinate dev use its map */
        dev = netdev_get_tx_queue(dev, index)->sb_dev ? : dev;
@@ -1740,18 +1758,21 @@ static ssize_t xps_cpus_show(struct netdev_queue *queue, char *buf)
                return -EINVAL;
        }
 
-       /* Make sure the subordinate device can't be freed */
-       get_device(&dev->dev);
+       /* Increase the net device refcnt to make sure it won't be freed while
+        * xps_queue_show is running.
+        */
+       dev_hold(dev);
        rtnl_unlock();
 
        len = xps_queue_show(dev, index, tc, buf, XPS_CPUS);
 
-       put_device(&dev->dev);
+       dev_put(dev);
        return len;
 }
 
-static ssize_t xps_cpus_store(struct netdev_queue *queue,
-                             const char *buf, size_t len)
+static ssize_t xps_cpus_store(struct kobject *kobj, struct attribute *attr,
+                             struct netdev_queue *queue, const char *buf,
+                             size_t len)
 {
        struct net_device *dev = queue->dev;
        unsigned int index;
@@ -1775,9 +1796,10 @@ static ssize_t xps_cpus_store(struct netdev_queue *queue,
                return err;
        }
 
-       if (!rtnl_trylock()) {
+       err = sysfs_rtnl_lock(kobj, attr, dev);
+       if (err) {
                free_cpumask_var(mask);
-               return restart_syscall();
+               return err;
        }
 
        err = netif_set_xps_queue(dev, mask, index);
@@ -1791,26 +1813,34 @@ static ssize_t xps_cpus_store(struct netdev_queue *queue,
 static struct netdev_queue_attribute xps_cpus_attribute __ro_after_init
        = __ATTR_RW(xps_cpus);
 
-static ssize_t xps_rxqs_show(struct netdev_queue *queue, char *buf)
+static ssize_t xps_rxqs_show(struct kobject *kobj, struct attribute *attr,
+                            struct netdev_queue *queue, char *buf)
 {
        struct net_device *dev = queue->dev;
        unsigned int index;
-       int tc;
+       int tc, ret;
 
        index = get_netdev_queue_index(queue);
 
-       if (!rtnl_trylock())
-               return restart_syscall();
+       ret = sysfs_rtnl_lock(kobj, attr, dev);
+       if (ret)
+               return ret;
 
        tc = netdev_txq_to_tc(dev, index);
+
+       /* Increase the net device refcnt to make sure it won't be freed while
+        * xps_queue_show is running.
+        */
+       dev_hold(dev);
        rtnl_unlock();
-       if (tc < 0)
-               return -EINVAL;
 
-       return xps_queue_show(dev, index, tc, buf, XPS_RXQS);
+       ret = tc >= 0 ? xps_queue_show(dev, index, tc, buf, XPS_RXQS) : -EINVAL;
+       dev_put(dev);
+       return ret;
 }
 
-static ssize_t xps_rxqs_store(struct netdev_queue *queue, const char *buf,
+static ssize_t xps_rxqs_store(struct kobject *kobj, struct attribute *attr,
+                             struct netdev_queue *queue, const char *buf,
                              size_t len)
 {
        struct net_device *dev = queue->dev;
@@ -1834,9 +1864,10 @@ static ssize_t xps_rxqs_store(struct netdev_queue *queue, const char *buf,
                return err;
        }
 
-       if (!rtnl_trylock()) {
+       err = sysfs_rtnl_lock(kobj, attr, dev);
+       if (err) {
                bitmap_free(mask);
-               return restart_syscall();
+               return err;
        }
 
        cpus_read_lock();