Re: [PATCH v6 2/9] rv: Add generic uprobe infrastructure for RV monitors

[Gabriele Monaco]

On Fri, 2026-08-21 at 00:45 +0800, wen.yang@xxxxxxxxx wrote:
> From: Wen Yang <wen.yang@xxxxxxxxx>
> +/**
> + * struct rv_uprobe - embeddable uprobe handle for RV monitors
> + *
> + * Embed via DECLARE_RV_UPROBE() and pass &name to rv_uprobe_register().
> + * The caller may free the containing struct after rv_uprobe_unregister()
> + * (or rv_uprobe_unregister_nosync() + rv_uprobe_sync()) returns.
> + *
> + * @uc:     embedded uprobe_consumer; set handler/ret_handler before
> registering
> + * @uprobe: registered uprobe pointer (NULL when not registered)
> + * @path:   path of the probed binary, held until unregistration
> + */
> +struct rv_uprobe {
> +	struct uprobe_consumer	uc;
> +	struct uprobe		*uprobe;
> +	struct path		path;
> +};
> +
> +/* Embed a named rv_uprobe inside a caller struct */
> +#define DECLARE_RV_UPROBE(name)		struct rv_uprobe name
> +
> +/**
> + * rv_uprobe_is_registered - test whether an uprobe is currently active
> + * @p: probe to test; may be NULL
> + */

I think kernel-docs for functions should stay with their definitions (in
rv_uprobes.c). This header should have kernel-docs only for what is defined here
(structs or inline functions) and the source should have complete kernel-docs fo
r what's defined there.
Use /** only for complete kernel-docs. You can validate it with
tools/docs/kernel-doc .

> +bool rv_uprobe_is_registered(const struct rv_uprobe *p);
> +
> +/**
> + * rv_uprobe_register - initialise and register an uprobe
> + * @binpath: absolute path to the target binary
> + * @offset:  byte offset within the binary
> + * @p:       caller-provided rv_uprobe (embedded via DECLARE_RV_UPROBE);
> + *           p->uc.handler and/or p->uc.ret_handler must be set before this
> call
> + *
> + * Resolves the path and registers p->uc with the uprobe subsystem.
> + * No heap allocation is performed.
> + *
> + * Returns 0 on success, negative errno on failure.
> + */
> +int rv_uprobe_register(const char *binpath, loff_t offset, struct rv_uprobe
> *p);
...
> +/**
> + * rv_uprobe_unregister_nosync - dequeue an uprobe without waiting
> + */
> +void rv_uprobe_unregister_nosync(struct rv_uprobe *p)
> +{
> +	if (!p || !p->uprobe)
> +		return;
> +
> +	uprobe_unregister_nosync(p->uprobe, &p->uc);
> +	p->uprobe = NULL;
> +	/* path held; caller must call rv_uprobe_sync() then path_put(&p-
> >path) */

This comment isn't necessary here if you have the kernel-doc up-to-date.

> +}
> +EXPORT_SYMBOL_GPL(rv_uprobe_unregister_nosync);

The implementation looks alright, but I still need to test it.

Thanks,
Gabriele