From mboxrd@z Thu Jan  1 00:00:00 1970
From: Cong Wang <xiyou.wangcong@gmail.com>
Subject: Re: [PATCH net-next v6 10/11] net: sched: atomically check-allocate action
Date: Fri, 10 Aug 2018 14:45:24 -0700
Message-ID: <CAM_iQpW2GAFY84K9pDjcXiRB9VVYKdEHBU6mGu83hLyexahaqw@mail.gmail.com>
References: <1530800673-12280-1-git-send-email-vladbu@mellanox.com>
 <1530800673-12280-11-git-send-email-vladbu@mellanox.com> <CAM_iQpU+tWo4HVmQV+UCcLCmULd=kfsPZg+M+2rVvMbHN=2jww@mail.gmail.com>
 <vbfpnyt2hbw.fsf@reg-r-vrt-018-180.mtr.labs.mlnx> <CAM_iQpU=+gSrmV5+bSekr5SD+PgrYHN6YXsLpRC=_p09m0a0ew@mail.gmail.com>
 <vbfeff6bjm0.fsf@reg-r-vrt-018-180.mtr.labs.mlnx>
Mime-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Cc: Linux Kernel Network Developers <netdev@vger.kernel.org>,
        David Miller <davem@davemloft.net>,
        Jamal Hadi Salim <jhs@mojatatu.com>,
        Jiri Pirko <jiri@resnulli.us>,
        Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Yevgeny Kliteynik <kliteyn@mellanox.com>,
        Jiri Pirko <jiri@mellanox.com>
To: Vlad Buslov <vladbu@mellanox.com>
Return-path: <netdev-owner@vger.kernel.org>
Received: from mail-pf1-f195.google.com ([209.85.210.195]:40899 "EHLO
        mail-pf1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1726708AbeHKAQy (ORCPT
        <rfc822;netdev@vger.kernel.org>); Fri, 10 Aug 2018 20:16:54 -0400
Received: by mail-pf1-f195.google.com with SMTP id e13-v6so5095797pff.7
        for <netdev@vger.kernel.org>; Fri, 10 Aug 2018 14:45:14 -0700 (PDT)
In-Reply-To: <vbfeff6bjm0.fsf@reg-r-vrt-018-180.mtr.labs.mlnx>
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

On Fri, Aug 10, 2018 at 3:29 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>
> Approach you suggest is valid, but has its own trade-offs:
>
> - As you noted, lock granularity becomes coarse-grained due to per-netns
> scope.

Sure, you acquire idrinfo->lock too, the only difference is how long
you take it.

The bottleneck of your approach is the same, also you take idrinfo->lock
twice, so the contention is heavier.


>
> - I am not sure it is possible to call idr_replace() without obtaining
> idrinfo->lock in this particular case. Concurrent delete of action with
> same id is possible and, according to idr_replace() description,
> unlocked execution is not supported for such use-case:

But we can hold its refcnt before releasing idrinfo->lock, so
idr_replace() can't race with concurrent delete.


>
> - High rate or replace request will generate a lot of unnecessary memory
> allocations and deallocations.
>

Yes, this is literally how RCU works, always allocate and copy,
release upon error.

Also, if this is really a problem, we have SLAB_TYPESAFE_BY_RCU
too. ;)