Thread (67 messages) 67 messages, 9 authors, 2024-07-10

Re: [PATCH v2 05/12] uprobes: move offset and ref_ctr_offset into uprobe_consumer

From: Oleg Nesterov <oleg@redhat.com>
Date: 2024-07-07 12:50:35
Also in: bpf

On 07/01, Andrii Nakryiko wrote:
quoted hunk ↗ jump to hunk
--- a/include/linux/uprobes.h
+++ b/include/linux/uprobes.h
@@ -42,6 +42,11 @@ struct uprobe_consumer {
 				enum uprobe_filter_ctx ctx,
 				struct mm_struct *mm);
 
+	/* associated file offset of this probe */
+	loff_t offset;
+	/* associated refctr file offset of this probe, or zero */
+	loff_t ref_ctr_offset;
+	/* for internal uprobe infra use, consumers shouldn't touch fields below */
 	struct uprobe_consumer *next;

Well, I don't really like this patch either...

If nothing else because all the consumers in uprobe->consumers list
must have the same offset/ref_ctr_offset.

--------------------------------------------------------------------------
But I agree, the ugly uprobe_register_refctr() must die, we need a single
function

	int uprobe_register(inode, offset, ref_ctr_offset, consumer);

This change is trivial.

--------------------------------------------------------------------------
And speaking of cleanups, I think another change makes sense:

	-	int uprobe_register(...);
	+	struct uprobe* uprobe_register(...);

so that uprobe_register() returns uprobe or ERR_PTR.

	-	void uprobe_unregister(inode, offset, consumer);
	+	void uprobe_unregister(uprobe, consumer);

this way unregister() doesn't need the extra find_uprobe() + put_uprobe().
The same for uprobe_apply().

The necessary changes in kernel/trace/trace_uprobe.c are trivial, we just
need to change struct trace_uprobe

	-	struct inode                    *inode;
	+	struct uprobe			*uprobe;

and fix the compilation errors.


As for kernel/trace/bpf_trace.c, I guess struct bpf_uprobe  needs the new
->uprobe member, we can't kill bpf_uprobe->offset because of
bpf_uprobe_multi_link_fill_link_info(), but I think this is not that bad.

What do you think?

Oleg.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help