[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 00/18] block: Introduce a block graph rwlock
From: |
Kevin Wolf |
Subject: |
Re: [PATCH 00/18] block: Introduce a block graph rwlock |
Date: |
Mon, 12 Dec 2022 18:14:52 +0100 |
Am 07.12.2022 um 15:12 hat Emanuele Giuseppe Esposito geschrieben:
> Am 07/12/2022 um 14:18 schrieb Kevin Wolf:
> > This series supersedes the first half of Emanuele's "Protect the block
> > layer with a rwlock: part 1". It introduces the basic infrastructure for
> > protecting the block graph (specifically parent/child links) with a
> > rwlock. Actually taking the reader lock in all necessary places is left
> > for future series.
> >
> > Compared to Emanuele's series, this one adds patches to make use of
> > clang's Thread Safety Analysis (TSA) feature in order to statically
> > check at compile time that the places where we assert that we hold the
> > lock actually do hold it. Once we cover all relevant places, the check
> > can be extended to verify that all accesses of bs->children and
> > bs->parents hold the lock.
> >
> > For reference, here is the more detailed version of our plan in
> > Emanuele's words from his series:
> >
> > The aim is to replace the current AioContext lock with much
> > fine-grained locks, aimed to protect only specific data. Currently
> > the AioContext lock is used pretty much everywhere, and it's not
> > even clear what it is protecting exactly.
> >
> > The aim of the rwlock is to cover graph modifications: more
> > precisely, when a BlockDriverState parent or child list is modified
> > or read, since it can be concurrently accessed by the main loop and
> > iothreads.
> >
> > The main assumption is that the main loop is the only one allowed to
> > perform graph modifications, and so far this has always been held by
> > the current code.
> >
> > The rwlock is inspired from cpus-common.c implementation, and aims
> > to reduce cacheline bouncing by having per-aiocontext counter of
> > readers. All details and implementation of the lock are in patch 2.
> >
> > We distinguish between writer (main loop, under BQL) that modifies
> > the graph, and readers (all other coroutines running in various
> > AioContext), that go through the graph edges, reading ->parents
> > and->children. The writer (main loop) has an "exclusive" access,
> > so it first waits for current read to finish, and then prevents
> > incoming ones from entering while it has the exclusive access. The
> > readers (coroutines in multiple AioContext) are free to access the
> > graph as long the writer is not modifying the graph. In case it is,
> > they go in a CoQueue and sleep until the writer is done.
> >
> > In this and following series, we try to follow the following locking
> > pattern:
> >
> > - bdrv_co_* functions that call BlockDriver callbacks always expect
> > the lock to be taken, therefore they assert.
> >
> > - blk_co_* functions are called from external code outside the block
> > layer, which should not have to care about the block layer's
> > internal locking. Usually they also call blk_wait_while_drained().
> > Therefore they take the lock internally.
> >
> > The long term goal of this series is to eventually replace the
> > AioContext lock, so that we can get rid of it once and for all.
> >
> > Emanuele Giuseppe Esposito (7):
> > graph-lock: Implement guard macros
> > async: Register/unregister aiocontext in graph lock list
> > block: wrlock in bdrv_replace_child_noperm
> > block: remove unnecessary assert_bdrv_graph_writable()
> > block: assert that graph read and writes are performed correctly
> > block-coroutine-wrapper.py: introduce annotations that take the graph
> > rdlock
> > block: use co_wrapper_mixed_bdrv_rdlock in functions taking the rdlock
> >
> > Kevin Wolf (10):
> > block: Factor out bdrv_drain_all_begin_nopoll()
> > Import clang-tsa.h
> > clang-tsa: Add TSA_ASSERT() macro
> > clang-tsa: Add macros for shared locks
> > configure: Enable -Wthread-safety if present
> > test-bdrv-drain: Fix incorrrect drain assumptions
> > block: Fix locking in external_snapshot_prepare()
> > graph-lock: TSA annotations for lock/unlock functions
> > Mark assert_bdrv_graph_readable/writable() GRAPH_RD/WRLOCK
> > block: GRAPH_RDLOCK for functions only called by co_wrappers
> >
> > Paolo Bonzini (1):
> > graph-lock: Introduce a lock to protect block graph operations
> >
> Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Thanks, applied to block-next.
Kevin
- [PATCH 06/18] clang-tsa: Add TSA_ASSERT() macro, (continued)
- [PATCH 06/18] clang-tsa: Add TSA_ASSERT() macro, Kevin Wolf, 2022/12/07
- [PATCH 14/18] graph-lock: TSA annotations for lock/unlock functions, Kevin Wolf, 2022/12/07
- [PATCH 13/18] block: assert that graph read and writes are performed correctly, Kevin Wolf, 2022/12/07
- [PATCH 16/18] block-coroutine-wrapper.py: introduce annotations that take the graph rdlock, Kevin Wolf, 2022/12/07
- [PATCH 15/18] Mark assert_bdrv_graph_readable/writable() GRAPH_RD/WRLOCK, Kevin Wolf, 2022/12/07
- [PATCH 17/18] block: use co_wrapper_mixed_bdrv_rdlock in functions taking the rdlock, Kevin Wolf, 2022/12/07
- [PATCH 18/18] block: GRAPH_RDLOCK for functions only called by co_wrappers, Kevin Wolf, 2022/12/07
- [PATCH 09/18] test-bdrv-drain: Fix incorrrect drain assumptions, Kevin Wolf, 2022/12/07
- Re: [PATCH 00/18] block: Introduce a block graph rwlock, Emanuele Giuseppe Esposito, 2022/12/07