Re: [PATCH v2 tip/perf/core 5/5] uprobes: add speculative lockless VMA-to-inode-to-uprobe resolution
From: Oleg Nesterov
Date: Thu Oct 03 2024 - 05:33:08 EST
On 10/02, Andrii Nakryiko wrote:
>
> On Wed, Oct 2, 2024 at 12:25 AM Oleg Nesterov <oleg@xxxxxxxxxx> wrote:
> >
> > > + vm_file = READ_ONCE(vma->vm_file);
> > > + if (!vm_file)
> > > + return NULL;
> > > +
> > > + offset = (loff_t)(vma->vm_pgoff << PAGE_SHIFT) + (bp_vaddr - vma->vm_start);
> >
> > LGTM. But perhaps vma->vm_pgoff and vma->vm_start need READ_ONCE() as well,
> > if nothing else to shut up KCSAN if this code races with, say, __split_vma() ?
>
> We keep going back and forth between reading directly, using
> READ_ONCE(), and annotating with data_race(). I don't think it matters
> in terms of correctness or performance, so I'm happy to add whatever
> incantations that will make everyone satisfied. Let's see what others
> think, and I'll incorporate that into the next revision.
OK, agreed...
And I guess I was wrong anyway, READ_ONCE() alone won't shutup KCSAN in
this case.
Oleg.