On Tue, May 23, 2023 at 06:17:42PM +0200, Thomas Gleixner wrote:
> 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.
>
Reviewed-by: Uladzislau Rezki (Sony) <urezki@xxxxxxxxx>
