Re: [PATCHv3 0/2]

Sean Christopherson
2025-02-28T15:29:47Z

On Fri, Feb 28, 2025, Keith Busch wrote:
> On Fri, Feb 28, 2025 at 06:32:47AM -0800, Sean Christopherson wrote:
> > > diff --git a/include/linux/call_once.h b/include/linux/call_once.h
> > > index ddcfd91493ea..b053f4701c94 100644
> > > --- a/include/linux/call_once.h
> > > +++ b/include/linux/call_once.h
> > > @@ -35,10 +35,12 @@ static inline int call_once(struct once *once, int (*cb)(struct once *))
> > > return 0;
> > >
> > > guard(mutex)(&once->lock);
> > > - WARN_ON(atomic_read(&once->state) == ONCE_RUNNING);
> > > - if (atomic_read(&once->state) != ONCE_NOT_STARTED)
> > > + if (WARN_ON(atomic_read(&once->state) == ONCE_RUNNING))
> > > return -EINVAL;
> > >
> > > + if (atomic_read(&once->state) == ONCE_COMPLETED)
> > > + return 0;
> > > +
> > > atomic_set(&once->state, ONCE_RUNNING);
> > > r = cb(once);
> > > if (r)
>
> Possible suggestion since it seems odd to do an atomic_read twice on the
> same value.

Yeah, good call. At the risk of getting too cute, how about this?

static inline int call_once(struct once *once, int (*cb)(struct once *))
{
int r, state;

/* Pairs with atomic_set_release() below. */
if (atomic_read_acquire(&once->state) == ONCE_COMPLETED)
return 0;

guard(mutex)(&once->lock);
state = atomic_read(&once->state);
if (unlikely(state != ONCE_NOT_STARTED))
return WARN_ON_ONCE(state != ONCE_COMPLETED) ? -EINVAL : 0;

atomic_set(&once->state, ONCE_RUNNING);
r = cb(once);
if (r)
atomic_set(&once->state, ONCE_NOT_STARTED);
else
atomic_set_release(&once->state, ONCE_COMPLETED);
return r;
}