Re: [PATCH 07/20] graph-lock: Fix GRAPH_RDLOCK_GUARD*() to be reader lock

[Eric Blake]

On Tue, Apr 25, 2023 at 07:31:45PM +0200, Kevin Wolf wrote:
> GRAPH_RDLOCK_GUARD() and GRAPH_RDLOCK_GUARD_MAINLOOP() only take a
> reader lock for the graph, so the correct annotation for them to use is
> TSA_ASSERT_SHARED rather than TSA_ASSERT.

The comments at the start of graph-lock.h state that there is only 1
writer (main loop, under BQL), and all others are readers (coroutines
in varous AioContext) - but that's regarding the main BdrvGraphRWlock.
I guess my confusion is over the act of writing the graph (only in the
main loop) and using TSA annotations to check for safety.  Am I
correct that the reason graph_lockable_auto_lock() only needs a
TSA_ASSERT_SHARED locking is that it is only reachable from the other
threads (and not the main loop thread itself) to check that we are
indeed in a point where we aren't contending with the main loop's
writable lock?

> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  include/block/graph-lock.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/block/graph-lock.h b/include/block/graph-lock.h
> index 7ef391fab3..adaa3ed089 100644
> --- a/include/block/graph-lock.h
> +++ b/include/block/graph-lock.h
> @@ -210,7 +210,7 @@ typedef struct GraphLockable { } GraphLockable;
>   * unlocked. TSA_ASSERT() makes sure that the following calls know that we
>   * hold the lock while unlocking is left unchecked.
>   */
> -static inline GraphLockable * TSA_ASSERT(graph_lock) TSA_NO_TSA
> +static inline GraphLockable * TSA_ASSERT_SHARED(graph_lock) TSA_NO_TSA
>  graph_lockable_auto_lock(GraphLockable *x)
>  {
>      bdrv_graph_co_rdlock();

But the act of grabbing a rdlock is definitely a SHARED not EXCLUSIVE
action, so I think I agree with your changing of the annotation, even
if the commit message could be slightly improved.

Assuming I've sorted out my own confusion on the various lock
terminoligies,

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org