From mboxrd@z Thu Jan 1 00:00:00 1970 From: Yonghong Song Subject: Re: [RFC PATCH bpf-next 2/6] bpf: add bpf_get_stack helper Date: Mon, 9 Apr 2018 09:52:47 -0700 Message-ID: References: <20180406214846.916265-1-yhs@fb.com> <20180406214846.916265-3-yhs@fb.com> <625de1bb-7cb2-5f9e-01c3-a863cd27b0e6@fb.com> <49c15114-c518-b591-470a-b4073a675588@fb.com> <181ba790-db7c-943c-8b77-aabe10f1a1de@iogearbox.net> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 8bit Cc: To: Daniel Borkmann , Alexei Starovoitov , Return-path: Received: from mx0a-00082601.pphosted.com ([67.231.145.42]:54768 "EHLO mx0a-00082601.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753167AbeDIQxQ (ORCPT ); Mon, 9 Apr 2018 12:53:16 -0400 In-Reply-To: <181ba790-db7c-943c-8b77-aabe10f1a1de@iogearbox.net> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 4/9/18 3:01 AM, Daniel Borkmann wrote: > On 04/09/2018 07:02 AM, Alexei Starovoitov wrote: >> On 4/8/18 9:53 PM, Yonghong Song wrote: >>>>> @@ -1004,7 +1007,8 @@ static void __bpf_prog_put(struct bpf_prog >>>>> *prog, bool do_idr_lock) >>>>>              bpf_prog_kallsyms_del(prog->aux->func[i]); >>>>>          bpf_prog_kallsyms_del(prog); >>>>> >>>>> -        call_rcu(&prog->aux->rcu, __bpf_prog_put_rcu); >>>>> +        synchronize_rcu(); >>>>> +        __bpf_prog_put_rcu(&prog->aux->rcu); >>>> >>>> there should have been lockdep splat. >>>> We cannot call synchronize_rcu here, since we cannot sleep >>>> in some cases. >>> >>> Let me double check this. The following is the reason >>> why I am using synchronize_rcu(). >>> >>> With call_rcu(&prog->aux->rcu, __bpf_prog_put_rcu) and >>> _bpf_prog_put_rcu calls put_callchain_buffers() which >>> calls mutex_lock(), the runtime with CONFIG_DEBUG_ATOMIC_SLEEP=y >>> will complains since potential sleep inside the call_rcu is not >>> allowed. >> >> I see. Indeed. We cannot call put_callchain_buffers() from rcu callback, >> but doing synchronize_rcu() here is also not possible. >> How about moving put_callchain into bpf_prog_free_deferred() ? > > +1, the assumption is that you can call bpf_prog_put() and also the > bpf_map_put() from any context. Sleeping here for a long time might > subtly break things badly. Thanks for the suggestion! This should work.