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

Thomas Gleixner
2023-05-23T16:17:47Z

On Tue, May 23 2023 at 17:29, Christoph Hellwig wrote:
> 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.

I don't see a problem with these lockless quickchecks, especially not
in this particular case, but no strong opinion either.

Thanks

tglx