|
From: | Paolo Bonzini |
Subject: | Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept |
Date: | Fri, 1 Apr 2022 13:01:53 +0200 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.7.0 |
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_unlockThe 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.
Paolo
+void bdrv_graph_list_rdlock(BlockDriverState *bs); +void bdrv_graph_list_rdunlock(BlockDriverState *bs);Apart from the naming change, these two would be coroutine_fn.+#define BS_GRAPH_READER(bs) /* in main loop OR bs->reading_graph */ +#define BS_GRAPH_WRITER(bs) /* in main loop AND bs->bs_graph_pending_opbs_graph_pending_op is not part of bs->, it is a global variable (corresponding to pending_cpus in cpus-common.c). I would call it bs_graph_pending_reader since you have "has_writer" below.
[Prev in Thread] | Current Thread | [Next in Thread] |