[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20250902231152.442041a74774d888cec39201@kernel.org>
Date: Tue, 2 Sep 2025 23:11:52 +0900
From: Masami Hiramatsu (Google) <mhiramat@...nel.org>
To: Jinchao Wang <wangjinchao600@...il.com>
Cc: akpm@...ux-foundation.org, naveen@...nel.org, davem@...emloft.net,
linux-mm@...ck.org, linux-kernel@...r.kernel.org,
linux-trace-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 02/13] x86/HWBP: Add arch_reinstall_hw_breakpoint()
for atomic updates
On Mon, 1 Sep 2025 18:23:44 +0800
Jinchao Wang <wangjinchao600@...il.com> wrote:
> On 9/1/25 15:06, Masami Hiramatsu (Google) wrote:
> > Hi Jinchao,
> >
> Hi Masami,
>
> > On Mon, 18 Aug 2025 20:26:07 +0800
> > Jinchao Wang <wangjinchao600@...il.com> wrote:
> >
> >> Add arch_reinstall_hw_breakpoint() to enable atomic context modification
> >> of hardware breakpoint parameters without deallocating and reallocating
> >> the breakpoint slot.
> >>
> >> The existing arch_install_hw_breakpoint() allocates a new debug register
> >> slot, while arch_uninstall_hw_breakpoint() deallocates it. However, some
> >> use cases require modifying breakpoint parameters (address, length, type)
> >> atomically without losing the allocated slot, particularly when operating
> >> in atomic contexts where allocation might fail or be unavailable.
> >>
> >> This is particularly useful for debugging tools like kstackwatch that
> >> need to dynamically update breakpoint targets in atomic contexts while
> >> maintaining consistent hardware state.
> >>
> >
> > I'm also trying to find this interface for my wprobe. So the idea is good.
> > But this looks hacky and only for x86. I think the interface should be
> > more generic and do not use this arch internal function directly.
> >
>
> I agree with your point about the architectural dependency. I have been
> considering this problem not only for the hardware breakpoint
> reinstallation,
> but also for other related parts of the series, such as canary finding and
> stack address resolving. These parts also rely on arch-specific code.
Yes, even though, the hw-breakpoint is an independent feature.
Directly using arch_*() functions (which are expected to be used
internally) introduces a hidden dependency between these two
components and looses maintainability.
> > It seems that the slot is allocated by "type", thus, if this reinstall
> > hwbp without deallocate/allocate slot, it must NOT change the type.
> > See __modify_bp_slot. Also, provide CONFIG_HAVE_... option for checking
> > whether the architecture support that interface.
> >
> Regarding the slot allocation, I would like to clarify my point. I
> believe the
> event->attr.type should not be changed when reinstalling a hardware
> breakpoint, as this defines the fundamental nature of the event. The type
> must always be PERF_TYPE_BREAKPOINT.
>
> The event->attr.bp_type, however, can be changed. For example, from a
> HW_BREAKPOINT_W to a HW_BREAKPOINT_RW without needing to deallocate and
> reallocate the slot. This is useful for future applications, even though the
> current use case for KStackWatch only requires HW_BREAKPOINT_W.
I understand your point, so it also needs another wrapper which checks
the type is compatible on the architecture.
>
> By the way, I have sent an updated series.
> https://lore.kernel.org/all/20250828073311.1116593-1-wangjinchao600@gmail.com/
Yeah, OK, let me review the series. Thanks for update!
>
> Thank you again for your valuable review.
> --
> Best regards,
> Jinchao
--
Masami Hiramatsu (Google) <mhiramat@...nel.org>
Powered by blists - more mailing lists