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;
From:   Song Liu <songliubraving@fb.com>
To:     Martin Lau <kafai@fb.com>
CC:     bpf <bpf@vger.kernel.org>, Networking <netdev@vger.kernel.org>,
        open list <linux-kernel@vger.kernel.org>,
        "mingo@redhat.com" <mingo@redhat.com>,
        "peterz@infradead.org" <peterz@infradead.org>,
        "ast@kernel.org" <ast@kernel.org>,
        "daniel@iogearbox.net" <daniel@iogearbox.net>,
        "andrii@kernel.org" <andrii@kernel.org>,
        "john.fastabend@gmail.com" <john.fastabend@gmail.com>,
        "kpsingh@chromium.org" <kpsingh@chromium.org>,
        Kernel Team <Kernel-team@fb.com>,
        "haoluo@google.com" <haoluo@google.com>,
        kernel test robot <lkp@intel.com>
Subject: Re: [PATCH bpf-next 1/4] bpf: enable task local storage for tracing
 programs
Thread-Topic: [PATCH bpf-next 1/4] bpf: enable task local storage for tracing
 programs
Thread-Index: AQHW5hbxN1bu8adE10+5MJhm5YS4VaoiytsAgABPhAA=
Date:   Mon, 11 Jan 2021 23:41:26 +0000
Message-ID: <4D0333B1-C729-41BF-AB1D-9AC233431094@fb.com>
References: <20210108231950.3844417-1-songliubraving@fb.com>
 <20210108231950.3844417-2-songliubraving@fb.com>
 <20210111185650.hsvfpoqmqc2mj7ci@kafai-mbp.dhcp.thefacebook.com>
In-Reply-To: <20210111185650.hsvfpoqmqc2mj7ci@kafai-mbp.dhcp.thefacebook.com>
Accept-Language: en-US
Content-Language: en-US
Content-Type: text/plain; charset="us-ascii"
Content-ID: <D9AE85DC217BCC46A44CF4881D671C58@namprd15.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 933adb73-fc8e-49f5-da28-08d8b68a69d9
X-MS-Exchange-CrossTenant-originalarrivaltime: 11 Jan 2021 23:41:27.0072
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 8ae927fe-1255-47a7-a2af-5f3a069daaa2
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: Cq460RPYwsaMVdQvlOx6b6LQeCwuH3riOJHPOQh6If4PV/bLyXlLizqeTmVnWRXnXrjaPEHMNzNGD/9W8wupwg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR15MB3666
X-OriginatorOrg: fb.com
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.343,18.0.737
 definitions=2021-01-11_34:2021-01-11,2021-01-11 signatures=0
X-Proofpoint-Spam-Details: rule=fb_default_notspam policy=fb_default score=0 priorityscore=1501
 phishscore=0 impostorscore=0 clxscore=1015 malwarescore=0 suspectscore=0
 mlxlogscore=942 spamscore=0 bulkscore=0 lowpriorityscore=0 adultscore=0
 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2009150000 definitions=main-2101110135
X-FB-Internal: deliver
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


> On Jan 11, 2021, at 10:56 AM, Martin Lau <kafai@fb.com> wrote:
>=20
> On Fri, Jan 08, 2021 at 03:19:47PM -0800, Song Liu wrote:
>=20
> [ ... ]
>=20
>> diff --git a/kernel/bpf/bpf_local_storage.c b/kernel/bpf/bpf_local_stora=
ge.c
>> index dd5aedee99e73..9bd47ad2b26f1 100644
>> --- a/kernel/bpf/bpf_local_storage.c
>> +++ b/kernel/bpf/bpf_local_storage.c
>> @@ -140,17 +140,18 @@ static void __bpf_selem_unlink_storage(struct bpf_=
local_storage_elem *selem)
>> {
>> 	struct bpf_local_storage *local_storage;
>> 	bool free_local_storage =3D false;
>> +	unsigned long flags;
>>=20
>> 	if (unlikely(!selem_linked_to_storage(selem)))
>> 		/* selem has already been unlinked from sk */
>> 		return;
>>=20
>> 	local_storage =3D rcu_dereference(selem->local_storage);
>> -	raw_spin_lock_bh(&local_storage->lock);
>> +	raw_spin_lock_irqsave(&local_storage->lock, flags);
> It will be useful to have a few words in commit message on this change
> for future reference purpose.
>=20
> Please also remove the in_irq() check from bpf_sk_storage.c
> to avoid confusion in the future.  It probably should
> be in a separate patch.

Do you mean we allow bpf_sk_storage_get_tracing() and=20
bpf_sk_storage_delete_tracing() in irq context? Like

diff --git i/net/core/bpf_sk_storage.c w/net/core/bpf_sk_storage.c
index 4edd033e899c0..14dd5e3c67402 100644
--- i/net/core/bpf_sk_storage.c
+++ w/net/core/bpf_sk_storage.c
@@ -425,7 +425,7 @@ BPF_CALL_4(bpf_sk_storage_get_tracing, struct bpf_map *=
, map, struct sock *, sk,
 BPF_CALL_2(bpf_sk_storage_delete_tracing, struct bpf_map *, map,
           struct sock *, sk)
 {
-       if (in_irq() || in_nmi())
+       if (in_nmi())
                return -EPERM;

        return ____bpf_sk_storage_delete(map, sk);

[...]=