Thread (29 messages) flat view 29 messages, 6 authors, 2024-10-25

Re: [PATCH v3 tip/perf/core 4/4] uprobes: add speculative lockless VMA-to-inode-to-uprobe resolution

From: Peter Zijlstra <peterz@infradead.org>
Date: 2024-10-23 20:19:41
Also in: bpf, linux-mm, lkml

On Wed, Oct 23, 2024 at 01:02:53PM -0700, Andrii Nakryiko wrote:
quoted
quoted
+      * but can't be freed from under us, so it's safe to read fields from
+      * it, even if the values are some garbage values; ultimately
+      * find_uprobe_rcu() + mmap_lock_speculation_end() check will ensure
+      * that whatever we speculatively found is correct
+      */
+     vm_file = READ_ONCE(vma->vm_file);
+     if (!vm_file)
+             return NULL;
+
+     vm_pgoff = data_race(vma->vm_pgoff);
+     vm_start = data_race(vma->vm_start);
+     vm_inode = data_race(vm_file->f_inode);
So... seqcount has kcsan annotations other than data_race(). I suppose
this works, but it all feels like a bad copy with random changes.
I'm not sure what this means... Do I need to change anything? Drop
data_race()? Use READ_ONCE()? Do nothing?
Keep for now. I've ranted at 1/n a bit, but unless the response is:
yeah, obviously this should be seqcount (unlikely) this is something
that can be fixed later (*sigh* always later... this todo list is a
problem).
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help