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 B741AC433EF for ; Tue, 10 May 2022 03:15:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235210AbiEJDTj (ORCPT ); Mon, 9 May 2022 23:19:39 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59024 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235232AbiEJDT1 (ORCPT ); Mon, 9 May 2022 23:19:27 -0400 Received: from mail-pf1-x42d.google.com (mail-pf1-x42d.google.com [IPv6:2607:f8b0:4864:20::42d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 424C22A7C36; Mon, 9 May 2022 20:15:28 -0700 (PDT) Received: by mail-pf1-x42d.google.com with SMTP id j6so13826534pfe.13; Mon, 09 May 2022 20:15:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=OzBRtyA4Byv4LHvvdlavJZPXa2xwqQ3WqNSL+ipftlE=; b=GRyifsnxSAzQclB46AIqh//+4UVGhBderEtEsgh88zGcsLB6KJfD4JqQotuLmYQk8C qlWplL77Gza4lP3BSveSdF+ZsPItB7vHxW3p1K06qWPGHVB0HKJm9rQYB/F3VYHbi6Po bYhc+MYISHqIWN8palL2/40KWuIy5SK+VMCp2078167w96Q7/bQccaovelMHkFE1N41v zJ6kHyBj+H0YTYOLH0Wk2d0vJx7PcFM3RHlx/IxKenDEjGnx/lGzd8vIipji3mkpornL 6nCxSOiGZPNmWOkDYhXbD89Cx9QTcpl/N7g4pgZdfSRAQ1os9u74kz+sOrYQUs52DGiX RMwA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=OzBRtyA4Byv4LHvvdlavJZPXa2xwqQ3WqNSL+ipftlE=; b=Ujl+paofVJPJ8B+ecRBAasmWgoX7nNpuaVMu/LDUN3fhRMNEmA6FkYx4b4kXT6e0bb z+R6BzahBKMgTnvj715wwVdmNwhcVPm8td3HMKZVJD1ap61+9zdvYfac7VI1gZicO+S5 ixDBrfDeKPe2a7nVmPFNQT6y5r9HeJEjJoroeRAWldLpWeSG2skBspnIf01NacsYepHN TWW9MVd2SH1CdvRCEQOqSr7YviTtbtLHk/88aZtnlyuAEv9lxmhwxv9pyB0VNAs8nLKo AgwhfO/UM5KZpBe9xN9ido6NG0mnJVyf4yvVUsYLNT6fTm5dMCiBz1HfLo68tTnCKkDl i0Sw== X-Gm-Message-State: AOAM532bGGyAHcr6JZRlaDMgjz6ww3qxDX8N6ftJFnvylyiDV6R0OMeI Huwr6YlkxITG3bLY+qyLjbKwWe8AOcyH1GhcnYg= X-Google-Smtp-Source: ABdhPJx5NoYTMAQANTkydC4IpcP+swG4dX1n9zoWhruqsIgQDTRC5XKQkT1vr5YaiY0DmWqZsC4y49eBzgbkatMRFIE= X-Received: by 2002:a05:6a00:8ce:b0:510:9298:ea26 with SMTP id s14-20020a056a0008ce00b005109298ea26mr12990895pfu.55.1652152527696; Mon, 09 May 2022 20:15:27 -0700 (PDT) MIME-Version: 1.0 References: <20220507024840.42662-1-zhoufeng.zf@bytedance.com> In-Reply-To: From: Alexei Starovoitov Date: Mon, 9 May 2022 20:15:16 -0700 Message-ID: Subject: Re: [External] Re: [PATCH bpf-next] bpf: add bpf_map_lookup_percpu_elem for percpu map To: Feng Zhou Cc: Yosry Ahmed , Andrii Nakryiko , Alexei Starovoitov , Daniel Borkmann , Andrii Nakryiko , Martin Lau , Song Liu , Yonghong Song , john fastabend , KP Singh , Steven Rostedt , Ingo Molnar , Jiri Olsa , Dave Marchevsky , Joanne Koong , Geliang Tang , Networking , bpf , open list , Xiongchun Duan , Muchun Song , Dongdong Wang , Cong Wang , Chengming Zhou Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, May 9, 2022 at 7:41 PM Feng Zhou wrote: > > =E5=9C=A8 2022/5/10 =E4=B8=8A=E5=8D=889:04, Yosry Ahmed =E5=86=99=E9=81= =93: > > On Mon, May 9, 2022 at 5:34 PM Andrii Nakryiko > > wrote: > >> On Fri, May 6, 2022 at 7:49 PM Feng zhou w= rote: > >>> From: Feng Zhou > >>> > >>> Trace some functions, such as enqueue_task_fair, need to access the > >>> corresponding cpu, not the current cpu, and bpf_map_lookup_elem percp= u map > >>> cannot do it. So add bpf_map_lookup_percpu_elem to accomplish it for > >>> percpu_array_map, percpu_hash_map, lru_percpu_hash_map. > >>> > >>> The implementation method is relatively simple, refer to the implemen= tation > >>> method of map_lookup_elem of percpu map, increase the parameters of c= pu, and > >>> obtain it according to the specified cpu. > >>> > >> I don't think it's safe in general to access per-cpu data from another > >> CPU. I'd suggest just having either a ARRAY_OF_MAPS or adding CPU ID > >> as part of the key, if you need such a custom access pattern. > > I actually just sent an RFC patch series containing a similar patch > > for the exact same purpose. There are instances in the kernel where > > per-cpu data is accessed from other cpus (e.g. > > mem_cgroup_css_rstat_flush()). I believe, like any other variable, > > percpu data can be safe or not safe to access, based on the access > > pattern. It is up to the user to coordinate accesses to the variable. > > > > For example, in my use case, one of the accessors only reads percpu > > values of different cpus, so it should be safe. If a user accesses > > percpu data of another cpu without guaranteeing safety, they corrupt > > their own data. I understand that the main purpose of percpu data is > > lockless (and therefore fast) access, but in some use cases the user > > may be able to safely (and locklessly) access the data concurrently. > > > > Regarding data security, I think users need to consider before using it, > such > as hook enqueue_task_fair, the function itself takes the rq lock of the > corresponding cpu, there is no problem, and the kernel only provides a > method, > like bpf_per_cpu_ptr and bpf_this_cpu_ptr, data security needs to be > guaranteed > by users in different scenarios, such as using bpf_spin_lock. Right. The new helper looks useful and is safe. Please add a selftest and respin.