Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

On 4/13/22 16:51, Kevin Wolf wrote: So the idea is that we can do bdrv_graph_co_rdlock() in one thread and the corresponding bdrv_graph_co_rdunlock() in a different thread? Would the unlock somehow remember the original thread, or do you use the "sum is correct" argument and allow negative

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

On 4/13/22 18:29, Kevin Wolf wrote: A reader does not have to be a coroutine. AIO_WAIT_WHILE is not mandatory to allow it to finish, it helps to ensure progress in case some reader is waiting for something, but other than that is not necessary IMO. When it's outside of a coroutine, how would

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

Am 13.04.2022 um 17:14 hat Emanuele Giuseppe Esposito geschrieben: > Am 13/04/2022 um 16:51 schrieb Kevin Wolf: > > Am 13.04.2022 um 15:43 hat Emanuele Giuseppe Esposito geschrieben: > >> So this is a more concrete and up-to-date header. > >> > >> Few things to notice: > >> - we have a list of

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

Am 13/04/2022 um 17:14 schrieb Emanuele Giuseppe Esposito: > > > Am 13/04/2022 um 16:51 schrieb Kevin Wolf: >> Am 13.04.2022 um 15:43 hat Emanuele Giuseppe Esposito geschrieben: >>> So this is a more concrete and up-to-date header. >>> >>> Few things to notice: >>> - we have a list of

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

Am 13/04/2022 um 16:51 schrieb Kevin Wolf: > Am 13.04.2022 um 15:43 hat Emanuele Giuseppe Esposito geschrieben: >> So this is a more concrete and up-to-date header. >> >> Few things to notice: >> - we have a list of AioContext. They are registered once an aiocontext >> is created, and deleted

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

Am 13.04.2022 um 15:43 hat Emanuele Giuseppe Esposito geschrieben: > So this is a more concrete and up-to-date header. > > Few things to notice: > - we have a list of AioContext. They are registered once an aiocontext > is created, and deleted when it is destroyed. > This list is helpful because

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

So this is a more concrete and up-to-date header. Few things to notice: - we have a list of AioContext. They are registered once an aiocontext is created, and deleted when it is destroyed. This list is helpful because each aiocontext can only modify its own number of readers, avoiding unnecessary

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

Am 30.03.2022 um 11:58 hat Emanuele Giuseppe Esposito geschrieben: > > > Am 30/03/2022 um 11:52 schrieb Vladimir Sementsov-Ogievskiy: > > 30.03.2022 12:09, Emanuele Giuseppe Esposito wrote: > >>> > >>> Ah seems I understand what you mean. > >>> > >>> One of my arguments is that "drain" - is not

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

Am 04.04.2022 um 11:41 hat Paolo Bonzini geschrieben: > As an aside, instead of is_external, QEMU could remove/add the ioeventfd > handler in the blk->dev_ops->drained_begin and blk->dev_ops->drained_end > callbacks respectively. But that's just a code cleanup. Yes, this is the proper way to do

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

On Mon, Apr 04, 2022 at 11:41:04AM +0200, Paolo Bonzini wrote: > On Mon, Apr 4, 2022 at 11:25 AM Stefan Hajnoczi wrote: > > - The new API still needs to be combined with bdrv_drained_begin/end() > > to ensure in-flight requests are done. > > > > I don't think so, because in-flight requests

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

On 4/4/22 11:51, Emanuele Giuseppe Esposito wrote: >> >> I agree that it doesn't. This new lock is only protecting ->parents and >> ->children. > Side note: it will also be used to protect other fields, like > .aio_context I think. I haven't checked if there is something else we > might want to

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

Am 04/04/2022 um 11:41 schrieb Paolo Bonzini: > > > On Mon, Apr 4, 2022 at 11:25 AM Stefan Hajnoczi > wrote: > > - The new API doesn't stop more I/O requests from being submitted, it >   just blocks the current coroutine so request processing is deferred.

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

On Mon, Apr 4, 2022 at 11:25 AM Stefan Hajnoczi wrote: > - The new API doesn't stop more I/O requests from being submitted, it > just blocks the current coroutine so request processing is deferred. > New I/O requests would not complete until the write-side critical section ends. However they

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

On Fri, Apr 01, 2022 at 01:01:53PM +0200, Paolo Bonzini wrote: > 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

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

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  

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

Am 31/03/2022 um 18:40 schrieb Paolo Bonzini: > On 3/31/22 15:51, Emanuele Giuseppe Esposito wrote: >> >> bdrv_graph_list_wrlock <-> start_exclusive >> bdrv_graph_list_wrunlock <-> end_exclusive >> bdrv_graph_list_rdlock <-> cpu_exec_start >> bdrv_graph_list_rdunlock <-> cpu_exec_end > > This

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

On 3/31/22 15:51, Emanuele Giuseppe Esposito wrote: bdrv_graph_list_wrlock <-> start_exclusive bdrv_graph_list_wrunlock <-> end_exclusive bdrv_graph_list_rdlock <-> cpu_exec_start bdrv_graph_list_rdunlock <-> cpu_exec_end This wouldn't protect the list but the whole graph, i.e. the parents

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

Am 31/03/2022 um 11:59 schrieb Paolo Bonzini: > On 3/30/22 18:02, Paolo Bonzini wrote: >> On 3/30/22 12:53, Hanna Reitz wrote: Seems a good compromise between drains and rwlock. What do you think? >>> >>> Well, sounds complicated.  So I’m asking myself whether this would be >>>

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

On 3/30/22 18:02, Paolo Bonzini wrote: On 3/30/22 12:53, Hanna Reitz wrote: Seems a good compromise between drains and rwlock. What do you think? Well, sounds complicated.  So I’m asking myself whether this would be noticeably better than just an RwLock for graph modifications, like the

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

On 3/30/22 12:53, Hanna Reitz wrote: Seems a good compromise between drains and rwlock. What do you think? Well, sounds complicated.  So I’m asking myself whether this would be noticeably better than just an RwLock for graph modifications, like the global lock Vladimir has proposed. A

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

On 30.03.22 13:55, Emanuele Giuseppe Esposito wrote: Am 30/03/2022 um 12:53 schrieb Hanna Reitz: On 17.03.22 17:23, Emanuele Giuseppe Esposito wrote: Am 09/03/2022 um 14:26 schrieb Emanuele Giuseppe Esposito: * Drains allow the caller (either main loop or iothread running the context) to

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

Am 30/03/2022 um 12:53 schrieb Hanna Reitz: > On 17.03.22 17:23, Emanuele Giuseppe Esposito wrote: >> >> Am 09/03/2022 um 14:26 schrieb Emanuele Giuseppe Esposito: > * Drains allow the caller (either main loop or iothread running > the context) to wait all in_flights requests and

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

On 17.03.22 17:23, Emanuele Giuseppe Esposito wrote: Am 09/03/2022 um 14:26 schrieb Emanuele Giuseppe Esposito: * Drains allow the caller (either main loop or iothread running the context) to wait all in_flights requests and operations of a BDS: normal drains target a given node and is

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

Am 30/03/2022 um 11:52 schrieb Vladimir Sementsov-Ogievskiy: > 30.03.2022 12:09, Emanuele Giuseppe Esposito wrote: >>> >>> Ah seems I understand what you mean. >>> >>> One of my arguments is that "drain" - is not a lock against other >>> clients who want to modify the graph. Because, drained

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

30.03.2022 12:09, Emanuele Giuseppe Esposito wrote: Ah seems I understand what you mean. One of my arguments is that "drain" - is not a lock against other clients who want to modify the graph. Because, drained section allows nested drained sections. And you try to solve it, by draining more

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

> > Ah seems I understand what you mean. > > One of my arguments is that "drain" - is not a lock against other > clients who want to modify the graph. Because, drained section allows > nested drained sections. > > And you try to solve it, by draining more things, this way, we'll drain > also the

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

09.03.2022 16:26, Emanuele Giuseppe Esposito wrote: Am 02/03/2022 um 12:07 schrieb Vladimir Sementsov-Ogievskiy: 01.03.2022 17:21, Emanuele Giuseppe Esposito wrote: This serie tries to provide a proof of concept and a clear explanation on why we need to use drains (and more precisely

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

09.03.2022 16:26, Emanuele Giuseppe Esposito wrote: Am 02/03/2022 um 12:07 schrieb Vladimir Sementsov-Ogievskiy: 01.03.2022 17:21, Emanuele Giuseppe Esposito wrote: This serie tries to provide a proof of concept and a clear explanation on why we need to use drains (and more precisely

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

17.03.2022 00:55, Emanuele Giuseppe Esposito wrote: Am 09/03/2022 um 14:26 schrieb Emanuele Giuseppe Esposito: Next, I have a problem in mind, that in past lead to a lot of iotest 30 failures. Next there were different fixes and improvements, but the core problem (as far as I understand) is

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

Am 09/03/2022 um 14:26 schrieb Emanuele Giuseppe Esposito: >>> * Drains allow the caller (either main loop or iothread running >>> the context) to wait all in_flights requests and operations >>> of a BDS: normal drains target a given node and is parents, while >>> subtree ones also include the

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

Am 09/03/2022 um 14:26 schrieb Emanuele Giuseppe Esposito: >> Next, I have a problem in mind, that in past lead to a lot of iotest 30 >> failures. Next there were different fixes and improvements, but the core >> problem (as far as I understand) is still here: nothing protects us when >> we are

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

On Wed, Mar 09, 2022 at 02:26:28PM +0100, Emanuele Giuseppe Esposito wrote: > Am 02/03/2022 um 10:47 schrieb Stefan Hajnoczi: > > On Tue, Mar 01, 2022 at 09:21:08AM -0500, Emanuele Giuseppe Esposito wrote: > >> Possible scenarios > >> --- > >> Keeping in mind that we can only have

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

Am 02/03/2022 um 10:47 schrieb Stefan Hajnoczi: > On Tue, Mar 01, 2022 at 09:21:08AM -0500, Emanuele Giuseppe Esposito wrote: >> This serie tries to provide a proof of concept and a clear explanation >> on why we need to use drains (and more precisely subtree_drains) >> to replace the

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

Am 02/03/2022 um 12:07 schrieb Vladimir Sementsov-Ogievskiy: > 01.03.2022 17:21, Emanuele Giuseppe Esposito wrote: >> This serie tries to provide a proof of concept and a clear explanation >> on why we need to use drains (and more precisely subtree_drains) >> to replace the aiocontext lock,

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

On Wed, Mar 02, 2022 at 02:07:30PM +0300, Vladimir Sementsov-Ogievskiy wrote: > 01.03.2022 17:21, Emanuele Giuseppe Esposito wrote: > > This serie tries to provide a proof of concept and a clear explanation > > on why we need to use drains (and more precisely subtree_drains) > > to replace the

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

01.03.2022 17:21, Emanuele Giuseppe Esposito wrote: This serie tries to provide a proof of concept and a clear explanation on why we need to use drains (and more precisely subtree_drains) to replace the aiocontext lock, especially to protect BlockDriverState ->children and ->parent lists. Just

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

On Tue, Mar 01, 2022 at 09:21:08AM -0500, Emanuele Giuseppe Esposito wrote: > This serie tries to provide a proof of concept and a clear explanation > on why we need to use drains (and more precisely subtree_drains) > to replace the aiocontext lock, especially to protect BlockDriverState >

Re: [RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

I would really love to hear opinions on this, since we already had some discussions on other similar patches. Thank you, Emanuele On 01/03/2022 15:21, Emanuele Giuseppe Esposito wrote: > This serie tries to provide a proof of concept and a clear explanation > on why we need to use drains (and

[RFC PATCH 0/5] Removal of AioContext lock, bs->parents and ->children: proof of concept

This serie tries to provide a proof of concept and a clear explanation on why we need to use drains (and more precisely subtree_drains) to replace the aiocontext lock, especially to protect BlockDriverState ->children and ->parent lists. Just a small recap on the key concepts: * We split block