Re: [PATCH v11 08/11] tracing: wprobe: Add wprobe event trigger

[Masami Hiramatsu (Google) <mhiramat@kernel.org>]

On Sun,  2 Aug 2026 17:19:55 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:

> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> Add wprobe event trigger to set and clear the watch event dynamically.
> This allows us to set an watchpoint on a given local variables and
> a slab object instead of static objects.
> 
> The trigger syntax is below:
> 
>   - set_wprobe:WPROBE:FIELD[+OFFSET][:COUNT] [if FILTER]
>   - clear_wprobe:WPROBE[:FIELD[+OFFSET]][:COUNT] [if FILTER]
> 
> set_wprobe sets the address pointed by FIELD[+offset] to the WPROBE
> event. The FIELD is the field name of trigger event.
> clear_wprobe clears the watch address of WPROBE event. If the FIELD
> option is specified, it clears only if the current watch address is
> same as the given FIELD[+OFFSET] value.
> COUNT is the max number of activating trigger.
> 
> The set_wprobe trigger does not change the type and length, these
> must be set when creating a new wprobe.
> 
> Also, the WPROBE event must be disabled when setting the new trigger
> and it will be busy afterwards. Recommended usage is to add a new
> wprobe at NULL address and keep disabled.
> 
> Assisted-by: Antigravity:gemini-3.5-flash
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> ---
>  Changes in v11:
>   - Use new modify_local_hw_breakpoint_addr() API.
>   - Add tracepoint_synchronize_unregister() in wprobe_unregister_trigger()
>     and parse error path.
>   - Safely check tw->bp_event for NULL in trace_wprobe_update_local() to
>     prevent race conditions.
>   - Use event_trigger_data::private_data_free.
>   - Add count option support.

Oops, I made a mistake on this count support.

> +static int wprobe_trigger_cmd_parse(struct event_command *cmd_ops,
> +				    struct trace_event_file *file,
> +				    char *glob, char *cmd,
> +				    char *param_and_filter)
> +{
[...]
> +	/* count is optional, "unlimited" by default */
> +	count_str = strsep(&param, ":");
> +	if (count_str) {
> +		long val;
> +
> +		if (strcmp(count_str, "unlimited")) {
> +			if (str_has_prefix(count_str, "count="))
> +				count_str += 6;
> +			if (kstrtol(count_str, 0, &val) < 0)
> +				return -EINVAL;
> +			wprobe_data->count = val;
> +		}
> +	}

This handles count option, but

[...]
> +	ret = event_trigger_parse_num(param, trigger_data);
> +	if (ret)
> +		return ret;

Here we parse the count again. However, this trigger_data->count
is ignored.

I think the new wprobe_trigger_data->count is redundant. it should
use trigger_data->count.

Thank you,

-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>