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

[Thomas Gleixner <tglx@linutronix.de>]

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