Re: [Qemu-devel] [PATCH 1/2] async: add aio_bh_schedule_oneshot

[Paolo Bonzini]

On 05/10/2016 16:20, Kevin Wolf wrote:
> Am 05.10.2016 um 15:55 hat Paolo Bonzini geschrieben:
>> On 05/10/2016 15:13, Stefan Hajnoczi wrote:
>>>> qemu_bh_delete is already clearing bh->scheduled at the same time
>>>> as it's setting bh->deleted.  Since it's not using any memory
>>>> barriers, there is no synchronization going on for bh->deleted,
>>>> and this makes the bh->deleted checks superfluous in aio_compute_timeout,
>>>> aio_bh_poll and aio_ctx_check.
>>>
>>> Yikes.  On one hand this sounds scary but in practice qemu_bh_delete()
>>> isn't called from another thread so the next aio_bh_poll() will indeed
>>> clean it up instead of dispatching a deleted BH.
>>>
>>> Due to the nature of this change I suggest making it in a separate
>>> patch.
>>
>> Separate from what?  (Sorry if I'm being dense).
>>
>>>>
>>>> + * aio_bh_schedule_oneshot: Allocate a new bottom half structure that 
>>>> will run
>>>> + * only once and as soon as possible.
>>>> + *
>>>> + * Bottom halves are lightweight callbacks whose invocation is guaranteed
>>>> + * to be wait-free, thread-safe and signal-safe.  The #QEMUBH structure
>>>> + * is opaque and must be allocated prior to its use.
>>>
>>> I'm confused.  There is no QEMUBH structure in this function
>>> prototype.  Is this comment from an earlier version of this function?
>>
>> No, it's from aio_bh_new.  Of course this one is neither wait-free nor
>> signal-safe.  Kevin, do you want me to respin?
> 
> If the comment is wrong, either post a v2 of this patch or just reply
> with a new version of the comment and I'll squash it in. Your choice, I
> don't mind either way.

Just removing those three lines would be okay.

Paolo