Re: [patch 4/6] mm/vmalloc: Check free space in vmap_block lockless

Christoph Hellwig
2023-05-23T15:29:45Z

On Tue, May 23, 2023 at 04:02:14PM +0200, Thomas Gleixner wrote:
> + if (READ_ONCE(vb->free) < (1UL << order))
> + continue;
> +
> spin_lock(&vb->lock);
> if (vb->free < (1UL << order)) {
> spin_unlock(&vb->lock);
> @@ -2174,7 +2177,7 @@ static void *vb_alloc(unsigned long size
>
> pages_off = VMAP_BBMAP_BITS - vb->free;
> vaddr = vmap_block_vaddr(vb->va->va_start, pages_off);
> - vb->free -= 1UL << order;
> + WRITE_ONCE(vb->free, vb->free - (1UL << order));

Maybe just a matter of preference, but wouldn't an atomic_t be
better here? We'd have another locked instruction in the alloc
path, but I always find the READ_ONCE/WRITE_ONCE usage a bit
fragile that I'd rather reserve them to well documented hot
path code.