From mboxrd@z Thu Jan 1 00:00:00 1970 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=-12.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 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 08F48C433E6 for ; Mon, 8 Feb 2021 22:22:17 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id CFCA164E77 for ; Mon, 8 Feb 2021 22:22:16 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229691AbhBHWWB (ORCPT ); Mon, 8 Feb 2021 17:22:01 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59680 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230265AbhBHWVv (ORCPT ); Mon, 8 Feb 2021 17:21:51 -0500 Received: from mail-il1-x133.google.com (mail-il1-x133.google.com [IPv6:2607:f8b0:4864:20::133]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id AA349C061788 for ; Mon, 8 Feb 2021 14:21:10 -0800 (PST) Received: by mail-il1-x133.google.com with SMTP id m20so14301484ilj.13 for ; Mon, 08 Feb 2021 14:21:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=HA2/imsxse4+Wvfox3XugjZfAuFpdA1Zz1Po5JcNkf8=; b=kuHWevKVQ5uQs6/PaYf7m99CJuGUt/t0sf6LRl8S310BzRqS8fbtyxFM/kY6tNa0Br 2mMUQhrkEwaftOItzVmIc+uyIj3+5QPw6VkXwULkYvMzZSqHh1GI2gDvchvuCEfR4eAc v9p+YpHeeWa+Cc6N3ck+IbDXGp3nqxAbb7i7AmPH6SGD8NhJFXbppLsXwZ/zr6m92OtE G2SlDL2UoH1e+7npeYiyNi/znmtEWPnqo9Wy6rBtLS6x3z+y/eFY94/I60hT944PC0km Eg7mcCjdTjQ2l+jalBBgP6GyBCxA8Lut93J7YPjTnCQ3QH7w+LNkCZ4QKIgH5tnwoKzE QGnA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=HA2/imsxse4+Wvfox3XugjZfAuFpdA1Zz1Po5JcNkf8=; b=F6+/b0yni6mbhcO2uNJytbhdgw+E6I/bKH/5krEAwW3KoEz6Ceb+GCaJyOyzg7ZpWS p2TM9Fy34g88DnGFZ5C8mKUuvMMvewS31CdY8wzfxePmMiOT7rc+DfnQ3pkgcd82jB/2 mmfvw9GZXtetHPlq8ddUY3j+QMexP3LuPeDftMN7tFvGCXz1hYpHezzZyxTUSMEDBGAt LseePub5U8xTjlWrOs1SS8l1WWs2/8VaV54CEqFb0OVM8Jo0dICvaUmsCjnQxl0hvCqf pbOvTdiVI7pgWDbEYvHlmAGTAvaZyWg7rxPL02AF8Pvul3etNCZtFwcFgr2iJsh8z0R8 uu0g== X-Gm-Message-State: AOAM532TJmnD9Nbyp3ALGMd9+SItKqEump+3t1Y+McSlJaXWWrLnXF4p zqZRDh6grAiKlNwHP5XSX5JhfjXEX+dWFFvh2wBDun9//Dc= X-Google-Smtp-Source: ABdhPJwpOQLGS5RhV6XBVXBHy5R0MJgNBpYPdXg04WaLHrHZYgFjCWVi7hAOc9dqNM0twiv9FbYqZ7TkP6RCNjncpyc= X-Received: by 2002:a05:6e02:2196:: with SMTP id j22mr17410953ila.64.1612822870054; Mon, 08 Feb 2021 14:21:10 -0800 (PST) MIME-Version: 1.0 References: <20210208171917.1088230-1-atenart@kernel.org> <20210208171917.1088230-10-atenart@kernel.org> In-Reply-To: <20210208171917.1088230-10-atenart@kernel.org> From: Alexander Duyck Date: Mon, 8 Feb 2021 14:20:58 -0800 Message-ID: Subject: Re: [PATCH net-next v2 09/12] net-sysfs: remove the rtnl lock when accessing the xps maps To: Antoine Tenart Cc: David Miller , Jakub Kicinski , Netdev Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Mon, Feb 8, 2021 at 9:19 AM Antoine Tenart wrote: > > Now that nr_ids and num_tc are stored in the xps dev_maps, which are RCU > protected, we do not have the need to protect the xps_cpus_show and > xps_rxqs_show functions with the rtnl lock. > > Signed-off-by: Antoine Tenart > --- > net/core/net-sysfs.c | 26 ++++---------------------- > 1 file changed, 4 insertions(+), 22 deletions(-) > > diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c > index c2276b589cfb..6ce5772e799e 100644 > --- a/net/core/net-sysfs.c > +++ b/net/core/net-sysfs.c > @@ -1328,17 +1328,12 @@ static ssize_t xps_cpus_show(struct netdev_queue *queue, > > index = get_netdev_queue_index(queue); > > - if (!rtnl_trylock()) > - return restart_syscall(); > - > /* If queue belongs to subordinate dev use its map */ > dev = netdev_get_tx_queue(dev, index)->sb_dev ? : dev; > > tc = netdev_txq_to_tc(dev, index); > - if (tc < 0) { > - ret = -EINVAL; > - goto err_rtnl_unlock; > - } > + if (tc < 0) > + return -EINVAL; > > rcu_read_lock(); > dev_maps = rcu_dereference(dev->xps_maps[XPS_CPUS]); So I think we hit a snag here. The sb_dev pointer is protected by the rtnl_lock. So I don't think we can release the rtnl_lock until after we are done with the dev pointer. Also I am not sure it is safe to use netdev_txq_to_tc without holding the lock. I don't know if we ever went through and guaranteed that it will always work if the lock isn't held since in theory the device could reprogram all the map values out from under us. Odds are we should probably fix traffic_class_show as I suspect it probably also needs to be holding the rtnl_lock to prevent any possible races. I'll submit a patch for that. > @@ -1371,16 +1366,12 @@ static ssize_t xps_cpus_show(struct netdev_queue *queue, > out_no_maps: > rcu_read_unlock(); > > - rtnl_unlock(); > - > len = bitmap_print_to_pagebuf(false, buf, mask, nr_ids); > bitmap_free(mask); > return len < PAGE_SIZE ? len : -EINVAL; > > err_rcu_unlock: > rcu_read_unlock(); > -err_rtnl_unlock: > - rtnl_unlock(); > return ret; > } > > @@ -1435,14 +1426,9 @@ static ssize_t xps_rxqs_show(struct netdev_queue *queue, char *buf) > > index = get_netdev_queue_index(queue); > > - if (!rtnl_trylock()) > - return restart_syscall(); > - > tc = netdev_txq_to_tc(dev, index); > - if (tc < 0) { > - ret = -EINVAL; > - goto err_rtnl_unlock; > - } > + if (tc < 0) > + return -EINVAL; > > rcu_read_lock(); > dev_maps = rcu_dereference(dev->xps_maps[XPS_RXQS]); > @@ -1475,8 +1461,6 @@ static ssize_t xps_rxqs_show(struct netdev_queue *queue, char *buf) > out_no_maps: > rcu_read_unlock(); > > - rtnl_unlock(); > - > len = bitmap_print_to_pagebuf(false, buf, mask, nr_ids); > bitmap_free(mask); > > @@ -1484,8 +1468,6 @@ static ssize_t xps_rxqs_show(struct netdev_queue *queue, char *buf) > > err_rcu_unlock: > rcu_read_unlock(); > -err_rtnl_unlock: > - rtnl_unlock(); > return ret; > } > > -- > 2.29.2 >