Messages in this thread | | | Date | Fri, 6 Mar 2020 15:55:24 -0500 (EST) | From | Mathieu Desnoyers <> | Subject | Re: [PATCH v4 16/27] tracing: Remove regular RCU context for _rcuidle tracepoints (again) |
| |
----- On Mar 6, 2020, at 3:45 PM, rostedt rostedt@goodmis.org wrote:
> On Fri, 6 Mar 2020 15:22:46 -0500 (EST) > Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote: > >> I agree with the overall approach. Just a bit of nitpicking on the API: >> >> I understand that the "prio" argument is a separate argument because it can take >> many values. However, "rcu" is just a boolean, so I wonder if we should not >> rather >> introduce a "int flags" with a bitmask enum, e.g. > > I thought about this approach, but thought it was a bit overkill. As the > kernel doesn't have an internal API, I figured we can switch this over to > flags when we get another flag to add. Unless you can think of one in the > near future.
The additional feature I have in mind for near future would be to register a probe which can take a page fault to a "sleepable" tracepoint. This would require preemption to be enabled and use of SRCU.
We can always change things when we get there.
Thanks,
Mathieu
> >> >> int tracepoint_probe_register_prio_flags(struct tracepoint *tp, void *probe, >> void *data, int prio, int flags) >> >> where flags would be populated through OR between labels of this enum: >> >> enum tracepoint_flags { >> TRACEPOINT_FLAG_RCU = (1U << 0), >> }; >> >> We can then be future-proof for additional flags without ending up calling e.g. >> >> tracepoint_probe_register_featurea_featureb_featurec(tp, probe, data, 0, 1, 0, >> 1) > > No, as soon as there is another boolean to add, the rcu version would be > switched to flags. I even thought about making the rcu and prio into one, > and change prio to be a SHRT_MAX max, and have the other 16 bits be for > flags. > > -- Steve > > >> > > which seems rather error-prone and less readable than a set of flags.
-- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com
| |