On Tue, Aug 9, 2022 at 3:29 PM Martin KaFai Lau <kafai@xxxxxx> wrote: > > On Tue, Aug 09, 2022 at 11:30:32PM +0200, Kumar Kartikeya Dwivedi wrote: > > The LRU map that is preallocated may have its elements reused while > > another program holds a pointer to it from bpf_map_lookup_elem. Hence, > > only check_and_free_fields is appropriate when the element is being > > deleted, as it ensures proper synchronization against concurrent access > > of the map value. After that, we cannot call check_and_init_map_value > > again as it may rewrite bpf_spin_lock, bpf_timer, and kptr fields while > > they can be concurrently accessed from a BPF program. > > > > This is safe to do as when the map entry is deleted, concurrent access > > is protected against by check_and_free_fields, i.e. an existing timer > > would be freed, and any existing kptr will be released by it. The > > program can create further timers and kptrs after check_and_free_fields, > > but they will eventually be released once the preallocated items are > > freed on map destruction, even if the item is never reused again. Hence, > > the deleted item sitting in the free list can still have resources > > attached to it, and they would never leak. > > > > With spin_lock, we never touch the field at all on delete or update, as > > we may end up modifying the state of the lock. Since the verifier > > ensures that a bpf_spin_lock call is always paired with bpf_spin_unlock > > call, the program will eventually release the lock so that on reuse the > > new user of the value can take the lock. > The bpf_spin_lock's verifier description makes sense. Note that > the lru map does not support spin lock for now. ahh. then it's not a bpf tree material. It's a minor cleanup for bpf-next? > > > > Essentially, for the preallocated case, we must assume that the map > > value may always be in use by the program, even when it is sitting in > > the freelist, and handle things accordingly, i.e. use proper > > synchronization inside check_and_free_fields, and never reinitialize the > > special fields when it is reused on update. > Acked-by: Martin KaFai Lau <kafai@xxxxxx>
