From patchwork Thu Jan 7 09:49:46 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Vladimir Oltean X-Patchwork-Id: 358724 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0F93FC43381 for ; Thu, 7 Jan 2021 09:52:04 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id CEC5C2333E for ; Thu, 7 Jan 2021 09:52:03 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727944AbhAGJvx (ORCPT ); Thu, 7 Jan 2021 04:51:53 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40940 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727906AbhAGJvr (ORCPT ); Thu, 7 Jan 2021 04:51:47 -0500 Received: from mail-ej1-x62c.google.com (mail-ej1-x62c.google.com [IPv6:2a00:1450:4864:20::62c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 62CB4C0612FD for ; Thu, 7 Jan 2021 01:51:14 -0800 (PST) Received: by mail-ej1-x62c.google.com with SMTP id d17so8807108ejy.9 for ; Thu, 07 Jan 2021 01:51:14 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=ZY8ji9fFG08blymAp9Y+P4geDkhw8vQUxrybxWKW3qs=; b=GEJ1kf+Ks1TUZNQcMn67KWGzsLJSAspBL/9c1SwcRXezRM/W134BYJLbqxkMBn6fUU aYmq7+k50UhlrD3gwzyWXFxCECIjyeDAp2Rf7Cla9MTSOs6QOLkpOW7lpce5l5/y/ZKN GlBp+pJ6LxzmkfGNBb49+YxMtpDBS/O7dXM+JZzvvcg+FtEHYk8+f8VZAKrLK61juAfB vDXhWaH2BA/56KHTDlczed0zsK1gGgxmtFKJ+Kn0/sR8SoKigcU6Oqtr4w2DSoIIJIGC z2v5OhN0oVz7S31kGviDUGlFctAJJh7tVh6bnGJdsRMjKFF3/Ir03ElNQXqwv06LJIxw jxYQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=ZY8ji9fFG08blymAp9Y+P4geDkhw8vQUxrybxWKW3qs=; b=Z/cd7ue3L/P5Gk3oPuHysL+E/hDkc1hOQjafipy872YskcZSDsw2yl2fZU9AtZw7Zx L11IaPh5gImAWgjHLJOHZwgpsCQ9pA86rNSsWek8UBBJOmXdAwMjkh0q+8UBnLmw6ywo ZRsw6rD0xJ8ANYVHLy7xP6tmNqG7GmaWUZxpdLg29gATkH4K1q5R4wbzAvc5wGnmiDdx yiMEGxuXReXV3RxmGSky+D4NXSsECCGWqpq1UuVLjbEWXn3lO1D2eoEUmzQ3qRAQyiLi 4aZZl3KK2ALRj94tJex6/1Ov2pDZz4ukOudsvqXZ+6jRQGXGC1ypYTWfg0ZXaoPePFiN KmzQ== X-Gm-Message-State: AOAM530jhAW7j+RSOkMr6fPq1a5a8JJsaWG07M/z3+T6xWf1RJZ6Rl7b 6vdqdY4MlKR428ki06HXmw4= X-Google-Smtp-Source: ABdhPJyA1behA7NXwJ0YIyS1peKH1CCoP6WXTV5ZK39bP3NiTOffT6hW+PaWZvK8S+0fZRRpTQX74w== X-Received: by 2002:a17:906:3a98:: with SMTP id y24mr5319017ejd.436.1610013073187; Thu, 07 Jan 2021 01:51:13 -0800 (PST) Received: from localhost.localdomain (5-12-227-87.residential.rdsnet.ro. [5.12.227.87]) by smtp.gmail.com with ESMTPSA id k15sm2251571ejc.79.2021.01.07.01.51.11 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 07 Jan 2021 01:51:12 -0800 (PST) From: Vladimir Oltean To: "David S . Miller" , Jakub Kicinski Cc: netdev@vger.kernel.org, Andrew Lunn , Florian Fainelli , Cong Wang , Stephen Hemminger , Eric Dumazet , George McCollister , Oleksij Rempel , Jay Vosburgh , Veaceslav Falico , Andy Gospodarek , Arnd Bergmann , Taehee Yoo , Jiri Pirko , Florian Westphal Subject: [PATCH v3 net-next 07/12] parisc/led: hold the netdev lists lock when retrieving device statistics Date: Thu, 7 Jan 2021 11:49:46 +0200 Message-Id: <20210107094951.1772183-8-olteanv@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210107094951.1772183-1-olteanv@gmail.com> References: <20210107094951.1772183-1-olteanv@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Vladimir Oltean In the effort of making .ndo_get_stats64 be able to sleep, we need to ensure the callers of dev_get_stats do not use atomic context. The LED driver for HP-PARISC workstations uses a workqueue to periodically check for updates in network interface statistics, and flicker when those have changed (i.e. there has been activity on the line). Ignoring the fact that this driver is completely duplicating drivers/leds/trigger/ledtrig-netdev.c, there is an even bigger problem. Now, the dev_get_stats call can sleep, and iterating through the list of network interfaces still needs to ensure the integrity of list of network interfaces. So that leaves us only one locking option given the current design of the network stack, and that is the netns mutex. Cc: "James E.J. Bottomley" Cc: Helge Deller Cc: linux-parisc@vger.kernel.org Signed-off-by: Vladimir Oltean --- Changes in v3: None. Changes in v2: None. drivers/parisc/led.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/drivers/parisc/led.c b/drivers/parisc/led.c index 3cada632a4be..c8c6b2301dc9 100644 --- a/drivers/parisc/led.c +++ b/drivers/parisc/led.c @@ -38,7 +38,6 @@ #include #include #include -#include #include #include #include @@ -355,25 +354,29 @@ static __inline__ int led_get_net_activity(void) int retval; rx_total = tx_total = 0; - - /* we are running as a workqueue task, so we can use an RCU lookup */ - rcu_read_lock(); - for_each_netdev_rcu(&init_net, dev) { + + /* we are running as a workqueue task, so we can sleep */ + netif_lists_lock(&init_net); + + for_each_netdev(&init_net, dev) { + struct in_device *in_dev = in_dev_get(dev); const struct rtnl_link_stats64 *stats; struct rtnl_link_stats64 temp; - struct in_device *in_dev = __in_dev_get_rcu(dev); - if (!in_dev || !in_dev->ifa_list) + if (!in_dev || !in_dev->ifa_list || + ipv4_is_loopback(in_dev->ifa_list->ifa_local)) { + in_dev_put(in_dev); continue; + } - if (ipv4_is_loopback(in_dev->ifa_list->ifa_local)) - continue; + in_dev_put(in_dev); stats = dev_get_stats(dev, &temp); rx_total += stats->rx_packets; tx_total += stats->tx_packets; } - rcu_read_unlock(); + + netif_lists_unlock(&init_net); retval = 0; From patchwork Thu Jan 7 09:49:48 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Vladimir Oltean X-Patchwork-Id: 358719 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED, USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D47FEC433E6 for ; Thu, 7 Jan 2021 09:52:33 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id A24682333F for ; Thu, 7 Jan 2021 09:52:33 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727989AbhAGJwG (ORCPT ); Thu, 7 Jan 2021 04:52:06 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41018 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727961AbhAGJv7 (ORCPT ); Thu, 7 Jan 2021 04:51:59 -0500 Received: from mail-ej1-x632.google.com (mail-ej1-x632.google.com [IPv6:2a00:1450:4864:20::632]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 78DBAC0612FF for ; Thu, 7 Jan 2021 01:51:17 -0800 (PST) Received: by mail-ej1-x632.google.com with SMTP id qw4so8759405ejb.12 for ; Thu, 07 Jan 2021 01:51:17 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=Nem25OmsmkjNSBbvqZ41iqgC+QLTepbfzN5gUedefIM=; b=QTQfk6NwC9aHiSW6tnJLMlXC49jVo3IefyXWD0DWThFSllY63Yv3fUoW+YjVwpt25i uCA+UqbZKtGMIlv1RSc5ftENwzGJvR9YNjMSt4ZHkvYyRpDufgt15CSN+rnjiVZjjfDW 3wJfv2FZmNhs8vELqjCL12M/iDl9SKTnjpqUkpID2HEQONSpiy/Oz3vkiJOZFjgn3QSU 6BCew6hQqi9Xd+3p4evemicLXAhzqpZxuIUFhEvxhPPaRs3YI7Hoy/62Zwj+1apUox+m 9BXQsF36qHS4Y4gQuB2RCbljS642phou8/u9B2AWWkD5lvtGeBZyAe7jc8Ww2w/Y8IxY TMUQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=Nem25OmsmkjNSBbvqZ41iqgC+QLTepbfzN5gUedefIM=; b=HiteX04TuyXY0PzPmJ0qHfPRDZSkpxKWDBRi0Fh2p16KLITd/flpfJmLANMbYfhx/7 SfZR4yO9E/fDedAs229TUmJV6I025+y/1d5JOlYCcZyUwHLDwesPM7g8m3Ijt4HFwyh9 plIAFxEerIGnDFmgVYN+wMnDvOCIYHt5E0OypSoLtFGJXMOs+fMm7VEJdTzR6mCIsxQq e1AKsX7A0YAVmJXCbUIzuoq9k9dX/qLEGIZ60jxSVIyMF28Hux19syz/zi9RL7qiU574 u83fJ9JMsTJCx+8AmwByB86DRNKqJ0afWrE4mGVJdG1oD2tIFVbjHk1XQvRm6tN9vCvx c5vA== X-Gm-Message-State: AOAM532vXVUN7CwT2VPfFLqDPFVGe1MiLtzHzSlkJgpOoZnch8O2+QQw aWXXx1hN9Snj9g62dyxwUtqEU9DvbkgFNA== X-Google-Smtp-Source: ABdhPJywu7JlHdM+ODHSPcaQ4L3iYf8jBzPnnhNkefzoyske7RnJ1LBa0+3vUVnll1dtMJPUMiu18w== X-Received: by 2002:a17:906:c097:: with SMTP id f23mr5909527ejz.136.1610013076245; Thu, 07 Jan 2021 01:51:16 -0800 (PST) Received: from localhost.localdomain (5-12-227-87.residential.rdsnet.ro. [5.12.227.87]) by smtp.gmail.com with ESMTPSA id k15sm2251571ejc.79.2021.01.07.01.51.14 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 07 Jan 2021 01:51:15 -0800 (PST) From: Vladimir Oltean To: "David S . Miller" , Jakub Kicinski Cc: netdev@vger.kernel.org, Andrew Lunn , Florian Fainelli , Cong Wang , Stephen Hemminger , Eric Dumazet , George McCollister , Oleksij Rempel , Jay Vosburgh , Veaceslav Falico , Andy Gospodarek , Arnd Bergmann , Taehee Yoo , Jiri Pirko , Florian Westphal Subject: [PATCH v3 net-next 09/12] net: net_failover: ensure .ndo_get_stats64 can sleep Date: Thu, 7 Jan 2021 11:49:48 +0200 Message-Id: <20210107094951.1772183-10-olteanv@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210107094951.1772183-1-olteanv@gmail.com> References: <20210107094951.1772183-1-olteanv@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Vladimir Oltean The failover framework sets up a virtio_net interface [ when it has the VIRTIO_NET_F_STANDBY feature ] and a VF interface, having the same MAC address, in a standby/active relationship. When the active VF is unplugged, the standby virtio_net temporarily kicks in. The failover framework registers a common upper for the active and the standby interface, which is what the application layer uses. This is similar to bonding/team. The statistics of the upper interface are the sum of the statistics of the active and of the standby interface. There is an effort to convert .ndo_get_stats64 to sleepable context, and for that to work, we need to prevent callers of dev_get_stats from using atomic locking. The failover driver needs protection via an RCU read-side critical section to access the standby and the active interface. This has two features: - It is atomic: this needs to change. - It is reentrant: this is ok, because generally speaking, dev_get_stats is recursive, and taking global locks is a bad thing from a recursive context. A better locking architecture would be to do what the team driver does. Instead of using something as broad as the rtnl_mutex to ensure serialization of updates, it should use something more specific, like a private mutex. This patch adds that and names it slaves_lock. The slaves_lock now protects the only updater, the rcu_assign_pointer sections from net_failover_slave_register. In the team driver, a separate lockdep class is created for each team lock, to account for possible nesting (team over team over ...). For the net_failover driver, we can do something simpler, which is to just not hold any lock while we call dev_get_stats recursively. We can "cheat" and use dev_hold to take a reference on the active and backup interfaces, and netdev_wait_allrefs() will just have to wait until we finish. Signed-off-by: Vladimir Oltean --- Changes in v3: None. Changes in v2: Switched to the new scheme of holding just a refcnt to the slave interfaces while recursing with dev_get_stats. drivers/net/net_failover.c | 62 +++++++++++++++++++++++++++----------- include/net/net_failover.h | 9 ++++-- 2 files changed, 52 insertions(+), 19 deletions(-) diff --git a/drivers/net/net_failover.c b/drivers/net/net_failover.c index 4f83165412bd..c83066b0ef70 100644 --- a/drivers/net/net_failover.c +++ b/drivers/net/net_failover.c @@ -27,6 +27,9 @@ #include #include +#define nfo_dereference(nfo_info, p) \ + rcu_dereference_protected(p, lockdep_is_held(&nfo_info->slaves_lock)) + static bool net_failover_xmit_ready(struct net_device *dev) { return netif_running(dev) && netif_carrier_ok(dev); @@ -183,32 +186,48 @@ static void net_failover_get_stats(struct net_device *dev, struct rtnl_link_stats64 *stats) { struct net_failover_info *nfo_info = netdev_priv(dev); - struct rtnl_link_stats64 temp; - struct net_device *slave_dev; + struct rtnl_link_stats64 primary_stats; + struct rtnl_link_stats64 standby_stats; + struct net_device *primary_dev; + struct net_device *standby_dev; - spin_lock(&nfo_info->stats_lock); - memcpy(stats, &nfo_info->failover_stats, sizeof(*stats)); + mutex_lock(&nfo_info->slaves_lock); - rcu_read_lock(); + primary_dev = nfo_dereference(nfo_info, nfo_info->primary_dev); + if (primary_dev) + dev_hold(primary_dev); - slave_dev = rcu_dereference(nfo_info->primary_dev); - if (slave_dev) { - dev_get_stats(slave_dev, &temp); - net_failover_fold_stats(stats, &temp, &nfo_info->primary_stats); - memcpy(&nfo_info->primary_stats, &temp, sizeof(temp)); + standby_dev = nfo_dereference(nfo_info, nfo_info->standby_dev); + if (standby_dev) + dev_hold(standby_dev); + + mutex_unlock(&nfo_info->slaves_lock); + + /* Don't hold slaves_lock while calling dev_get_stats, just a + * reference to ensure they won't get unregistered. + */ + if (primary_dev) { + dev_get_stats(primary_dev, &primary_stats); + dev_put(primary_dev); } - slave_dev = rcu_dereference(nfo_info->standby_dev); - if (slave_dev) { - dev_get_stats(slave_dev, &temp); - net_failover_fold_stats(stats, &temp, &nfo_info->standby_stats); - memcpy(&nfo_info->standby_stats, &temp, sizeof(temp)); + if (standby_dev) { + dev_get_stats(standby_dev, &standby_stats); + dev_put(standby_dev); } - rcu_read_unlock(); + mutex_lock(&nfo_info->stats_lock); + + memcpy(stats, &nfo_info->failover_stats, sizeof(*stats)); + + net_failover_fold_stats(stats, &primary_stats, &nfo_info->primary_stats); + memcpy(&nfo_info->primary_stats, &primary_stats, sizeof(primary_stats)); + net_failover_fold_stats(stats, &standby_stats, &nfo_info->standby_stats); + memcpy(&nfo_info->standby_stats, &standby_stats, sizeof(standby_stats)); memcpy(&nfo_info->failover_stats, stats, sizeof(*stats)); - spin_unlock(&nfo_info->stats_lock); + + mutex_unlock(&nfo_info->stats_lock); } static int net_failover_change_mtu(struct net_device *dev, int new_mtu) @@ -540,6 +559,8 @@ static int net_failover_slave_register(struct net_device *slave_dev, primary_dev = rtnl_dereference(nfo_info->primary_dev); slave_is_standby = slave_dev->dev.parent == failover_dev->dev.parent; + mutex_lock(&nfo_info->slaves_lock); + if (slave_is_standby) { rcu_assign_pointer(nfo_info->standby_dev, slave_dev); standby_dev = slave_dev; @@ -552,6 +573,8 @@ static int net_failover_slave_register(struct net_device *slave_dev, failover_dev->max_mtu = slave_dev->max_mtu; } + mutex_unlock(&nfo_info->slaves_lock); + net_failover_lower_state_changed(slave_dev, primary_dev, standby_dev); net_failover_compute_features(failover_dev); @@ -709,6 +732,7 @@ static struct failover_ops net_failover_ops = { struct failover *net_failover_create(struct net_device *standby_dev) { struct device *dev = standby_dev->dev.parent; + struct net_failover_info *nfo_info; struct net_device *failover_dev; struct failover *failover; int err; @@ -753,6 +777,10 @@ struct failover *net_failover_create(struct net_device *standby_dev) failover_dev->min_mtu = standby_dev->min_mtu; failover_dev->max_mtu = standby_dev->max_mtu; + nfo_info = netdev_priv(failover_dev); + mutex_init(&nfo_info->slaves_lock); + mutex_init(&nfo_info->stats_lock); + err = register_netdev(failover_dev); if (err) { dev_err(dev, "Unable to register failover_dev!\n"); diff --git a/include/net/net_failover.h b/include/net/net_failover.h index b12a1c469d1c..988cdfaf14ca 100644 --- a/include/net/net_failover.h +++ b/include/net/net_failover.h @@ -23,8 +23,13 @@ struct net_failover_info { /* aggregated stats */ struct rtnl_link_stats64 failover_stats; - /* spinlock while updating stats */ - spinlock_t stats_lock; + /* lock for updating stats */ + struct mutex stats_lock; + + /* lock for protecting lower interfaces. + * TODO: convert all rtnl_dereference instances to nfo_dereference + */ + struct mutex slaves_lock; }; struct failover *net_failover_create(struct net_device *standby_dev); From patchwork Thu Jan 7 09:49:49 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Vladimir Oltean X-Patchwork-Id: 358722 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id DD141C43333 for ; Thu, 7 Jan 2021 09:52:04 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C2B9E2333D for ; Thu, 7 Jan 2021 09:52:04 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727977AbhAGJwC (ORCPT ); Thu, 7 Jan 2021 04:52:02 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41116 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727957AbhAGJv7 (ORCPT ); Thu, 7 Jan 2021 04:51:59 -0500 Received: from mail-ej1-x62b.google.com (mail-ej1-x62b.google.com [IPv6:2a00:1450:4864:20::62b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E91E6C061285 for ; Thu, 7 Jan 2021 01:51:18 -0800 (PST) Received: by mail-ej1-x62b.google.com with SMTP id g20so8937642ejb.1 for ; Thu, 07 Jan 2021 01:51:18 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=rrVC3mmeskMWF5wSTd77r+DBVddBmm9itzymiAtQXiI=; b=OjypJGlPC67dx3VlsmlyqUDvXk+D0l2Kq8Q8X7rVnmYIAXzSKAKg9aGjQEjKmUt8ES stig7EyEuuSSGlqXDBNsrj78nB1uzdji2ZKXZYp+VFG1kXh0TEXCeGku2lNwmH6W4dJW Bxfb/6M4xsbRP1E2/dz68+pF3KgK2gZkSTZvf1LvtTxWbH3eCyXC6i6KO0UArVkyJOPD /M3onrvzMDmLeYvq94uNNmttArDWMp1MR3hjYKVV77SEuzSPA701rDnex8ExCy+gX0Hc TqjwjC39QwhdYbcmnTaHP1mZ5lRdR2+aU0ce/L7Fhsu0bWTQvqErg35wQWvql1/GHGDk Q+NQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=rrVC3mmeskMWF5wSTd77r+DBVddBmm9itzymiAtQXiI=; b=ubsqGE/2pmUD08OeWeclcuBW1iwtARCxVgLIt0ExEef/OaPCJWyd6jZbp8tlWdBTX0 z1gwUsM5kDCJPm9RmlL6R4ziPw7E0ZjMFoEKq0773nxEMOadfd/eLiQJaX89mHgAce6a 8qS9R9Cy0A6ZnVg4f+IhEuwcW8pBvLnJ3Y1RILNxjpM/4hQXz1ZJrVfOmavfAfXnnw58 dU2Z23Ov8ukypedXJl/kfcpgxJTb3piIAj1QTLtf+RZraXx5ubPJsBBjluCv2Gyt5BE9 6ZFzOHHIeHFhgIjlXQYxUyoCaS/W6UQSa+F9JKEEuvcAbtVoK9ywRs7xZS/2fh4UDv6H EZmQ== X-Gm-Message-State: AOAM532aNpLBjCp9icMkJk/HIgPl0MpsZ79HOb+ezXdNcYSFz7nAn/C7 JyX11dAtW2NLscPssuNXFOA= X-Google-Smtp-Source: ABdhPJweFJlAc+TwAyxWNg/q4RUJVAlDM/DU67T2hXKGSTdBVWN5RRYeoyL144RPfsxx9FnU82RuvQ== X-Received: by 2002:a17:906:f949:: with SMTP id ld9mr5817617ejb.401.1610013077659; Thu, 07 Jan 2021 01:51:17 -0800 (PST) Received: from localhost.localdomain (5-12-227-87.residential.rdsnet.ro. [5.12.227.87]) by smtp.gmail.com with ESMTPSA id k15sm2251571ejc.79.2021.01.07.01.51.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 07 Jan 2021 01:51:17 -0800 (PST) From: Vladimir Oltean To: "David S . Miller" , Jakub Kicinski Cc: netdev@vger.kernel.org, Andrew Lunn , Florian Fainelli , Cong Wang , Stephen Hemminger , Eric Dumazet , George McCollister , Oleksij Rempel , Jay Vosburgh , Veaceslav Falico , Andy Gospodarek , Arnd Bergmann , Taehee Yoo , Jiri Pirko , Florian Westphal Subject: [PATCH v3 net-next 10/12] net: bonding: ensure .ndo_get_stats64 can sleep Date: Thu, 7 Jan 2021 11:49:49 +0200 Message-Id: <20210107094951.1772183-11-olteanv@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210107094951.1772183-1-olteanv@gmail.com> References: <20210107094951.1772183-1-olteanv@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Vladimir Oltean There is an effort to convert .ndo_get_stats64 to sleepable context, and for that to work, we need to prevent callers of dev_get_stats from using atomic locking. The bonding driver retrieves its statistics recursively from its lower interfaces, with additional care to only count packets sent/received while those lowers were actually enslaved to the bond - see commit 5f0c5f73e5ef ("bonding: make global bonding stats more reliable"). Since commit 87163ef9cda7 ("bonding: remove last users of bond->lock and bond->lock itself"), the bonding driver uses the following protection for its array of slaves: RCU for readers and rtnl_mutex for updaters. This is not great because there is another movement [ somehow simultaneous with the one to make .ndo_get_stats64 sleepable ] to reduce driver usage of rtnl_mutex. This makes sense, because the rtnl_mutex has become a very contended resource. The aforementioned commit removed an interesting comment: /* [...] we can't hold bond->lock [...] because we'll * deadlock. The only solution is to rely on the fact * that we're under rtnl_lock here, and the slaves * list won't change. This doesn't solve the problem * of setting the slave's MTU while it is * transmitting, but the assumption is that the base * driver can handle that. * * TODO: figure out a way to safely iterate the slaves * list, but without holding a lock around the actual * call to the base driver. */ The above summarizes pretty well the challenges we have with nested bonding interfaces (bond over bond over bond over...), which need to be addressed by a better locking scheme that also not relies on the bloated rtnl_mutex. Instead of using something as broad as the rtnl_mutex to ensure serialization of updates to the slave array, we can reintroduce a private mutex in the bonding driver, called slaves_lock. This mutex circles the only updater, bond_update_slave_arr, and ensures that whatever other readers want to see a consistent slave array, they don't need to hold the rtnl_mutex for that. Now _of_course_ I did not convert the entire driver to use bond_for_each_slave protected by the bond->slaves_lock, and rtnl_dereference to bond_dereference. I just started that process by converting the one reader I needed: ndo_get_stats64. Not only is it nice to not hold rtnl_mutex in .ndo_get_stats64, but it is also in fact forbidden to do so (since top-level callers may hold netif_lists_lock, which is a sub-lock of the rtnl_mutex, and therefore this would cause a lock inversion and a deadlock). To solve the nesting problem, the simple way is to not hold any locks when recursing into the slave netdev operation, which is exactly the approach that we take. We can "cheat" and use dev_hold to take a reference on the slave net_device, which is enough to ensure that netdev_wait_allrefs() waits until we finish, and the kernel won't fault. However, the slave structure might no longer be valid, just its associated net_device. That isn't a biggie. We just need to do some more work to ensure that the slave exists after we took the statistics, and if it still does, reapply the logic from Andy's commit 5f0c5f73e5ef. Signed-off-by: Vladimir Oltean --- Changes in v3: - Added kfree(dev_stats); - Removed the now useless stats_lock (bond->bond_stats and slave->slave_stats are now protected by bond->slaves_lock) Changes in v2: Switched to the new scheme of holding just a refcnt to the slave interfaces while recursing with dev_get_stats. drivers/net/bonding/bond_main.c | 118 +++++++++++++++----------------- include/net/bonding.h | 49 ++++++++++++- 2 files changed, 104 insertions(+), 63 deletions(-) diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 714aa0e5d041..6e38079ea66d 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -3693,77 +3693,64 @@ static void bond_fold_stats(struct rtnl_link_stats64 *_res, } } -#ifdef CONFIG_LOCKDEP -static int bond_get_lowest_level_rcu(struct net_device *dev) -{ - struct net_device *ldev, *next, *now, *dev_stack[MAX_NEST_DEV + 1]; - struct list_head *niter, *iter, *iter_stack[MAX_NEST_DEV + 1]; - int cur = 0, max = 0; - - now = dev; - iter = &dev->adj_list.lower; - - while (1) { - next = NULL; - while (1) { - ldev = netdev_next_lower_dev_rcu(now, &iter); - if (!ldev) - break; - - next = ldev; - niter = &ldev->adj_list.lower; - dev_stack[cur] = now; - iter_stack[cur++] = iter; - if (max <= cur) - max = cur; - break; - } - - if (!next) { - if (!cur) - return max; - next = dev_stack[--cur]; - niter = iter_stack[cur]; - } - - now = next; - iter = niter; - } - - return max; -} -#endif - static void bond_get_stats(struct net_device *bond_dev, struct rtnl_link_stats64 *stats) { struct bonding *bond = netdev_priv(bond_dev); - struct rtnl_link_stats64 temp; - struct list_head *iter; - struct slave *slave; - int nest_level = 0; + struct rtnl_link_stats64 *dev_stats; + struct net_device **slaves; + int i, res, num_slaves; + res = bond_get_slave_arr(bond, &slaves, &num_slaves); + if (res) { + netdev_err(bond->dev, + "failed to allocate memory for slave array\n"); + return; + } - rcu_read_lock(); -#ifdef CONFIG_LOCKDEP - nest_level = bond_get_lowest_level_rcu(bond_dev); -#endif + dev_stats = kcalloc(num_slaves, sizeof(*dev_stats), GFP_KERNEL); + if (!dev_stats) { + netdev_err(bond->dev, + "failed to allocate memory for slave stats\n"); + bond_put_slave_arr(slaves, num_slaves); + return; + } + + /* Recurse with no locks taken */ + for (i = 0; i < num_slaves; i++) + dev_get_stats(slaves[i], &dev_stats[i]); + + /* When taking the slaves lock again, the new slave array might be + * different from the original one. + */ + mutex_lock(&bond->slaves_lock); - spin_lock_nested(&bond->stats_lock, nest_level); memcpy(stats, &bond->bond_stats, sizeof(*stats)); - bond_for_each_slave_rcu(bond, slave, iter) { - dev_get_stats(slave->dev, &temp); + for (i = 0; i < num_slaves; i++) { + struct list_head *iter; + struct slave *slave; - bond_fold_stats(stats, &temp, &slave->slave_stats); + bond_for_each_slave(bond, slave, iter) { + if (slave->dev != slaves[i]) + continue; - /* save off the slave stats for the next run */ - memcpy(&slave->slave_stats, &temp, sizeof(temp)); + bond_fold_stats(stats, &dev_stats[i], + &slave->slave_stats); + + /* save off the slave stats for the next run */ + memcpy(&slave->slave_stats, &dev_stats[i], + sizeof(dev_stats[i])); + break; + } } memcpy(&bond->bond_stats, stats, sizeof(*stats)); - spin_unlock(&bond->stats_lock); - rcu_read_unlock(); + + mutex_unlock(&bond->slaves_lock); + + kfree(dev_stats); + bond_put_slave_arr(slaves, num_slaves); } static int bond_do_ioctl(struct net_device *bond_dev, struct ifreq *ifr, int cmd) @@ -4287,11 +4274,11 @@ static void bond_set_slave_arr(struct bonding *bond, { struct bond_up_slave *usable, *all; - usable = rtnl_dereference(bond->usable_slaves); + usable = bond_dereference(bond, bond->usable_slaves); rcu_assign_pointer(bond->usable_slaves, usable_slaves); kfree_rcu(usable, rcu); - all = rtnl_dereference(bond->all_slaves); + all = bond_dereference(bond, bond->all_slaves); rcu_assign_pointer(bond->all_slaves, all_slaves); kfree_rcu(all, rcu); } @@ -4333,6 +4320,8 @@ int bond_update_slave_arr(struct bonding *bond, struct slave *skipslave) WARN_ON(lockdep_is_held(&bond->mode_lock)); #endif + mutex_lock(&bond->slaves_lock); + usable_slaves = kzalloc(struct_size(usable_slaves, arr, bond->slave_cnt), GFP_KERNEL); all_slaves = kzalloc(struct_size(all_slaves, arr, @@ -4376,17 +4365,22 @@ int bond_update_slave_arr(struct bonding *bond, struct slave *skipslave) } bond_set_slave_arr(bond, usable_slaves, all_slaves); + + mutex_unlock(&bond->slaves_lock); + return ret; out: if (ret != 0 && skipslave) { - bond_skip_slave(rtnl_dereference(bond->all_slaves), + bond_skip_slave(bond_dereference(bond, bond->all_slaves), skipslave); - bond_skip_slave(rtnl_dereference(bond->usable_slaves), + bond_skip_slave(bond_dereference(bond, bond->usable_slaves), skipslave); } kfree_rcu(all_slaves, rcu); kfree_rcu(usable_slaves, rcu); + mutex_unlock(&bond->slaves_lock); + return ret; } @@ -4699,6 +4693,7 @@ void bond_setup(struct net_device *bond_dev) { struct bonding *bond = netdev_priv(bond_dev); + mutex_init(&bond->slaves_lock); spin_lock_init(&bond->mode_lock); bond->params = bonding_defaults; @@ -5189,7 +5184,6 @@ static int bond_init(struct net_device *bond_dev) if (!bond->wq) return -ENOMEM; - spin_lock_init(&bond->stats_lock); netdev_lockdep_set_classes(bond_dev); list_add_tail(&bond->bond_list, &bn->dev_list); diff --git a/include/net/bonding.h b/include/net/bonding.h index adc3da776970..146b46d276c0 100644 --- a/include/net/bonding.h +++ b/include/net/bonding.h @@ -222,7 +222,6 @@ struct bonding { * ALB mode (6) - to sync the use and modifications of its hash table */ spinlock_t mode_lock; - spinlock_t stats_lock; u8 send_peer_notif; u8 igmp_retrans; #ifdef CONFIG_PROC_FS @@ -249,6 +248,11 @@ struct bonding { #ifdef CONFIG_XFRM_OFFLOAD struct xfrm_state *xs; #endif /* CONFIG_XFRM_OFFLOAD */ + + /* Protects the slave array. TODO: convert all instances of + * rtnl_dereference to bond_dereference + */ + struct mutex slaves_lock; }; #define bond_slave_get_rcu(dev) \ @@ -257,6 +261,9 @@ struct bonding { #define bond_slave_get_rtnl(dev) \ ((struct slave *) rtnl_dereference(dev->rx_handler_data)) +#define bond_dereference(bond, p) \ + rcu_dereference_protected(p, lockdep_is_held(&(bond)->slaves_lock)) + void bond_queue_slave_event(struct slave *slave); void bond_lower_state_changed(struct slave *slave); @@ -449,6 +456,46 @@ static inline void bond_hw_addr_copy(u8 *dst, const u8 *src, unsigned int len) memcpy(dst, src, len); } +static inline int bond_get_slave_arr(struct bonding *bond, + struct net_device ***slaves, + int *num_slaves) +{ + struct net *net = dev_net(bond->dev); + struct list_head *iter; + struct slave *slave; + int i = 0; + + mutex_lock(&bond->slaves_lock); + + *slaves = kcalloc(bond->slave_cnt, sizeof(*slaves), GFP_KERNEL); + if (!(*slaves)) { + netif_lists_unlock(net); + return -ENOMEM; + } + + bond_for_each_slave(bond, slave, iter) { + dev_hold(slave->dev); + *slaves[i++] = slave->dev; + } + + *num_slaves = bond->slave_cnt; + + mutex_unlock(&bond->slaves_lock); + + return 0; +} + +static inline void bond_put_slave_arr(struct net_device **slaves, + int num_slaves) +{ + int i; + + for (i = 0; i < num_slaves; i++) + dev_put(slaves[i]); + + kfree(slaves); +} + #define BOND_PRI_RESELECT_ALWAYS 0 #define BOND_PRI_RESELECT_BETTER 1 #define BOND_PRI_RESELECT_FAILURE 2 From patchwork Thu Jan 7 09:49:50 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Vladimir Oltean X-Patchwork-Id: 358720 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id EEF4FC433E9 for ; Thu, 7 Jan 2021 09:52:33 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C1C0D2333D for ; Thu, 7 Jan 2021 09:52:33 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727997AbhAGJwI (ORCPT ); Thu, 7 Jan 2021 04:52:08 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41114 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727959AbhAGJv7 (ORCPT ); Thu, 7 Jan 2021 04:51:59 -0500 Received: from mail-ej1-x632.google.com (mail-ej1-x632.google.com [IPv6:2a00:1450:4864:20::632]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 56FACC061290 for ; Thu, 7 Jan 2021 01:51:20 -0800 (PST) Received: by mail-ej1-x632.google.com with SMTP id jx16so8786052ejb.10 for ; Thu, 07 Jan 2021 01:51:20 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=s6/KSwpYaFicCBOmHGp8olwhxQfTeT9BvGFmL3TZtU0=; b=k4srG0hUz3ymRjl2O61v8y2Ij5FqmVzgBT99dg0biEI3M4W0wvYtyYCLMwYQjVwUfC 9fHUe/YR5e3Z1YoGzKYkZL7vWG+Qh4JLZvUiNX3jCVvdzRm/4IxEoU82qWt9pXJeTQwJ 5HCznBH1nGj9+ZAvcTuw97BQ4LBHCYWgN2dvBBqwTWkdmLg0UI3jl0F96tQ+xnGo6LTA 2l8GMxiEGeFt/f61qg8eCOeanBolv6R1aGsAzgCOekniV7s+JB3+o3wWHP3GXaRwWTGm HSHczR9dBbfdNSw/OYHZ87J84D3JtnJShvm8Juk+K1GMUHDyG5jtNxCObpCWxcTtG6cL ubsQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=s6/KSwpYaFicCBOmHGp8olwhxQfTeT9BvGFmL3TZtU0=; b=nwHYXI5swf3oAyq/NqYyGbHu5pXWTJ5Q18sHD2UxQ4gMM1S3PC/+HFWkpTn/bXM5a/ Ca2ZrXbgTTuGDE5CGPDkzt+2aY8oYjHyNnoA+olpsfGZcDytlHecoLV7O37xFabiFxGU m4CPcH1jvnXkVlQUR3PLXuiye6nNpj3C7tAmfcd2s0/2ievYzKiPpadRdy+iVoOHPaFO 1j3Gs3ScrZkH0wtFpccP5OxvJk1e72c/zJZ9Ri8UsZ8C6euIQWeebtISsYcuPVjE6QkQ yiDWNCJ0o/8qOVd81qTbDX3q9iDjXfpK64vScq++fjcFz5NwkHIDW9XYwZCK/rsE381R HwLA== X-Gm-Message-State: AOAM5302pzR2+0vPZzRy+CAnCd9kRTBpN3hyMwHlh+tnR98Nj1hEFp2T DqS2lce09dVjJnocvdh+gs0= X-Google-Smtp-Source: ABdhPJwMHXEemzYTsIss0OBGFMrA2zcrhE44kmejdeBnjaIpJ1sAnBL6wgsSDptH8pFcr6GKddrmPA== X-Received: by 2002:a17:906:2499:: with SMTP id e25mr5713103ejb.446.1610013079060; Thu, 07 Jan 2021 01:51:19 -0800 (PST) Received: from localhost.localdomain (5-12-227-87.residential.rdsnet.ro. [5.12.227.87]) by smtp.gmail.com with ESMTPSA id k15sm2251571ejc.79.2021.01.07.01.51.17 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 07 Jan 2021 01:51:18 -0800 (PST) From: Vladimir Oltean To: "David S . Miller" , Jakub Kicinski Cc: netdev@vger.kernel.org, Andrew Lunn , Florian Fainelli , Cong Wang , Stephen Hemminger , Eric Dumazet , George McCollister , Oleksij Rempel , Jay Vosburgh , Veaceslav Falico , Andy Gospodarek , Arnd Bergmann , Taehee Yoo , Jiri Pirko , Florian Westphal Subject: [PATCH v3 net-next 11/12] net: mark ndo_get_stats64 as being able to sleep Date: Thu, 7 Jan 2021 11:49:50 +0200 Message-Id: <20210107094951.1772183-12-olteanv@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210107094951.1772183-1-olteanv@gmail.com> References: <20210107094951.1772183-1-olteanv@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Vladimir Oltean Now that all callers have been converted to not use atomic context when calling dev_get_stats, it is time to update the documentation and put a notice in the function that it expects process context. Signed-off-by: Vladimir Oltean --- Changes in v3: None. Changes in v2: Updated the documentation. Documentation/networking/netdevices.rst | 8 ++++++-- Documentation/networking/statistics.rst | 9 ++++----- net/core/dev.c | 2 ++ 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/Documentation/networking/netdevices.rst b/Documentation/networking/netdevices.rst index 5a85fcc80c76..944599722c76 100644 --- a/Documentation/networking/netdevices.rst +++ b/Documentation/networking/netdevices.rst @@ -64,8 +64,12 @@ ndo_do_ioctl: Context: process ndo_get_stats: - Synchronization: dev_base_lock rwlock. - Context: nominally process, but don't sleep inside an rwlock + Synchronization: + none. netif_lists_lock(net) might be held, but not guaranteed. + It is illegal to hold rtnl_lock() in this method, since it will + cause a lock inversion with netif_lists_lock and a deadlock. + Context: + process ndo_start_xmit: Synchronization: __netif_tx_lock spinlock. diff --git a/Documentation/networking/statistics.rst b/Documentation/networking/statistics.rst index 234abedc29b2..ad3e353df0dd 100644 --- a/Documentation/networking/statistics.rst +++ b/Documentation/networking/statistics.rst @@ -155,11 +155,10 @@ Drivers must ensure best possible compliance with Please note for example that detailed error statistics must be added into the general `rx_error` / `tx_error` counters. -The `.ndo_get_stats64` callback can not sleep because of accesses -via `/proc/net/dev`. If driver may sleep when retrieving the statistics -from the device it should do so periodically asynchronously and only return -a recent copy from `.ndo_get_stats64`. Ethtool interrupt coalescing interface -allows setting the frequency of refreshing statistics, if needed. +Drivers may sleep when retrieving the statistics from the device, or they might +read the counters periodically and only return in `.ndo_get_stats64` a recent +copy collected asynchronously. In the latter case, the ethtool interrupt +coalescing interface allows setting the frequency of refreshing statistics. Retrieving ethtool statistics is a multi-syscall process, drivers are advised to keep the number of statistics constant to avoid race conditions with diff --git a/net/core/dev.c b/net/core/dev.c index 93618300ac90..ccb31a52e514 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -10407,6 +10407,8 @@ void dev_get_stats(struct net_device *dev, struct rtnl_link_stats64 *storage) { const struct net_device_ops *ops = dev->netdev_ops; + might_sleep(); + if (ops->ndo_get_stats64) { memset(storage, 0, sizeof(*storage)); ops->ndo_get_stats64(dev, storage); From patchwork Thu Jan 7 09:49:51 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Vladimir Oltean X-Patchwork-Id: 358721 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI, SPF_HELO_NONE, SPF_PASS, USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id BAFBDC433E0 for ; Thu, 7 Jan 2021 09:52:33 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 7FE0D2333E for ; Thu, 7 Jan 2021 09:52:33 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727983AbhAGJwF (ORCPT ); Thu, 7 Jan 2021 04:52:05 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41020 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727962AbhAGJv7 (ORCPT ); Thu, 7 Jan 2021 04:51:59 -0500 Received: from mail-ej1-x62a.google.com (mail-ej1-x62a.google.com [IPv6:2a00:1450:4864:20::62a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C5677C0612A0 for ; Thu, 7 Jan 2021 01:51:21 -0800 (PST) Received: by mail-ej1-x62a.google.com with SMTP id 6so8858495ejz.5 for ; Thu, 07 Jan 2021 01:51:21 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=PoqSBxPScrFd49HRen6PJJlMx5D97p4DjodtTjMvHqQ=; b=jA8DAiV6jAxFdO+Z1RFOzWlPl5Sk+pWfHmlHbcan+f98fA9EEYh1Do5QkWSXkgVezP CU5+Q1N25frOSbI+Dy8Wu87ML8BCiYcJouNDoklQStXBlXPxGYGXMnJf5VQQJgSYRRxh SE9dzzGXyC2dCQcJHFWOi3Ymu3K9czsgHBQKs5/aq7LHScl64nvgLgUAAQ1Bv/6EZWCW 5FQKCmvfq+7ESziaRgLDMPVxp2yjNO++RZKVi12omSfhKiYJlYONL2ZeO+Yr8mnqMJZG y27w8muhGy4iaXtcvaglsdAryEO5qvTWr6g/cun95uCxZR2vc/mNpc3FOTdZA90Ooun0 zprQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=PoqSBxPScrFd49HRen6PJJlMx5D97p4DjodtTjMvHqQ=; b=sDpYUr6Dg8f7A4583PuT6fy4/zeRaSbdgOBiQpq1rKFuJIRsXneO3BBwni965Blr+q ZxyJfvPJy0Hmy5s1Ue6Ws9oCONS+hQlCCyG2+bRh9uxD5C+qiZMUx3S7chwkHbH5uYQI KwgzjtbwFtaKKJkkffF812MocphX/lsv+i7noxmHoDk6aDrlHWIfUQG/MQG3BOuzfyhm 0dujC8Q3amffy4xUoPD6hE07OTEs3X2y7d6Juh8KFocBDOz6BXgM/+O+A8Ovd3/5NAWq WHCObCygcoYQnT90ia2hG9qnd13fUdhc2+EtOIff9P5R37+24s2h6bbfe5Q13QTC4W0P HdLQ== X-Gm-Message-State: AOAM5310muhZ7Czeu107eKFMud3RHZqbxJOay039PquO4RGoDIC/clSr 2cAwRHL4t+8j/DarCYnw4Oo= X-Google-Smtp-Source: ABdhPJycVd91GlMHzPs3txSIm1KLwT5PWdTw1HNI7IVLvJhk8O6RUIr3t5ZQc9SmDvwLpN8cQorjng== X-Received: by 2002:a17:906:90d6:: with SMTP id v22mr5801709ejw.88.1610013080557; Thu, 07 Jan 2021 01:51:20 -0800 (PST) Received: from localhost.localdomain (5-12-227-87.residential.rdsnet.ro. [5.12.227.87]) by smtp.gmail.com with ESMTPSA id k15sm2251571ejc.79.2021.01.07.01.51.19 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 07 Jan 2021 01:51:20 -0800 (PST) From: Vladimir Oltean To: "David S . Miller" , Jakub Kicinski Cc: netdev@vger.kernel.org, Andrew Lunn , Florian Fainelli , Cong Wang , Stephen Hemminger , Eric Dumazet , George McCollister , Oleksij Rempel , Jay Vosburgh , Veaceslav Falico , Andy Gospodarek , Arnd Bergmann , Taehee Yoo , Jiri Pirko , Florian Westphal Subject: [PATCH v3 net-next 12/12] net: remove obsolete comments about ndo_get_stats64 context from eth drivers Date: Thu, 7 Jan 2021 11:49:51 +0200 Message-Id: <20210107094951.1772183-13-olteanv@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210107094951.1772183-1-olteanv@gmail.com> References: <20210107094951.1772183-1-olteanv@gmail.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org From: Vladimir Oltean Now that we have a good summary in Documentation/networking/netdevices.rst, these comments serve no purpose and are actually distracting/confusing. Signed-off-by: Vladimir Oltean --- Changes in v3: None. Changes in v2: None. drivers/net/ethernet/cisco/enic/enic_main.c | 1 - drivers/net/ethernet/nvidia/forcedeth.c | 2 -- drivers/net/ethernet/sfc/efx_common.c | 1 - drivers/net/ethernet/sfc/falcon/efx.c | 1 - 4 files changed, 5 deletions(-) diff --git a/drivers/net/ethernet/cisco/enic/enic_main.c b/drivers/net/ethernet/cisco/enic/enic_main.c index fb269d587b74..7425f94f9091 100644 --- a/drivers/net/ethernet/cisco/enic/enic_main.c +++ b/drivers/net/ethernet/cisco/enic/enic_main.c @@ -870,7 +870,6 @@ static netdev_tx_t enic_hard_start_xmit(struct sk_buff *skb, return NETDEV_TX_OK; } -/* dev_base_lock rwlock held, nominally process context */ static void enic_get_stats(struct net_device *netdev, struct rtnl_link_stats64 *net_stats) { diff --git a/drivers/net/ethernet/nvidia/forcedeth.c b/drivers/net/ethernet/nvidia/forcedeth.c index 8724d6a9ed02..8fa254dc64e9 100644 --- a/drivers/net/ethernet/nvidia/forcedeth.c +++ b/drivers/net/ethernet/nvidia/forcedeth.c @@ -1761,8 +1761,6 @@ static void nv_get_stats(int cpu, struct fe_priv *np, /* * nv_get_stats64: dev->ndo_get_stats64 function * Get latest stats value from the nic. - * Called with read_lock(&dev_base_lock) held for read - - * only synchronized against unregister_netdevice. */ static void nv_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *storage) diff --git a/drivers/net/ethernet/sfc/efx_common.c b/drivers/net/ethernet/sfc/efx_common.c index de797e1ac5a9..4d8047b35fb2 100644 --- a/drivers/net/ethernet/sfc/efx_common.c +++ b/drivers/net/ethernet/sfc/efx_common.c @@ -596,7 +596,6 @@ void efx_stop_all(struct efx_nic *efx) efx_stop_datapath(efx); } -/* Context: process, dev_base_lock or RTNL held, non-blocking. */ void efx_net_stats(struct net_device *net_dev, struct rtnl_link_stats64 *stats) { struct efx_nic *efx = netdev_priv(net_dev); diff --git a/drivers/net/ethernet/sfc/falcon/efx.c b/drivers/net/ethernet/sfc/falcon/efx.c index f8979991970e..6db2b6583dec 100644 --- a/drivers/net/ethernet/sfc/falcon/efx.c +++ b/drivers/net/ethernet/sfc/falcon/efx.c @@ -2096,7 +2096,6 @@ int ef4_net_stop(struct net_device *net_dev) return 0; } -/* Context: process, dev_base_lock or RTNL held, non-blocking. */ static void ef4_net_stats(struct net_device *net_dev, struct rtnl_link_stats64 *stats) {