On Thu, Nov 7, 2019 at 5:10 PM Song Liu <liu.song.a23@xxxxxxxxx> wrote: > > > >>>>> + goto out; > > > >>>>> + tr->selector++; > > > >>>> > > > >>>> Shall we do selector-- for unlink? > > > >>> > > > >>> It's a bit flip. I think it would be more confusing with -- > > > >> > > > >> Right.. Maybe should use int instead of u64 for selector? > > > > > > > > No, since int can overflow. > > > > > > I guess it is OK to overflow, no? > > > > overflow is not ok, since transition 0->1 should use nop->call patching > > whereas 1->2, 2->3 should use call->call. > > > > In my initial implementation (one I didn't share with anyone) I had > > trampoline_mutex taken inside bpf_trampoline_update(). And multiple link() > > operation were allowed. The idea was to attach multiple progs and update > > trampoline once. But then I realized that I cannot do that since 'unlink + > > update' where only 'update' is taking lock will not guarantee success. Since > > other 'link' operations can race and 'update' can potentially fail in > > arch_prepare_bpf_trampoline() due to new things that 'link' brought in. In that > > version (since there several fentry/fexit progs can come in at once) I used > > separate 'selector' ticker to pick the side of the page. Once I realized the > > issue (to guarantee that unlink+update == always success) I moved mutex all the > > way to unlink and link and left 'selector' as-is. Just now I realized that > > 'selector' can be removed. fentry_cnt + fexit_cnt can be used instead. This > > sum of counters will change 1 bit at a time. Am I right? > > Yeah, I think fentry_cnt + fexit_cnt is cleaner. ... and that didn't work. It's transition that matters. Either need to remember previous sum value or have separate selector. imo selector is cleaner, so I'm back to that.
