On Thu, 21 Nov 2024 at 19:09, Eduard Zingerman <eddyz87@xxxxxxxxx> wrote: > > On Wed, 2024-11-20 at 16:53 -0800, Kumar Kartikeya Dwivedi wrote: > > To ensure consistency in resource handling, move RCU and preemption > > state counters to bpf_func_state, and convert all users to access them > > through cur_func(env). > > > > For the sake of consistency, also compare active_locks in ressafe as a > > quick way to eliminate iteration and entry matching if the number of > > locks are not the same. > > > > OTOH, the comparison of active_preempt_locks and active_rcu_lock is > > needed for correctness, as state exploration cannot be avoided if these > > counters do not match, and not comparing them will lead to problems > > since they lack an actual entry in the acquired_res array. > > > > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx> > > --- > > This change is a bit confusing to me. > The following is done currently: > - in setup_func_entry() called from check_func_call(): > copy_resource_state(callee, caller); > - in prepare_func_exit(): > copy_resource_state(caller, callee); > > So it seems that it is logical to track resources in the > bpf_verifier_state and avoid copying. > There is probably something I don't understand. > This is what we were doing all along, and you're right, it is sort of a global entity. But we've moved active_locks to bpf_func_state, where references reside, while RCU and preempt lock state stays in verifier state. Either everything should be in cur_func, or in bpf_verifier_state. I am fine with either of them, because it would materially does not matter too much. Alexei's preference has been stashing this in bpf_func_state instead in [0]. Let me know what you think. [0] https://lore.kernel.org/bpf/CAADnVQKxgE7=WhjNckvMDTZ5GZujPuT3Dqd+sY=pW8CWoaF9FA@xxxxxxxxxxxxxx > [...] >
