qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Peter Xu <peterx@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Paolo Bonzini" <pbonzini@redhat.com>,
	"Daniel P . Berrange" <berrange@redhat.com>,
	"Juan Quintela" <quintela@redhat.com>,
	peterx@redhat.com, "Markus Armbruster" <armbru@redhat.com>,
	"Marc-André Lureau" <marcandre.lureau@redhat.com>,
	"Stefan Hajnoczi" <stefanha@redhat.com>,
	"Dr . David Alan Gilbert" <dgilbert@redhat.com>
Subject: [Qemu-devel] [PATCH 13/14] qio: allow threaded qiotask to switch contexts
Date: Wed, 28 Feb 2018 13:06:32 +0800	[thread overview]
Message-ID: <20180228050633.7410-14-peterx@redhat.com> (raw)
In-Reply-To: <20180228050633.7410-1-peterx@redhat.com>

This is the part of work to allow the QIOTask to use a different
gcontext rather than the default main gcontext, by providing
qio_task_context_set() API.

We have done some work before on doing similar things to add non-default
gcontext support.  The general idea is that we delete the old GSource
from the main context, then re-add a new one to the new context when
context changed to a non-default one.  However this trick won't work
easily for threaded QIOTasks since we can't easily stop a real thread
and re-setup the whole thing from the very beginning.

But luckily, we don't need to do anything to the thread.  We just need
to keep an eye on the GSource that completes the QIOTask, which is
assigned to gcontext after the sync operation finished.

So when we setup a non-default GMainContext for a threaded QIO task, we
may face two cases:

- the thread is still running the sync task: then we don't need to do
  anything, only to update QIOTask.context to the new context

- the thread has finished the sync task and queued an idle task to main
  thread: then we destroy that old idle task, and re-create it on the
  new GMainContext.

Note that along the way when we modify either idle GSource or the
context, we need to take the mutex before hand, since the thread may be
modifying them at the same time.

Finally, call qio_task_context_set() in the tcp chardev update read
handler hook if QIOTask is running.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 chardev/char-socket.c |  4 +++
 include/io/task.h     |  1 +
 io/task.c             | 70 ++++++++++++++++++++++++++++++++++++++++++---------
 3 files changed, 63 insertions(+), 12 deletions(-)

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 9d51b8da07..164a64ff34 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -585,6 +585,10 @@ static void tcp_chr_update_read_handler(Chardev *chr)
         tcp_chr_telnet_init(CHARDEV(s));
     }
 
+    if (s->thread_task) {
+        qio_task_context_set(s->thread_task, chr->gcontext);
+    }
+
     if (!s->connected) {
         return;
     }
diff --git a/include/io/task.h b/include/io/task.h
index c6acd6489c..87e0152d8a 100644
--- a/include/io/task.h
+++ b/include/io/task.h
@@ -324,5 +324,6 @@ Object *qio_task_get_source(QIOTask *task);
 
 void qio_task_ref(QIOTask *task);
 void qio_task_unref(QIOTask *task);
+void qio_task_context_set(QIOTask *task, GMainContext *context);
 
 #endif /* QIO_TASK_H */
diff --git a/io/task.c b/io/task.c
index 080f9560ea..59bc439bdf 100644
--- a/io/task.c
+++ b/io/task.c
@@ -42,6 +42,9 @@ struct QIOTask {
     uint32_t refcount;
 
     /* Threaded QIO task specific fields */
+    bool has_thread;
+    QemuThread thread;
+    QemuMutex mutex;       /* Protects threaded QIO task fields */
     GSource *idle_source;  /* The idle task to run complete routine */
     GMainContext *context; /* The context that idle task will run with */
     QIOTaskThreadData thread_data;
@@ -57,6 +60,8 @@ QIOTask *qio_task_new(Object *source,
 
     task = g_new0(QIOTask, 1);
 
+    qemu_mutex_init(&task->mutex);
+
     task->source = source;
     object_ref(source);
     task->func = func;
@@ -88,7 +93,16 @@ static void qio_task_free(QIOTask *task)
     if (task->context) {
         g_main_context_unref(task->context);
     }
+    /*
+     * Make sure the thread quitted before we destroy the mutex,
+     * otherwise the thread might still be using it.
+     */
+    if (task->has_thread) {
+        qemu_thread_join(&task->thread);
+    }
+
     object_unref(task->source);
+    qemu_mutex_destroy(&task->mutex);
 
     g_free(task);
 }
@@ -117,12 +131,28 @@ static gboolean qio_task_thread_result(gpointer opaque)
     return FALSE;
 }
 
+/* Must be with QIOTask.mutex held. */
+static void qio_task_thread_create_complete_job(QIOTask *task)
+{
+    GSource *idle;
+
+    /* Remove the old if there is */
+    if (task->idle_source) {
+        g_source_destroy(task->idle_source);
+        g_source_unref(task->idle_source);
+    }
+
+    idle = g_idle_source_new();
+    g_source_set_callback(idle, qio_task_thread_result, task, NULL);
+    g_source_attach(idle, task->context);
+
+    task->idle_source = idle;
+}
 
 static gpointer qio_task_thread_worker(gpointer opaque)
 {
     QIOTask *task = opaque;
     QIOTaskThreadData *data = &task->thread_data;
-    GSource *idle;
 
     trace_qio_task_thread_run(task);
     data->worker(task, data->opaque);
@@ -134,10 +164,9 @@ static gpointer qio_task_thread_worker(gpointer opaque)
      */
     trace_qio_task_thread_exit(task);
 
-    idle = g_idle_source_new();
-    g_source_set_callback(idle, qio_task_thread_result, data, NULL);
-    g_source_attach(idle, task->context);
-    task->idle_source = idle;
+    qemu_mutex_lock(&task->mutex);
+    qio_task_thread_create_complete_job(task);
+    qemu_mutex_unlock(&task->mutex);
 
     return NULL;
 }
@@ -149,24 +178,21 @@ void qio_task_run_in_thread(QIOTask *task,
                             GDestroyNotify destroy,
                             GMainContext *context)
 {
-    QemuThread thread;
     QIOTaskThreadData *data = &task->thread_data;
 
-    if (context) {
-        g_main_context_ref(context);
-        task->context = context;
-    }
+    qio_task_context_set(task, context);
 
     data->worker = worker;
     data->opaque = opaque;
     data->destroy = destroy;
 
     trace_qio_task_thread_start(task, worker, opaque);
-    qemu_thread_create(&thread,
+    qemu_thread_create(&task->thread,
                        "io-task-worker",
                        qio_task_thread_worker,
                        task,
-                       QEMU_THREAD_DETACHED);
+                       QEMU_THREAD_JOINABLE);
+    task->has_thread = true;
 }
 
 
@@ -235,3 +261,23 @@ void qio_task_unref(QIOTask *task)
         qio_task_free(task);
     }
 }
+
+void qio_task_context_set(QIOTask *task, GMainContext *context)
+{
+    qemu_mutex_lock(&task->mutex);
+    if (task->context) {
+        g_main_context_unref(task->context);
+    }
+    if (context) {
+        g_main_context_ref(task->context);
+        task->context = context;
+    }
+    if (task->idle_source) {
+        /*
+         * We have had an idle job on the old context. Firstly delete
+         * the old one, then create a new one on the new context.
+         */
+        qio_task_thread_create_complete_job(task);
+    }
+    qemu_mutex_unlock(&task->mutex);
+}
-- 
2.14.3

  parent reply	other threads:[~2018-02-28  5:07 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-28  5:06 [Qemu-devel] [PATCH 00/14] qio: general non-default GMainContext support Peter Xu
2018-02-28  5:06 ` [Qemu-devel] [PATCH 01/14] chardev: fix leak in tcp_chr_telnet_init_io() Peter Xu
2018-02-28  9:26   ` Daniel P. Berrangé
2018-02-28  5:06 ` [Qemu-devel] [PATCH 02/14] qio: rename qio_task_thread_result Peter Xu
2018-02-28  9:26   ` Daniel P. Berrangé
2018-02-28  5:06 ` [Qemu-devel] [PATCH 03/14] qio: introduce qio_channel_add_watch_full() Peter Xu
2018-02-28  9:08   ` Daniel P. Berrangé
2018-02-28 12:44     ` Peter Xu
2018-02-28 12:47       ` Daniel P. Berrangé
2018-02-28 13:01         ` Peter Xu
2018-02-28  5:06 ` [Qemu-devel] [PATCH 04/14] migration: let incoming side use thread context Peter Xu
2018-02-28  9:10   ` Daniel P. Berrangé
2018-03-01  4:33     ` Peter Xu
2018-02-28 17:43   ` Dr. David Alan Gilbert
2018-03-01  2:53     ` Peter Xu
2018-03-01  9:58       ` Dr. David Alan Gilbert
2018-02-28  5:06 ` [Qemu-devel] [PATCH 05/14] qio: refactor net listener source operations Peter Xu
2018-02-28  5:06 ` [Qemu-devel] [PATCH 06/14] qio: store gsources for net listeners Peter Xu
2018-02-28  5:06 ` [Qemu-devel] [PATCH 07/14] qio/chardev: update net listener gcontext Peter Xu
2018-02-28  9:25   ` Daniel P. Berrangé
2018-02-28 12:52     ` Peter Xu
2018-02-28 13:06       ` Daniel P. Berrangé
2018-02-28  5:06 ` [Qemu-devel] [PATCH 08/14] chardev: allow telnet gsource to switch gcontext Peter Xu
2018-02-28  5:06 ` [Qemu-devel] [PATCH 09/14] qio: basic non-default context support for thread Peter Xu
2018-02-28  5:06 ` [Qemu-devel] [PATCH 10/14] qio: refcount QIOTask Peter Xu
2018-02-28  9:16   ` Daniel P. Berrangé
2018-02-28 12:54     ` Peter Xu
2018-02-28 13:07       ` Daniel P. Berrangé
2018-02-28 13:15         ` Peter Xu
2018-02-28  5:06 ` [Qemu-devel] [PATCH 11/14] qio/chardev: return QIOTask when connect async Peter Xu
2018-02-28  9:20   ` Daniel P. Berrangé
2018-02-28 13:07     ` Peter Xu
2018-02-28  5:06 ` [Qemu-devel] [PATCH 12/14] qio: move QIOTaskThreadData into QIOTask Peter Xu
2018-02-28  5:06 ` Peter Xu [this message]
2018-02-28  9:23   ` [Qemu-devel] [PATCH 13/14] qio: allow threaded qiotask to switch contexts Daniel P. Berrangé
2018-02-28 13:05     ` Peter Xu
2018-02-28 13:20       ` Daniel P. Berrangé
2018-03-01  8:49         ` Peter Xu
2018-02-28  5:06 ` [Qemu-devel] [PATCH 14/14] qio/chardev: specify gcontext for TLS handshake Peter Xu
2018-02-28 13:22   ` Daniel P. Berrangé
2018-03-01  6:28     ` Peter Xu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180228050633.7410-14-peterx@redhat.com \
    --to=peterx@redhat.com \
    --cc=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@redhat.com \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).