On Wed, Jan 15, 2025 at 7:38 AM Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote: > > On Wed, Jan 15, 2025 at 04:35:07PM +0100, Peter Zijlstra wrote: > > > Consider: > > > > CPU0 CPU1 > > > > rcu_read_lock(); > > vma = vma_lookup(mm, vaddr); > > > > ... cpu goes sleep for a *long time* ... > > > > __vma_exit_locked(); > > vma_area_free() > > .. > > vma = vma_area_alloc(); > > vma_mark_attached(); > > > > ... comes back once vma is re-used ... > > > > vma_start_read() > > vm_refcount_inc(); // success!! > > > > At which point we need to validate vma is for mm and covers vaddr, which > > is what patch 15 does, no? Correct. Sorry, I thought by "secondary validation" you only meant vm_lock_seq check in vma_start_read(). Now I understand your point. Yes, if the vma we found gets reused before we read-lock it then the checks for mm and address range should catch a possibly incorrect vma. If these checks fail, we retry. If they succeed we have the correct vma even if it was recycled since we found it. > > Also, critically, we want these reads to happen *after* the refcount > increment. Yes, and I think the acquire fence in the refcount_add_not_zero_limited() replacement should guarantee that ordering. >
