Re: [PATCHv2 perf/core 01/22] uprobes: Remove breakpoint in unapply_uprobe under mmap_write_lock
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Date: 2025-05-22 14:48:26
Also in:
bpf, lkml
On Tue, 20 May 2025 16:19:26 +0200 Oleg Nesterov [off-list ref] wrote:
On 05/20, Masami Hiramatsu wrote:quoted
On Thu, 15 May 2025 14:10:58 +0200 Jiri Olsa [off-list ref] wrote:quoted
Currently unapply_uprobe takes mmap_read_lock, but it might call remove_breakpoint which eventually changes user pages. Current code writes either breakpoint or original instruction, so it can probably go away with that, but with the upcoming change that writes multiple instructions on the probed address we need to ensure that any update to mm's pages is exclusive.So, this is a bugfix, right?No, mmap_read_lock() is fine. To remind, this was already discussed with you, see [PATCH 02/12] uprobes: grab write mmap lock in unapply_uprobe() https://lore.kernel.org/all/20240625002144.3485799-3-andrii@kernel.org/ (local) And you even reviewed this patch [PATCH 1/2] uprobes: document the usage of mm->mmap_lock https://lore.kernel.org/all/20240710140045.GA1084@redhat.com/ (local) But, as the changelog explains, this patch is needed for the upcoming changes.
Oops, OK. So current code is good with either mmap_read_lock() or mmap_write_lock(). But the patch description is a bit confusing. If the point is an atomic (byte?) update or not, it should describe it. Thank you,
-------------------------------------------------------------------------- Just in case... I'll try to read this series tomorrow, but at first glance this version addresses all my concerns. Oleg.
-- Masami Hiramatsu (Google) [off-list ref]