Gabriele
**Suggested simplification:**
---
diff --git a/include/rv/rv_uprobe.h b/include/rv/rv_uprobe.h
index 9106c5c927..4fb3f50a63 100644
--- a/include/rv/rv_uprobe.h
+++ b/include/rv/rv_uprobe.h
@@ -7,83 +7,56 @@
#ifndef _RV_UPROBE_H
#define _RV_UPROBE_H
-#include <linux/path.h>
#include <linux/types.h>
+#include <linux/uprobes.h>
struct pt_regs;
+struct inode;
/**
* struct rv_uprobe - a single uprobe registered on behalf of an RV monitor
*
- * @offset: byte offset within the ELF binary where the probe is installed
- * @priv: monitor-private pointer; set at attach time, never touched by
- * this layer; passed unchanged to entry_fn / ret_fn
- * @path: resolved path of the probed binary (read-only after attach);
- * callers may use path.dentry for identity comparisons
- *
- * The implementation fields (uprobe_consumer, uprobe handle, callbacks) are
- * private to rv_uprobe.c and are not exposed here; monitors must not access
- * them directly.
+ * @uc: underlying uprobe consumer (publicly visible)
+ * @uprobe: active uprobe structure handle
+ * @inode: inode of the target binary (read-only after registration)
*/
struct rv_uprobe {
- /* public: read-only after rv_uprobe_attach*() */
- loff_t offset;
- void *priv;
- struct path path;
+ struct uprobe_consumer uc;
+ struct uprobe *uprobe;
+ struct inode *inode;
};
-/**
- * rv_uprobe_attach_path - register an uprobe given an already-resolved path
- * @path: path of the target binary; rv_uprobe takes its own reference
- * @offset: byte offset within the binary
- * @entry_fn: called on probe hit (entry); may be NULL
- * @ret_fn: called on function return (uretprobe); may be NULL
- * @priv: opaque pointer forwarded to callbacks unchanged
- *
- * Use this variant when the caller has already resolved the path (e.g. to
- * register multiple probes on the same binary with a single kern_path call).
- * The inode is derived internally via d_real_inode(), so inode and path are
- * always consistent.
- *
- * Returns a pointer to the new rv_uprobe on success, ERR_PTR on failure.
- */
-struct rv_uprobe *rv_uprobe_attach_path(struct path *path, loff_t offset,
- int (*entry_fn)(struct rv_uprobe *p, struct pt_regs *regs, __u64 *data),
- int (*ret_fn)(struct rv_uprobe *p, unsigned long func,
- struct pt_regs *regs, __u64 *data),
- void *priv);
+/* Seamless inline declaration of a named uprobe inside user structs */
+#define DECLARE_RV_UPROBE(name) \
+ struct rv_uprobe name
/**
- * rv_uprobe_attach - resolve binpath and register an uprobe
- * @binpath: absolute path to the target binary
- * @offset: byte offset within the binary
- * @entry_fn: called on probe hit (entry); may be NULL
- * @ret_fn: called on function return (uretprobe); may be NULL
- * @priv: opaque pointer forwarded to callbacks unchanged
+ * rv_uprobe_register - resolve binpath and register an uprobe
+ * @binpath: absolute path to the target binary
+ * @offset: byte offset within the binary
+ * @p: pointer to the allocated/embedded rv_uprobe structure
+ * @handler: called on probe hit (entry); may be NULL
+ * @ret_handler: called on function return (uretprobe); may be NULL
*
- * Resolves binpath via kern_path(), then delegates to rv_uprobe_attach_path().
+ * Resolves binpath via kern_path(), registers the uprobe directly using the
+ * embedded `uprobe_consumer`, and immediately releases the path reference.
*
- * Returns a pointer to the new rv_uprobe on success, ERR_PTR on failure.
+ * Returns 0 on success, or a negative error code on failure.
*/
-struct rv_uprobe *rv_uprobe_attach(const char *binpath, loff_t offset,
- int (*entry_fn)(struct rv_uprobe *p, struct pt_regs *regs, __u64 *data),
- int (*ret_fn)(struct rv_uprobe *p, unsigned long func,
- struct pt_regs *regs, __u64 *data),
- void *priv);
+int rv_uprobe_register(const char *binpath, loff_t offset, struct rv_uprobe *p,
+ int (*handler)(struct uprobe_consumer *self, struct pt_regs *regs,
__u64 *data),
+ int (*ret_handler)(struct uprobe_consumer *self, unsigned long func,
+ struct pt_regs *regs, __u64 *data));
/**
- * rv_uprobe_detach - synchronously unregister an uprobe and free it
- * @p: probe to detach; may be NULL (no-op)
- *
- * Calls uprobe_unregister_nosync(), then uprobe_unregister_sync() to wait
- * for any in-progress handler to finish, then releases the path reference
- * and frees the rv_uprobe struct. The caller's priv data is NOT freed.
+ * rv_uprobe_unregister - synchronously unregister a uprobe
+ * @p: probe to unregister; may be NULL (no-op)
*
- * When removing a single probe, prefer this over the three-phase API.
- * Safe to call from process context only (uprobe_unregister_sync() may
- * schedule).
+ * Dequeues the uprobe and waits synchronously for all in-flight handlers
+ * to complete.
*/
-void rv_uprobe_detach(struct rv_uprobe *p);
+void rv_uprobe_unregister(struct rv_uprobe *p);
/**
* rv_uprobe_unregister_nosync - dequeue an uprobe without waiting
@@ -91,9 +64,7 @@ void rv_uprobe_detach(struct rv_uprobe *p);
*
* Removes the uprobe from the uprobe subsystem but does NOT wait for
* in-flight handlers to complete. The caller must call rv_uprobe_sync()
- * before calling rv_uprobe_free() on the same probe.
- *
- * Use this to batch multiple deregistrations before a single rv_uprobe_sync().
+ * before freeing any container holding this probe.
*/
void rv_uprobe_unregister_nosync(struct rv_uprobe *p);
@@ -101,19 +72,8 @@ void rv_uprobe_unregister_nosync(struct rv_uprobe *p);
* rv_uprobe_sync - wait for all in-flight uprobe handlers to complete
*
* Global barrier: waits for every in-flight uprobe handler across the system
- * to finish. Call once after a batch of rv_uprobe_unregister_nosync() calls
- * and before any rv_uprobe_free() call.
+ * to finish.
*/
void rv_uprobe_sync(void);
-/**
- * rv_uprobe_free - release resources of a previously deregistered probe
- * @p: probe to free; may be NULL (no-op)
- *
- * Releases the path reference and frees the rv_uprobe struct. Must only
- * be called after rv_uprobe_sync() has returned. The caller's priv data
- * is NOT freed.
- */
-void rv_uprobe_free(struct rv_uprobe *p);
-
#endif /* _RV_UPROBE_H */
diff --git a/kernel/trace/rv/monitors/tlob/tlob.c
b/kernel/trace/rv/monitors/tlob/tlob.c
index d8e0c47947..28a6c740c7 100644
--- a/kernel/trace/rv/monitors/tlob/tlob.c
+++ b/kernel/trace/rv/monitors/tlob/tlob.c
@@ -252,8 +252,8 @@ struct tlob_uprobe_binding {
char binpath[TLOB_MAX_PATH];
loff_t offset_start;
loff_t offset_stop;
- struct rv_uprobe *start_probe;
- struct rv_uprobe *stop_probe;
+ DECLARE_RV_UPROBE(start_probe);
+ DECLARE_RV_UPROBE(stop_probe);
};
/* RCU callback: free the slab once no readers remain. */
@@ -512,16 +512,16 @@ int tlob_stop_task(struct task_struct *task)
EXPORT_SYMBOL_GPL(tlob_stop_task);
-static int tlob_uprobe_entry_handler(struct rv_uprobe *p, struct pt_regs *regs,
+static int tlob_uprobe_entry_handler(struct uprobe_consumer *self, struct
pt_regs *regs,
__u64 *data)
{
- struct tlob_uprobe_binding *b = p->priv;
+ struct tlob_uprobe_binding *b = container_of(self, struct
tlob_uprobe_binding, start_probe.uc);
tlob_start_task(current, b->threshold_ns);
return 0;
}
-static int tlob_uprobe_stop_handler(struct rv_uprobe *p, struct pt_regs *regs,
+static int tlob_uprobe_stop_handler(struct uprobe_consumer *self, struct
pt_regs *regs,
__u64 *data)
{
tlob_stop_task(current);
@@ -537,6 +537,7 @@ static int tlob_add_uprobe(u64 threshold_ns, const char
*binpath,
{
struct tlob_uprobe_binding *b, *tmp_b;
char pathbuf[TLOB_MAX_PATH];
+ struct inode *inode;
struct path path;
char *canon;
int ret;
@@ -561,10 +562,12 @@ static int tlob_add_uprobe(u64 threshold_ns, const char
*binpath,
goto err_path;
}
- /* Reject duplicate start offset for the same binary. */
+ inode = d_real_inode(path.dentry);
+
+ /* Reject duplicate start offset for the same binary inode. */
list_for_each_entry(tmp_b, &tlob_uprobe_list, list) {
if (tmp_b->offset_start == offset_start &&
- tmp_b->start_probe->path.dentry == path.dentry) {
+ tmp_b->start_probe.inode == inode) {
ret = -EEXIST;
goto err_path;
}
@@ -577,29 +580,25 @@ static int tlob_add_uprobe(u64 threshold_ns, const char
*binpath,
}
strscpy(b->binpath, canon, sizeof(b->binpath));
- /* Both probes share b (priv) and path; attach_path refs path itself. */
- b->start_probe = rv_uprobe_attach_path(&path, offset_start,
- tlob_uprobe_entry_handler, NULL,
b);
- if (IS_ERR(b->start_probe)) {
- ret = PTR_ERR(b->start_probe);
- b->start_probe = NULL;
- goto err_path;
- }
+ path_put(&path);
+
+ /* Both probes are registered directly on the embedded fields */
+ ret = rv_uprobe_register(b->binpath, offset_start, &b->start_probe,
+ tlob_uprobe_entry_handler, NULL);
+ if (ret)
+ goto err_free;
- b->stop_probe = rv_uprobe_attach_path(&path, offset_stop,
- tlob_uprobe_stop_handler, NULL,
b);
- if (IS_ERR(b->stop_probe)) {
- ret = PTR_ERR(b->stop_probe);
- b->stop_probe = NULL;
+ ret = rv_uprobe_register(b->binpath, offset_stop, &b->stop_probe,
+ tlob_uprobe_stop_handler, NULL);
+ if (ret)
goto err_start;
- }
- path_put(&path);
list_add_tail(&b->list, &tlob_uprobe_list);
return 0;
err_start:
- rv_uprobe_detach(b->start_probe);
+ rv_uprobe_unregister(&b->start_probe);
+ goto err_free;
err_path:
path_put(&path);
err_free:
@@ -611,21 +610,24 @@ static int tlob_remove_uprobe_by_key(loff_t offset_start,
const char *binpath)
{
struct tlob_uprobe_binding *b, *tmp;
struct path remove_path;
+ struct inode *inode;
int ret;
ret = kern_path(binpath, LOOKUP_FOLLOW, &remove_path);
if (ret)
return ret;
+ inode = d_real_inode(remove_path.dentry);
+
ret = -ENOENT;
list_for_each_entry_safe(b, tmp, &tlob_uprobe_list, list) {
if (b->offset_start != offset_start)
continue;
- if (b->start_probe->path.dentry != remove_path.dentry)
+ if (b->start_probe.inode != inode)
continue;
list_del(&b->list);
- rv_uprobe_detach(b->start_probe);
- rv_uprobe_detach(b->stop_probe);
+ rv_uprobe_unregister(&b->start_probe);
+ rv_uprobe_unregister(&b->stop_probe);
kfree(b);
ret = 0;
break;
@@ -643,8 +645,8 @@ static void tlob_remove_all_uprobes(void)
mutex_lock(&tlob_uprobe_mutex);
list_for_each_entry_safe(b, tmp, &tlob_uprobe_list, list) {
list_move(&b->list, &pending);
- rv_uprobe_unregister_nosync(b->start_probe);
- rv_uprobe_unregister_nosync(b->stop_probe);
+ rv_uprobe_unregister_nosync(&b->start_probe);
+ rv_uprobe_unregister_nosync(&b->stop_probe);
}
mutex_unlock(&tlob_uprobe_mutex);
@@ -658,8 +660,6 @@ static void tlob_remove_all_uprobes(void)
rv_uprobe_sync();
list_for_each_entry_safe(b, tmp, &pending, list) {
- rv_uprobe_free(b->start_probe);
- rv_uprobe_free(b->stop_probe);
kfree(b);
}
}
diff --git a/kernel/trace/rv/rv_uprobe.c b/kernel/trace/rv/rv_uprobe.c
index 3d8b764dde..69b8b0c27e 100644
--- a/kernel/trace/rv/rv_uprobe.c
+++ b/kernel/trace/rv/rv_uprobe.c
@@ -10,149 +10,74 @@
#include <linux/uprobes.h>
#include <rv/rv_uprobe.h>
-/*
- * Private extension of struct rv_uprobe. Allocated by rv_uprobe_attach*()
- * and returned to callers as &impl->pub.
- */
-struct rv_uprobe_impl {
- struct rv_uprobe pub; /* must be first; callers hold &pub */
- struct uprobe_consumer uc;
- struct uprobe *uprobe;
- int (*entry_fn)(struct rv_uprobe *p, struct pt_regs *regs, __u64 *data);
- int (*ret_fn)(struct rv_uprobe *p, unsigned long func,
- struct pt_regs *regs, __u64 *data);
-};
-
-static int rv_uprobe_handler(struct uprobe_consumer *uc,
- struct pt_regs *regs, __u64 *data)
-{
- struct rv_uprobe_impl *impl = container_of(uc, struct rv_uprobe_impl,
uc);
-
- if (impl->entry_fn)
- return impl->entry_fn(&impl->pub, regs, data);
- return 0;
-}
-
-static int rv_uprobe_ret_handler(struct uprobe_consumer *uc,
- unsigned long func,
- struct pt_regs *regs, __u64 *data)
-{
- struct rv_uprobe_impl *impl = container_of(uc, struct rv_uprobe_impl,
uc);
-
- if (impl->ret_fn)
- return impl->ret_fn(&impl->pub, func, regs, data);
- return 0;
-}
-
-static struct rv_uprobe *
-__rv_uprobe_attach(struct inode *inode, struct path *path, loff_t offset,
- int (*entry_fn)(struct rv_uprobe *p, struct pt_regs *regs,
__u64 *data),
- int (*ret_fn)(struct rv_uprobe *p, unsigned long func,
- struct pt_regs *regs, __u64 *data),
- void *priv)
-{
- struct rv_uprobe_impl *impl;
- int ret;
-
- if (!entry_fn && !ret_fn)
- return ERR_PTR(-EINVAL);
-
- impl = kzalloc_obj(*impl, GFP_KERNEL);
- if (!impl)
- return ERR_PTR(-ENOMEM);
-
- impl->pub.offset = offset;
- impl->pub.priv = priv;
- impl->entry_fn = entry_fn;
- impl->ret_fn = ret_fn;
- path_get(path);
- impl->pub.path = *path;
-
- if (entry_fn)
- impl->uc.handler = rv_uprobe_handler;
- if (ret_fn)
- impl->uc.ret_handler = rv_uprobe_ret_handler;
-
- impl->uprobe = uprobe_register(inode, offset, 0, &impl->uc);
- if (IS_ERR(impl->uprobe)) {
- ret = PTR_ERR(impl->uprobe);
- path_put(&impl->pub.path);
- kfree(impl);
- return ERR_PTR(ret);
- }
-
- return &impl->pub;
-}
-
-/**
- * rv_uprobe_attach_path - register an uprobe given an already-resolved path
- */
-struct rv_uprobe *rv_uprobe_attach_path(struct path *path, loff_t offset,
- int (*entry_fn)(struct rv_uprobe *p, struct pt_regs *regs, __u64 *data),
- int (*ret_fn)(struct rv_uprobe *p, unsigned long func,
- struct pt_regs *regs, __u64 *data),
- void *priv)
-{
- struct inode *inode = d_real_inode(path->dentry);
-
- return __rv_uprobe_attach(inode, path, offset, entry_fn, ret_fn, priv);
-}
-EXPORT_SYMBOL_GPL(rv_uprobe_attach_path);
-
/**
- * rv_uprobe_attach - resolve binpath and register an uprobe
+ * rv_uprobe_register - resolve binpath and register an uprobe
*/
-struct rv_uprobe *rv_uprobe_attach(const char *binpath, loff_t offset,
- int (*entry_fn)(struct rv_uprobe *p, struct pt_regs *regs, __u64 *data),
- int (*ret_fn)(struct rv_uprobe *p, unsigned long func,
- struct pt_regs *regs, __u64 *data),
- void *priv)
+int rv_uprobe_register(const char *binpath, loff_t offset, struct rv_uprobe *p,
+ int (*handler)(struct uprobe_consumer *self, struct pt_regs *regs,
__u64 *data),
+ int (*ret_handler)(struct uprobe_consumer *self, unsigned long func,
+ struct pt_regs *regs, __u64 *data))
{
- struct rv_uprobe *p;
+ struct inode *inode;
struct path path;
int ret;
+ if (!handler && !ret_handler)
+ return -EINVAL;
+
ret = kern_path(binpath, LOOKUP_FOLLOW, &path);
if (ret)
- return ERR_PTR(ret);
+ return ret;
if (!d_is_reg(path.dentry)) {
path_put(&path);
- return ERR_PTR(-EINVAL);
+ return -EINVAL;
}
- p = rv_uprobe_attach_path(&path, offset, entry_fn, ret_fn, priv);
+ inode = d_real_inode(path.dentry);
+
+ p->uc.handler = handler;
+ p->uc.ret_handler = ret_handler;
+ p->inode = inode;
+
+ p->uprobe = uprobe_register(inode, offset, 0, &p->uc);
path_put(&path);
- return p;
+
+ if (IS_ERR(p->uprobe)) {
+ ret = PTR_ERR(p->uprobe);
+ p->uprobe = NULL;
+ p->inode = NULL;
+ return ret;
+ }
+
+ return 0;
}
-EXPORT_SYMBOL_GPL(rv_uprobe_attach);
+EXPORT_SYMBOL_GPL(rv_uprobe_register);
/**
- * rv_uprobe_detach - synchronously unregister an uprobe and free it
+ * rv_uprobe_unregister - synchronously unregister a uprobe
*/
-void rv_uprobe_detach(struct rv_uprobe *p)
+void rv_uprobe_unregister(struct rv_uprobe *p)
{
- if (!p)
+ if (!p || IS_ERR_OR_NULL(p->uprobe))
return;
rv_uprobe_unregister_nosync(p);
rv_uprobe_sync();
- rv_uprobe_free(p);
}
-EXPORT_SYMBOL_GPL(rv_uprobe_detach);
+EXPORT_SYMBOL_GPL(rv_uprobe_unregister);
/**
* rv_uprobe_unregister_nosync - dequeue an uprobe without waiting
*/
void rv_uprobe_unregister_nosync(struct rv_uprobe *p)
{
- struct rv_uprobe_impl *impl;
-
- if (!p)
+ if (!p || IS_ERR_OR_NULL(p->uprobe))
return;
- impl = container_of(p, struct rv_uprobe_impl, pub);
- uprobe_unregister_nosync(impl->uprobe, &impl->uc);
+ uprobe_unregister_nosync(p->uprobe, &p->uc);
+ p->uprobe = NULL;
+ p->inode = NULL;
}
EXPORT_SYMBOL_GPL(rv_uprobe_unregister_nosync);
@@ -164,19 +89,3 @@ void rv_uprobe_sync(void)
uprobe_unregister_sync();
}
EXPORT_SYMBOL_GPL(rv_uprobe_sync);
-
-/**
- * rv_uprobe_free - release resources of a previously deregistered probe
- */
-void rv_uprobe_free(struct rv_uprobe *p)
-{
- struct rv_uprobe_impl *impl;
-
- if (!p)
- return;
-
- impl = container_of(p, struct rv_uprobe_impl, pub);
- path_put(&p->path);
- kfree(impl);
-}
-EXPORT_SYMBOL_GPL(rv_uprobe_free);
