[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v6 06/12] monitor: Make current monitor a per-coroutine prope

From: Daniel P . Berrangé
Subject: Re: [PATCH v6 06/12] monitor: Make current monitor a per-coroutine property
Date: Tue, 4 Aug 2020 17:14:52 +0100
User-agent: Mutt/1.14.5 (2020-06-23)

On Tue, Aug 04, 2020 at 03:50:54PM +0200, Markus Armbruster wrote:
> Kevin Wolf <kwolf@redhat.com> writes:
> > This way, a monitor command handler will still be able to access the
> > current monitor, but when it yields, all other code code will correctly
> > get NULL from monitor_cur().
> >
> > Outside of coroutine context, qemu_coroutine_self() returns the leader
> > coroutine of the current thread.
> Unsaid: you use it as a hash table key to map from coroutine to monitor,
> and for that you need it to return a value unique to the coroutine in
> coroutine context, and a value unique to the thread outside coroutine
> context.  Which qemu_coroutine_self() does.  Correct?
> The hash table works, but I hate it just as much as I hate
> pthread_getspecific() / pthread_setspecific().
> What we have here is a need for coroutine-local data.  Feels like a
> perfectly natural concept to me.
> Are we going to create another hash table whenever we need another piece
> of coroutine-local data?  Or shall we reuse the hash table, suitably
> renamed and moved to another file?
> Why not simply associate an opaque pointer with each coroutine?  All it
> takes is one more member of struct Coroutine.  Whatever creates the
> coroutine decides what to use it for.  The monitor coroutine would use
> it to point to the monitor.

Possible benefit of having the coroutine-local data stored in the
coroutine stack is that we can probably make it lock-less. Using
the hash table in monitor.c results in a serialization of across
all coroutines & threads.

Also, by providing a GDestroyNotify against the coroutine-local
data we can easily guarantee cleanup with the coroutine is freed.

Since we'll have a limited number of data items, we could make do
with a simple array in the coroutine struct, instead of a hashtable.

  enum CoroutineLocalKeys {


  struct Coroutine {
    gpointer localData[CO_LOCAL_LAST];
    GDestroyNotify localDataFree[CO_LOCAL_LAST];

|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

reply via email to

[Prev in Thread] Current Thread [Next in Thread]