Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
MIME-Version: 1.0
References: <20220518062715.27809-1-zhoufeng.zf@bytedance.com>
 <CAADnVQ+x-A87Z9_c+3vuRJOYm=gCOBXmyCJQ64CiCNukHS6FpA@mail.gmail.com>
 <6ae715b3-96b1-2b42-4d1a-5267444d586b@bytedance.com> <9c0c3e0b-33bc-51a7-7916-7278f14f308e@fb.com>
 <380fa11e-f15d-da1a-51f7-70e14ed58ffc@bytedance.com>
In-Reply-To: <380fa11e-f15d-da1a-51f7-70e14ed58ffc@bytedance.com>
From:   Alexei Starovoitov <alexei.starovoitov@gmail.com>
Date:   Thu, 19 May 2022 09:12:04 -0700
Message-ID: <CAADnVQL9naBBKzQdAOWu2ZH=i7HA1VDi7uNzsDQ1TM9Jr+c0Ww@mail.gmail.com>
Subject: Re: [External] Re: [PATCH] bpf: avoid grabbing spin_locks of all cpus
 when no free elems
To:     Feng Zhou <zhoufeng.zf@bytedance.com>
Cc:     Yonghong Song <yhs@fb.com>, Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Andrii Nakryiko <andrii@kernel.org>,
        Martin KaFai Lau <kafai@fb.com>,
        Song Liu <songliubraving@fb.com>,
        John Fastabend <john.fastabend@gmail.com>,
        KP Singh <kpsingh@kernel.org>,
        Network Development <netdev@vger.kernel.org>,
        bpf <bpf@vger.kernel.org>, LKML <linux-kernel@vger.kernel.org>,
        Xiongchun Duan <duanxiongchun@bytedance.com>,
        Muchun Song <songmuchun@bytedance.com>,
        Dongdong Wang <wangdongdong.6@bytedance.com>,
        Cong Wang <cong.wang@bytedance.com>,
        Chengming Zhou <zhouchengming@bytedance.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Wed, May 18, 2022 at 8:12 PM Feng Zhou <zhoufeng.zf@bytedance.com> wrote=
:
>
> =E5=9C=A8 2022/5/19 =E4=B8=8A=E5=8D=884:39, Yonghong Song =E5=86=99=E9=81=
=93:
> >
> >
> > On 5/17/22 11:57 PM, Feng Zhou wrote:
> >> =E5=9C=A8 2022/5/18 =E4=B8=8B=E5=8D=882:32, Alexei Starovoitov =E5=86=
=99=E9=81=93:
> >>> On Tue, May 17, 2022 at 11:27 PM Feng zhou
> >>> <zhoufeng.zf@bytedance.com> wrote:
> >>>> From: Feng Zhou <zhoufeng.zf@bytedance.com>
> >>>>
> >>>> We encountered bad case on big system with 96 CPUs that
> >>>> alloc_htab_elem() would last for 1ms. The reason is that after the
> >>>> prealloc hashtab has no free elems, when trying to update, it will
> >>>> still
> >>>> grab spin_locks of all cpus. If there are multiple update users, the
> >>>> competition is very serious.
> >>>>
> >>>> So this patch add is_empty in pcpu_freelist_head to check freelist
> >>>> having free or not. If having, grab spin_lock, or check next cpu's
> >>>> freelist.
> >>>>
> >>>> Before patch: hash_map performance
> >>>> ./map_perf_test 1
> >
> > could you explain what parameter '1' means here?
>
> This code is here:
> samples/bpf/map_perf_test_user.c
> samples/bpf/map_perf_test_kern.c
> parameter '1' means testcase flag, test hash_map's performance
> parameter '2048' means test hash_map's performance when free=3D0.
> testcase flag '2048' is added by myself to reproduce the problem phenomen=
on.

Please convert it to selftests/bpf/bench,
so that everyone can reproduce the issue you're seeing
and can assess whether it's a real issue or a corner case.

Also please avoid adding indent in the patch.
Instead of
 if (!s->extralist.is_empty) {
  .. churn

do

 if (s->extralist.is_empty)