Re: [Qemu-devel] [PATCH 02/35] WIP: coroutine: manually tag the fast-paths

["Marc-André Lureau" <marcandre.lureau@redhat.com>]

Hi

----- Original Message -----
> On Wed, Jul 05, 2017 at 12:03:13AM +0200, Marc-André Lureau wrote:
> > Some functions are both regular and coroutine. They may call coroutine
> > functions in some cases, if it is known to be running in a coroutine.
> > 
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  block.c                 |  2 ++
> >  block/block-backend.c   |  2 ++
> >  block/io.c              | 16 +++++++++++++++-
> >  block/sheepdog.c        |  2 ++
> >  block/throttle-groups.c | 10 ++++++++--
> >  migration/rdma.c        |  2 ++
> >  6 files changed, 31 insertions(+), 3 deletions(-)
> > 
> > diff --git a/block.c b/block.c
> > index 694396281b..b08c006da4 100644
> > --- a/block.c
> > +++ b/block.c
> > @@ -443,7 +443,9 @@ int bdrv_create(BlockDriver *drv, const char* filename,
> >  
> >      if (qemu_in_coroutine()) {
> >          /* Fast-path if already in coroutine context */
> > +        co_role_acquire(_coroutine_fn);
> >          bdrv_create_co_entry(&cco);
> > +        co_role_release(_coroutine_fn);
> >      } else {
> >          co = qemu_coroutine_create(bdrv_create_co_entry, &cco);
> >          qemu_coroutine_enter(co);
> 
> I guess the clever analysis for clang would be to detect that if
> (qemu_in_coroutine()) means we have the _coroutine_fn role.  It's
> similar to how Coverity sees an if (ptr) and knows whether the pointer
> is NULL/non-NULL in the branches.
> 
> But this patch is okay too :-).

Right, I though about using try_acquire_capability, similarly needed for try_lock etc. However, I don't see how to automatically release the capability when going out of scope. Apparently there are some known limitations around these patterns. I would love to hear from compilers folks what they think about -Wthread-safety and if it can be added to gcc with various kind of improvements.