Re: [PATCH] mm/vma: avoid spurious write locks when mm->mm_lock_seq overflows

[Jose A. Perez de Azpillaga]

On 10/15/26 10:31 PM, Lorenzo Stoakes (ARM) wrote:
[snip]

> The reset, performed in reset_vma_seqcounts(), only needs to WRITE_ONCE()
> the value. The stores follow the smp_wmb() in do_raw_write_seqcount_end(),
> so a reader observing a reset value also observes the VMA modifications
> made under the write lock.

the writer side looks fine, but what orders it on the reader side?
raw_read_seqcount() is just an smp_load_acquire() since d0dd066a0fa2,
not the smp_rmb() the comment mentions, so the earlier
READ_ONCE(vma->vm_lock_seq) isn't ordered against the later vma reads.
couldn't a reader on arm64 see the reset 0 with the old odd
mm_lock_seq, pass the parity check and read stale vma fields? would
smp_load_acquire(&vma->vm_lock_seq) be enough?

[snip]

> Cc: stable@xxxxxxxxxxxxxxx

the parity check relies on e5e7fb278e59. before that, mm_lock_seq only
increments on unlock, so locked values can be odd or even. should this
be limited to 6.14+, with a note that older trees need a different fix?
a straight backport of the odd check would ignore write locks taken on
even cycles.

[snip]

> +	/*
> +	 * The VMA seqcount is always odd if locked. An even value can only be
> +	 * the zero stored by reset_vma_seqcounts(), never a write lock.
> +	 */

vma_lock_init() now stores 0 too, so this isn't quite true, is it?

[snip]

> -static inline struct vm_area_struct *vma_start_read(struct mm_struct *mm,
> -						    struct vm_area_struct *vma)
> +static struct vm_area_struct *vma_start_read(struct mm_struct *mm,
> +					     struct vm_area_struct *vma)

is dropping the inline intended?

[snip]

> +EXPORT_SYMBOL(reset_vma_seqcounts);

since it has to be exported for the inline mmap_write_unlock(), would a
__ prefix make sense, so nobody calls it outside vma_end_write_all()?

I tested it on x86 in qemu with KASAN, lockdep and DEBUG_VM on top of
mm-unstable and saw no issues, but I'm hesitant about the points above.
happy to give a Tested-by on the next version :)

-- 
cheers,
jose a. p-a