Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200917234953.CB1D295C0A69@us180.sjc.aristanetworks.com> <CANn89iJCm9Rw2U1bK9hAQAzdwebggsWh0DFkHpJF=4OZ2JiSOw@mail.gmail.com>
In-Reply-To: <CANn89iJCm9Rw2U1bK9hAQAzdwebggsWh0DFkHpJF=4OZ2JiSOw@mail.gmail.com>
From:   Eric Dumazet <edumazet@google.com>
Date:   Fri, 18 Sep 2020 11:00:02 +0200
Message-ID: <CANn89iJebrj+cE1iVFbFQH0Bso3EFw8Bw0Ep8ozikS_5Ldu2oA@mail.gmail.com>
Subject: Re: [PATCH v3] net: use exponential backoff in netdev_wait_allrefs
To:     Francesco Ruggeri <fruggeri@arista.com>
Cc:     LKML <linux-kernel@vger.kernel.org>,
        netdev <netdev@vger.kernel.org>,
        Cong Wang <xiyou.wangcong@gmail.com>,
        Taehee Yoo <ap420073@gmail.com>,
        Andrii Nakryiko <andriin@fb.com>,
        Jiri Pirko <jiri@mellanox.com>,
        Alexei Starovoitov <ast@kernel.org>,
        Jakub Kicinski <kuba@kernel.org>,
        David Miller <davem@davemloft.net>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, Sep 18, 2020 at 10:48 AM Eric Dumazet <edumazet@google.com> wrote:

>
>
> Also, I would try using synchronize_rcu() instead of the first

s/synchronize_rcu/rcu_barrier/  of course :/

> msleep(), this might avoid all msleep() calls in your case.
>
> Patch without the macros to see the general idea :
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 266073e300b5fc21440ea8f8ffc9306a1fc9f370..2d3b65034bc0dd99017dea846e6c0a966f1207ee
> 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -9989,7 +9989,7 @@ EXPORT_SYMBOL(netdev_refcnt_read);
>  static void netdev_wait_allrefs(struct net_device *dev)
>  {
>         unsigned long rebroadcast_time, warning_time;
> -       int refcnt;
> +       int wait = 0, refcnt;
>
>         linkwatch_forget_dev(dev);
>
> @@ -10023,8 +10023,13 @@ static void netdev_wait_allrefs(struct net_device *dev)
>                         rebroadcast_time = jiffies;
>                 }
>
> -               msleep(250);
> -
> +               if (!wait) {
> +                       synchronize_rcu();

rcu_barrier();

> +                       wait = 1;
> +               } else {
> +                       msleep(wait);
> +                       wait = min(wait << 1, 250);
> +               }
>                 refcnt = netdev_refcnt_read(dev);
>
>                 if (refcnt && time_after(jiffies, warning_time + 10 * HZ)) {