Received-SPF: pass (google.com: domain of linux-kernel+bounces-201222-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <5fc7a866-ecd9-4b57-9740-369544df1264@efficios.com>
Date: Tue, 4 Jun 2024 14:03:05 -0400
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2 2/3] tracing/fprobe: Support raw tracepoint events on
 modules
To: Steven Rostedt <rostedt@goodmis.org>, Luis Chamberlain
 <mcgrof@kernel.org>, linux-modules@vger.kernel.org
Cc: "Masami Hiramatsu (Google)" <mhiramat@kernel.org>, don
 <zds100@gmail.com>, linux-kernel@vger.kernel.org,
 linux-trace-kernel@vger.kernel.org
References: <171723014778.258703.6731294779199848686.stgit@devnote2>
 <171723016594.258703.1629777910752596529.stgit@devnote2>
 <fbfec8d9-d0ed-4384-bbd2-dd5c1e568ed1@efficios.com>
 <20240604084955.29b9440687522a1347e0e7cd@kernel.org>
 <419b80da-9cbf-4bb2-aabb-dc04f0fb0f37@efficios.com>
 <20240604123418.22e16e97@gandalf.local.home>
From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Content-Language: en-US
In-Reply-To: <20240604123418.22e16e97@gandalf.local.home>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

On 2024-06-04 12:34, Steven Rostedt wrote:
> On Tue, 4 Jun 2024 11:02:16 -0400
> Mathieu Desnoyers <mathieu.desnoyers@efficios.com> 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.

>>
>> 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.
> 
> 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