On Fri, Apr 01, 2022 at 01:01:53PM +0200, Paolo Bonzini wrote: > On 4/1/22 10:05, Emanuele Giuseppe Esposito wrote: > > > The list itself would be used internally to implement the write-side > > > lock and unlock primitives, but it would not be protected by the above > > > functions. So there would be a couple additional functions: > > > > > > bdrv_graph_list_lock <-> cpu_list_lock > > > bdrv_graph_list_unlock <-> cpu_list_unlock > > > > The list would be graph_bdrv_states, why do we need to protect it with a > > lock? Currently it is protected by BQL, and theoretically only > > bdrv_graph_wrlock iterates on it. And as we defined in the assertion > > below, wrlock is always in the main loop too. > > You're right, CPU_FOREACH only appears in start_exclusive; so likewise you > only need to walk the list in bdrv_graph_wrlock, i.e. only under BQL. > > My thought was that, within the implementation, you'll need a mutex to > protect has_waiter, and protecting the list with the same mutex made sense > to me. But indeed it's not necessary.
What is the relationship between this new API and aio_set_fd_handler()'s is_external? A few thoughts: - The new API doesn't stop more I/O requests from being submitted, it just blocks the current coroutine so request processing is deferred. - In other words, is_external is a flow control API whereas the new API queues up request coroutines without notifying the caller. - The new API still needs to be combined with bdrv_drained_begin/end() to ensure in-flight requests are done. - It's not obvious to me whether the new API obsoletes is_external. I think it probably doesn't. Stefan
signature.asc
Description: PGP signature
