On 25/01/19 11:30, Kevin Wolf wrote: >> On the other hand, the AioContext lock is only used in >> some special cases around block jobs and blk_set_aio_context, and in >> general the block devices already should not have any dependencies >> (unless they crept in without me noticing). > It's also used in those cases where coroutines don't need locking, but > threads would. Did you audit all of the drivers for such cases?
I did and the drivers already have a QemuMutex if that's the case (e.g. curl, iscsi). >> In particular... >> >>> But raw doesn't have an s->lock yet, so I >>> think removing the AioContext lock involves some work on it anyway and >>> adding this doesn't really change the amount of work. >> ... BDRVRawState doesn't have any data that changes after open, does it? >> This is why it doesn't have an s->lock. > No important data anyway. We do things like setting s->has_write_zeroes > = false after failure, but if we got a race and end up trying twice > before disabling it, it doesn't really hurt either. > > Then there is reopen, but that involves a drain anyway. And that's it > probably. > > So do you think I should introduce a CoMutex for raw here? Or QemuMutex? For the cache you can introduce either a CoMutex or QemuMutex, it's the same. Paolo
