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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 788FDC4332F for ; Fri, 11 Nov 2022 07:00:51 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233002AbiKKHAu (ORCPT ); Fri, 11 Nov 2022 02:00:50 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43402 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231167AbiKKHAr (ORCPT ); Fri, 11 Nov 2022 02:00:47 -0500 Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [45.249.212.188]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4DFCB65865; Thu, 10 Nov 2022 23:00:45 -0800 (PST) Received: from kwepemi500013.china.huawei.com (unknown [172.30.72.57]) by szxga02-in.huawei.com (SkyGuard) with ESMTP id 4N7qN10zY6zHtk9; Fri, 11 Nov 2022 15:00:17 +0800 (CST) Received: from [10.67.111.192] (10.67.111.192) by kwepemi500013.china.huawei.com (7.221.188.120) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.31; Fri, 11 Nov 2022 15:00:42 +0800 Message-ID: Date: Fri, 11 Nov 2022 15:00:42 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.9.0 Subject: Re: [PATCH bpf-next v4] bpf: Initialize same number of free nodes for each pcpu_freelist Content-Language: en-US To: wuqiang , , CC: Alexei Starovoitov , Daniel Borkmann , Andrii Nakryiko , Martin KaFai Lau , Song Liu , Yonghong Song , John Fastabend , KP Singh , Stanislav Fomichev , Hao Luo , Jiri Olsa References: <20221110122128.105214-1-xukuohai@huawei.com> <83161e5e-5aa4-acdc-630b-95274bfb47d3@bytedance.com> From: Xu Kuohai In-Reply-To: <83161e5e-5aa4-acdc-630b-95274bfb47d3@bytedance.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.111.192] X-ClientProxiedBy: dggems705-chm.china.huawei.com (10.3.19.182) To kwepemi500013.china.huawei.com (7.221.188.120) X-CFilter-Loop: Reflected Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/11/2022 11:53 AM, wuqiang wrote: > On 2022/11/10 20:21, Xu Kuohai wrote: >> pcpu_freelist_populate() initializes nr_elems / num_possible_cpus() + 1 >> free nodes for some CPUs, and then possibly one CPU with fewer nodes, >> followed by remaining cpus with 0 nodes. For example, when nr_elems == 256 >> and num_possible_cpus() == 32, CPU 0~27 each gets 9 free nodes, CPU 28 gets >> 4 free nodes, CPU 29~31 get 0 free nodes, while in fact each CPU should get >> 8 nodes equally. >> >> This patch initializes nr_elems / num_possible_cpus() free nodes for each >> CPU firstly, then allocates the remaining free nodes by one for each CPU >> until no free nodes left. >> >> Signed-off-by: Xu Kuohai >> Acked-by: Yonghong Song >> --- >> v4: Remove unneeded min() >> v3: Simplify code as suggested by Andrii >> v2: Update commit message and add Yonghong's ack >> --- >>   kernel/bpf/percpu_freelist.c | 26 +++++++++++++------------- >>   1 file changed, 13 insertions(+), 13 deletions(-) >> >> diff --git a/kernel/bpf/percpu_freelist.c b/kernel/bpf/percpu_freelist.c >> index b6e7f5c5b9ab..27f2c4aff623 100644 >> --- a/kernel/bpf/percpu_freelist.c >> +++ b/kernel/bpf/percpu_freelist.c >> @@ -100,22 +100,22 @@ void pcpu_freelist_populate(struct pcpu_freelist *s, void *buf, u32 elem_size, >>                   u32 nr_elems) >>   { >>       struct pcpu_freelist_head *head; >> -    int i, cpu, pcpu_entries; >> +    unsigned int cpu, cpu_idx, i, j, n, m; >> -    pcpu_entries = nr_elems / num_possible_cpus() + 1; >> -    i = 0; >> +    n = nr_elems / num_possible_cpus(); >> +    m = nr_elems % num_possible_cpus(); >> + >> +    cpu_idx = 0; >>       for_each_possible_cpu(cpu) { >> -again: >> -        head = per_cpu_ptr(s->freelist, cpu); >> -        /* No locking required as this is not visible yet. */ >> -        pcpu_freelist_push_node(head, buf); >> -        i++; >> -        buf += elem_size; >> -        if (i == nr_elems) >> -            break; >> -        if (i % pcpu_entries) >> -            goto again; >> +        j = n + (cpu_idx < m ? 1 : 0); >> +        for (i = 0; i < j; i++) { >> +            head = per_cpu_ptr(s->freelist, cpu); > > Better move it out of "i-loop", OK, will do > and rename "j" to a meaningful name to avoid > possible misuse. > The loop is short enough to be readable and "j" is not used elsewhere, so I think it's good to keep the name simple. >> +            /* No locking required as this is not visible yet. */ >> +            pcpu_freelist_push_node(head, buf); >> +            buf += elem_size; >> +        } >> +        cpu_idx++; >>       } >>   } > > .