Re: [RFC PATCH v2 3/8] block: introduce a lock to protect graph operations

[Stefan Hajnoczi]

On Mon, May 02, 2022 at 09:54:14AM +0200, Emanuele Giuseppe Esposito wrote:
> 
> 
> Am 30/04/2022 um 07:48 schrieb Stefan Hajnoczi:
> > On Fri, Apr 29, 2022 at 10:37:54AM +0200, Emanuele Giuseppe Esposito wrote:
> >> Am 28/04/2022 um 15:45 schrieb Stefan Hajnoczi:
> >>> On Tue, Apr 26, 2022 at 04:51:09AM -0400, Emanuele Giuseppe Esposito 
> >>> wrote:
> >>>> +static int has_writer;
> >>>
> >>> bool?
> >>
> >> Yes and no. With the latest findings and current implementation we could
> >> have something like:
> >>
> >> wrlock()
> >>    has_writer = 1
> >>    AIO_WAIT_WHILE(reader_count >=1) --> job_exit()
> >>                                            wrlock()
> >>
> >> But we are planning to get rid of AIO_WAIT_WHILE and allow wrlock to
> >> only run in coroutines. This requires a lot of changes, and switch a lot
> >> of callbacks in coroutines, but then we would avoid having such problems
> >> and nested event loops.
> > 
> > I don't understand how this answer is related to the question about
> > whether the type of has_writer should be bool?
> 
> Yes sorry I did not conclude the explanation, but taking into account
> the above case we would have an assertion failure `assert(!has_writer)`
> in bdrv_graph_wrlock(), and just removing that would make the lock
> inconsistent because the first unlock() would reset the flag to
> zero/false and forget about the previous wrlock().
> Example:
> 
> wrlock()
>       has_writer = 1
>       AIO_WAIT_WHILE(reader_count >=1) --> job_exit()
>                                               wrlock()
>                                                       has_writer = 1
>                                               /* performs a write */
>                                               wrunlock()
>                                                       has_writer = 0
>                                       <---
>       /* performs a write but has_writer = 0! */

How is this related to the question of whether has_writer should be bool
instead of int?

Are you saying has_writer needs to be a recursive lock and is therefore
a counter? If yes, please revisit the cover letter, which says the lock
must not be recursive. I'm confused.

Stefan

signature.asc
Description: PGP signature