On Tue, Oct 03, 2023 at 08:24:47PM +0200, Florian Westphal wrote: > Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote: > > > Right, I think that will work. > > > For rbtree, sync gc is kept in place, elements are not zapped, > > > they get tagged as DEAD, including the end element. > > > > > > Then from commit, do full scan and remove any and all elements > > > that are flagged as DEAD or have expired. > > > > Sounds good. > > > > Would you follow this approach to fix the existing issue with the > > rbtree on-demand GC in nf.git? > > Actually, I don't see why its needed. With your proposal > to make the "is_expired" check during transaction consistently based on > a fixed tstamp, expiry during transaction becomes impossible. > So we can keep immediate rb_erase around. Makes sense. > I suggest to take my proposal to erase, signal -EAGAIN to caller, > then have caller retry. Apply this to nf.git as a bug fix. > > Then, I can take my patches that are mixed into the gc rework; > split those up, and we take the "no more async rbtree gc" for nf-next. > > Do you still spot a problem if we retain the on-insert node erase? Apart from this unbound loop, which sooner or later will not hit EAGAIN, no. > To give some numbers (async gc disabled): > > Insert 20k ranges into rbtree (takes ~4minutes). > Wait until all have expired. > Insert a single range: takes 250ms (entire tree has to be purged). > > Don't think it will be any faster with dead-bit approach, > we simply move cost to later in the transaction. > > The only nf.git "advantage" is that we typically won't have > to zap the entire tree during transaction, but thats due to > async gc and I'd rather remove it. > > What do you think? I am fine with this approach. What it comes, will be redo in nf-next.
