Re: [PATCHv3 0/2]

Paolo Bonzini
2025-02-28T16:43:49Z

On 2/28/25 16:36, Keith Busch wrote:
> On Fri, Feb 28, 2025 at 07:29:45AM -0800, Sean Christopherson wrote:
>> On Fri, Feb 28, 2025, Keith Busch wrote:
>>> On Fri, Feb 28, 2025 at 06:32:47AM -0800, Sean Christopherson wrote:
>>>>> @@ -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?
>
> Sure, that also looks good to me.

Just to overthink it a bit more, I'm changing "if (r)" to "if (r < 0)".
Not because it's particularly useful to return a meaningful nonzero
value on the first initialization, but more because 0+ for success and
-errno for failure is a more common.

Queued with this change, thanks.

(Keith, I haven't forgotten about AVX by the way).

Paolo