[GEDI] [PATCH 00/15] Protect the block layer with a rwlock: part 3

Paolo Bonzini pbonzini at redhat.com
Fri Nov 18 15:13:06 UTC 2022


On Fri, Nov 18, 2022 at 4:01 PM Emanuele Giuseppe Esposito
<eesposit at 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



More information about the integration mailing list