On Tue, Nov 17, 2020 at 06:05:51PM -0500, Mathieu Desnoyers wrote: > ----- On Nov 16, 2020, at 5:10 PM, rostedt rostedt@xxxxxxxxxxx wrote: > > > On Mon, 16 Nov 2020 16:34:41 -0500 (EST) > > Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx> wrote: > > [...] > > >> I think you'll want a WRITE_ONCE(old[i].func, tp_stub_func) here, matched > >> with a READ_ONCE() in __DO_TRACE. This introduces a new situation where the > >> func pointer can be updated and loaded concurrently. > > > > I thought about this a little, and then only thing we really should worry > > about is synchronizing with those that unregister. Because when we make > > this update, there are now two states. the __DO_TRACE either reads the > > original func or the stub. And either should be OK to call. > > > > Only the func gets updated and not the data. So what exactly are we worried > > about here? > > Indeed with a stub function, I don't see any need for READ_ONCE/WRITE_ONCE. I'm not sure if this is a practical issue, but without WRITE_ONCE, can't the write be torn? A racing __traceiter_ could potentially see a half-modified function pointer, which wouldn't work out too well. This was actually my gut instinct before I wrote the __GFP_NOFAIL instead -- currently that whole array's memory ordering is provided by RCU and I didn't dive deep enough to evaluate getting too clever with atomic modifications to it. > > However, if we want to compare the function pointer to some other value and > conditionally do (or skip) the call, I think you'll need the READ_ONCE/WRITE_ONCE > to make sure the pointer is not re-fetched between comparison and call. > > Thanks, > > Mathieu > > -- > Mathieu Desnoyers > EfficiOS Inc. > http://www.efficios.com
