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.
