On Fri, Nov 03, 2023 at 10:45:11AM +0100, Kevin Wolf wrote: > Am 27.10.2023 um 22:22 hat Eric Blake geschrieben: > > On Fri, Oct 27, 2023 at 05:53:13PM +0200, Kevin Wolf wrote: > > > Instead of taking the writer lock internally, require callers to already > > > hold it when calling bdrv_root_attach_child(). These callers will > > > typically already hold the graph lock once the locking work is > > > completed, which means that they can't call functions that take it > > > internally. > > > > > > Signed-off-by: Kevin Wolf <kw...@redhat.com> > > > --- > > > include/block/block_int-global-state.h | 13 +++++++------ > > > block.c | 5 +---- > > > block/block-backend.c | 2 ++ > > > blockjob.c | 2 ++ > > > 4 files changed, 12 insertions(+), 10 deletions(-) > > > > > > +++ b/block.c > > > @@ -3214,8 +3214,6 @@ BdrvChild *bdrv_root_attach_child(BlockDriverState > > > *child_bs, > > > > > > GLOBAL_STATE_CODE(); > > > > > > - bdrv_graph_wrlock(child_bs); > > > - > > > child = bdrv_attach_child_common(child_bs, child_name, child_class, > > > > Do we need some sort of assertion that the caller did indeed grab the > > lock at this point? Or is that redundant with assertions made in all > > helper functions we are calling where the lock already matters? > > The GRAPH_WRLOCK in the header file makes it a compiler error with TSA > enabled if the caller doesn't already hold the lock. And our CI has some > builds with TSA, so even if the maintainer only uses gcc, we'll catch > it.
TSA is awesome - guaranteeing code correctness during CI has been an awesome result of this massive refactoring effort. -- Eric Blake, Principal Software Engineer Red Hat, Inc. Virtualization: qemu.org | libguestfs.org
