Hi, Sorry I missed this thread. Thanks for your comments. On Tue, 4 Jun 2024 14:03:05 -0400 Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx> wrote: > On 2024-06-04 12:34, Steven Rostedt wrote: > > On Tue, 4 Jun 2024 11:02:16 -0400 > > Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx> wrote: > > > >> I see. > >> > >> It looks like there are a few things we could improve there: > >> > >> 1) With your approach, modules need to be already loaded before > >> attaching an fprobe event to them. This effectively prevents > >> attaching to any module init code. Is there any way we could allow > >> this by implementing a module coming notifier in fprobe as well ? > >> This would require that fprobes are kept around in a data structure > >> that matches the modules when they are loaded in the coming notifier. > > > > The above sounds like a nice enhancement, but not something necessary for > > this series. > > IMHO it is nevertheless relevant to discuss the impact of supporting > this kind of use-case on the ABI presented to userspace, at least to > validate that what is exposed today can incrementally be enhanced > towards that goal. > > I'm not saying that it needs to be implemented today, but we should > at least give it some thoughts right now to make sure the ABI is a > good fit. OK, let me try to update to handle module loading. I also need to add a module which has a test tracepoint in init function. > > >> > >> 2) Given that the fprobe module going notifier is protected by the > >> event_mutex, can we use locking rather than reference counting > >> in fprobe attach to guarantee the target module is not reclaimed > >> concurrently ? This would remove the transient side-effect of > >> holding a module reference count which temporarily prevents module > >> unload. See trace_kprobe_module_callback()@kernel/trace/trace_kprobe.c. I think we can filter the MODULE_STATE_COMING flag before locking event_mutex. We anyway don't check the module is going because it would be a waste to disarm the raw tracepoint events from the going module. Thank you, > > > > Why do we care about unloading modules during the transition? Note, module > > unload has always been considered a second class citizen, and there's been > > talks in the past to even rip it out. > > As a general rule I try to ensure tracing has as little impact on the > system behavior so issues that occur without tracing can be reproduced > with instrumentation. > > On systems where modules are loaded/unloaded with udev, holding > references on modules can spuriously prevent module unload, which > as a consequence changes the system behavior. > > About the relative importance of the various kernel subsystems, > following your reasoning that module unload is considered a > second-class citizen within the kernel, I would argue that tracing > is a third-class citizen and should not needlessly modify the > behavior of classes above it. > > Thanks, > > Mathieu > > -- > Mathieu Desnoyers > EfficiOS Inc. > https://www.efficios.com > -- Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>
