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
