On Tue, 9 Apr 2024 13:07:04 +0200
Florian Westphal <fw@xxxxxxxxx> wrote:
> Stefano Brivio <sbrivio@xxxxxxxxxx> wrote:
> > > The rcu_barrier() is removed, its not needed: old call_rcu instances
> > > for pipapo_reclaim_match do not access struct nft_set.
> >
> > True, pipapo_reclaim_match() won't, but nft_set_pipapo_match_destroy()
> > will, right? That is:
> >
> > > - if (m) {
> > > - rcu_barrier();
> >
> > ...before b0e256f3dd2b ("netfilter: nft_set_pipapo: release elements in
> > clone only from destroy path"), this rcu_barrier() was needed because we'd
> > call nft_set_pipapo_match_destroy() on 'm'.
> >
> > That call is now gone, and we could have dropped it at that point, but:
>
> I do not follow. nft_pipapo_destroy() is not invoked asynchronously via
> call_rcu, its invoked from either abort path or the gc work queue at at
> time where there must be no references to nft_set anymore.
Hmm, sorry, I was all focused on nft_set_pipapo_match_destroy()
accessing nft_set, but that has nothing to do with
pipapo_reclaim_match(). However:
> What do we wait for, i.e., which outstanding rcu callback could
> reference a data structure that nft_pipapo_destroy() will free?
...we still have pipapo_free_match(), called by pipapo_reclaim_match(),
referencing the per-CPU scratch areas, and nft_pipapo_destroy() freeing
them (using pipapo_free_match() since this patch).
I guess *that* was the original reason why I added this rcu_barrier()
call here? Unless I'm missing something, nothing changed in this regard
since then.
Or is there another reason why pending call_rcu() callbacks can't touch
the same scratch areas now?
--
Stefano
