[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 00/15] Protect the block layer with a rwlock: part 3
From: |
Paolo Bonzini |
Subject: |
Re: [PATCH 00/15] Protect the block layer with a rwlock: part 3 |
Date: |
Fri, 18 Nov 2022 16:13:06 +0100 |
On Fri, Nov 18, 2022 at 4:01 PM Emanuele Giuseppe Esposito
<eesposit@redhat.com> wrote:
> > - generated_co_wrapper_simple -> coroutine_wrapper
> > - generated_co_wrapper_blk -> coroutine_wrapper_mixed
> > - generated_co_wrapper -> coroutine_wrapper_mixed_bdrv
> >
> > ? It is not clear to me yet if you will have bdrv_* functions that take
> > the rdlock as well - in which case however coroutine_wrapper_bdrv would
> > not be hard to add.
>
> Why _mixed? I thought _blk was a nice name to identify only the blk_*
> API that have this slightly different behavior (ie do not take the
> rwlock anywhere).
_mixed means that they are callable from both coroutine and
non-coroutine function, but (unlike "normal" functions) they will
suspend if called in coroutine context.
In fact we could also have a coroutine_mixed_fn static analysis
annotation for functions that *call* coroutine_wrapper_mixed (so
ultimately they can suspend when called from coroutine context, e.g.
vhdx_log_write_and_flush or qcow2's update_refcount):
- coroutine_fn can call coroutine_mixed_fn if needed, but cannot call
any coroutine_wrapper*
- coroutine_mixed_fn can call coroutine_mixed_fn or
coroutine_wrapper_mixed{,_bdrv}, but cannot call coroutine_wrapper
This of course is completely independent of your series, but since the
naming is adjacent it would be nice to only change it once.
> No, I don't think there will be bdrv_* functions that will behave as
> blk_*, if that's what you mean.
>
> Regarding the bdrv_* functions in general, there are a couple of
> additional places (which should be all in the main loop) where we need
> to use assert_bdrv_grapg_readable. In those places we theoretically need
> nothing, but if we use the automatic TSA locking checker that is being
> tested by kevin, we need some sort of "placeholder" so that cc/gcc (I
> don't remember anymore which) won't throw false positives.
clang. :) That can be sorted out with review, but in general I think
asserting is always okay.
Paolo
- [PATCH 13/15] block: convert bdrv_io_plug in generated_co_wrapper_simple, (continued)
- [PATCH 13/15] block: convert bdrv_io_plug in generated_co_wrapper_simple, Emanuele Giuseppe Esposito, 2022/11/16
- [PATCH 04/15] block: convert bdrv_refresh_total_sectors in generated_co_wrapper, Emanuele Giuseppe Esposito, 2022/11/16
- [PATCH 05/15] block: use bdrv_co_refresh_total_sectors when possible, Emanuele Giuseppe Esposito, 2022/11/16
- [PATCH 08/15] block: convert bdrv_is_inserted in generated_co_wrapper_simple, Emanuele Giuseppe Esposito, 2022/11/16
- [PATCH 09/15] block-coroutine-wrapper: support void functions, Emanuele Giuseppe Esposito, 2022/11/16
- [PATCH 12/15] block: convert bdrv_debug_event in generated_co_wrapper, Emanuele Giuseppe Esposito, 2022/11/16
- [PATCH 14/15] block: convert bdrv_io_unplug in generated_co_wrapper_simple, Emanuele Giuseppe Esposito, 2022/11/16
- [PATCH 15/15] block: rename newly converted BlockDriver IO coroutine functions, Emanuele Giuseppe Esposito, 2022/11/16
- Re: [PATCH 00/15] Protect the block layer with a rwlock: part 3, Paolo Bonzini, 2022/11/18
Re: [PATCH 00/15] Protect the block layer with a rwlock: part 3, Emanuele Giuseppe Esposito, 2022/11/21