All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 1/1] util/thread-pool: add parameter to limit maximum threads in thread pool
@ 2018-07-13  7:48 FelixYao
  2018-07-27 10:18 ` Stefan Hajnoczi
  0 siblings, 1 reply; 4+ messages in thread
From: FelixYao @ 2018-07-13  7:48 UTC (permalink / raw)
  To: kwolf, mreitz, armbru; +Cc: yaozhenguo, qemu-devel, FelixYao

Hi all:

Max threads in thread pool is fixed at 64 before which is not
propriate in some situations. For public cloud environment,
there are lots of VMs in one host machine. We should limit the worker thread
numbers for each machine alone based on its ability to prevent impacting
each other. Although we can use iotune to limit bandwidth or IOPS of block
device. That is not enough. Lots of worker threads still impact other VMs.
I think the ability to limit maximum threads in thread pool is necessary.

What's your opinion about this patch? please review it.

Signed-off-by: FelixYao <felix.yzg@gmail.com>
---
 include/block/thread-pool.h |  1 +
 qemu-options.hx             | 11 +++++++++++
 util/thread-pool.c          |  3 ++-
 vl.c                        | 31 +++++++++++++++++++++++++++++++
 4 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/include/block/thread-pool.h b/include/block/thread-pool.h
index 7dd7d73..2634339 100644
--- a/include/block/thread-pool.h
+++ b/include/block/thread-pool.h
@@ -23,6 +23,7 @@
 typedef int ThreadPoolFunc(void *opaque);
 
 typedef struct ThreadPool ThreadPool;
+extern int max_threads;
 
 ThreadPool *thread_pool_new(struct AioContext *ctx);
 void thread_pool_free(ThreadPool *pool);
diff --git a/qemu-options.hx b/qemu-options.hx
index 16208f6..e7337f6 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3948,6 +3948,17 @@ STEXI
 prepend a timestamp to each log message.(default:on)
 ETEXI
 
+DEF("thread-pool", HAS_ARG, QEMU_OPTION_threadpool,
+    "-thread-pool max-threads=[n]\n"
+    "                limit maximum threads in thread pool \n"
+    "                when block device using aio=threads parameter\n",
+    QEMU_ARCH_ALL)
+STEXI
+@item -thread-pool max-threads=[n]
+@findex -thread-pool
+limit maximum threads in thread pool.(default:64)
+ETEXI
+
 DEF("dump-vmstate", HAS_ARG, QEMU_OPTION_dump_vmstate,
     "-dump-vmstate <file>\n"
     "                Output vmstate information in JSON format to file.\n"
diff --git a/util/thread-pool.c b/util/thread-pool.c
index 610646d..debad3c 100644
--- a/util/thread-pool.c
+++ b/util/thread-pool.c
@@ -26,6 +26,7 @@
 static void do_spawn_thread(ThreadPool *pool);
 
 typedef struct ThreadPoolElement ThreadPoolElement;
+int max_threads = 64;
 
 enum ThreadState {
     THREAD_QUEUED,
@@ -308,7 +309,7 @@ static void thread_pool_init_one(ThreadPool *pool, AioContext *ctx)
     qemu_mutex_init(&pool->lock);
     qemu_cond_init(&pool->worker_stopped);
     qemu_sem_init(&pool->sem, 0);
-    pool->max_threads = 64;
+    pool->max_threads = max_threads;
     pool->new_thread_bh = aio_bh_new(ctx, spawn_thread_bh_fn, pool);
 
     QLIST_INIT(&pool->head);
diff --git a/vl.c b/vl.c
index 16b913f..3f40424 100644
--- a/vl.c
+++ b/vl.c
@@ -82,6 +82,7 @@ int main(int argc, char **argv)
 #include "qemu/log.h"
 #include "sysemu/blockdev.h"
 #include "hw/block/block.h"
+#include "block/thread-pool.h"
 #include "migration/misc.h"
 #include "migration/snapshot.h"
 #include "migration/global_state.h"
@@ -540,6 +541,22 @@ static QemuOptsList qemu_fw_cfg_opts = {
     },
 };
 
+static QemuOptsList qemu_thread_pool_opts = {
+    .name = "thread-pool",
+    .implied_opt_name = "max-threads",
+    .head = QTAILQ_HEAD_INITIALIZER(qemu_thread_pool_opts.head),
+    .desc = {
+        {
+            .name = "max-threads",
+            .type = QEMU_OPT_NUMBER,
+            .help = "limit maximum threads in thread pool \n"
+		    "when block device using aio=threads parameter",
+            .def_value_str = "64",
+        },
+        { /* end of list */ }
+    },
+};
+
 /**
  * Get machine options
  *
@@ -2993,6 +3010,8 @@ int main(int argc, char **argv, char **envp)
     qemu_add_opts(&qemu_icount_opts);
     qemu_add_opts(&qemu_semihosting_config_opts);
     qemu_add_opts(&qemu_fw_cfg_opts);
+    qemu_add_opts(&qemu_thread_pool_opts);
+
     module_call_init(MODULE_INIT_OPTS);
 
     runstate_init();
@@ -3947,6 +3966,18 @@ int main(int argc, char **argv, char **envp)
                 }
                 configure_msg(opts);
                 break;
+            case QEMU_OPTION_threadpool:
+                opts = qemu_opts_parse_noisily(qemu_find_opts("thread-pool"),
+                                               optarg, false);
+                if (!opts) {
+                    exit(1);
+                }
+                max_threads = qemu_opt_get_number(opts, "max-threads", 0);
+                if (max_threads <= 0) {
+                    error_report("max-threads is invalid");
+                    exit(1);
+                }
+                break;
             case QEMU_OPTION_dump_vmstate:
                 if (vmstate_dump_file) {
                     error_report("only one '-dump-vmstate' "
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH 1/1] util/thread-pool: add parameter to limit maximum threads in thread pool
  2018-07-13  7:48 [Qemu-devel] [PATCH 1/1] util/thread-pool: add parameter to limit maximum threads in thread pool FelixYao
@ 2018-07-27 10:18 ` Stefan Hajnoczi
  2018-07-27 12:49   ` Daniel P. Berrangé
  2018-07-30  8:25   ` felix yao
  0 siblings, 2 replies; 4+ messages in thread
From: Stefan Hajnoczi @ 2018-07-27 10:18 UTC (permalink / raw)
  To: FelixYao; +Cc: kwolf, mreitz, armbru, yaozhenguo, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 1325 bytes --]

On Fri, Jul 13, 2018 at 03:48:27PM +0800, FelixYao wrote:
> Hi all:
> 
> Max threads in thread pool is fixed at 64 before which is not
> propriate in some situations. For public cloud environment,
> there are lots of VMs in one host machine. We should limit the worker thread
> numbers for each machine alone based on its ability to prevent impacting
> each other. Although we can use iotune to limit bandwidth or IOPS of block
> device. That is not enough. Lots of worker threads still impact other VMs.
> I think the ability to limit maximum threads in thread pool is necessary.
> 
> What's your opinion about this patch? please review it.

I haven't seen any discussion so here are some thoughts:

It sounds like you are trying to control I/O resources so that guests do
not affect each other.  The threads are an implementation detail and not
the root of the problem.

For example, if you use -drive aio=native then reads and writes will not
use the thread pool and you will setting the thread pool size won't
solve the problem.

Have you tried QEMU's I/O throttling feature?  You can set bandwidth
(bps) and IOPS limits, including bursts if you wish to allow temporary
spikes.  The basic parameters are -drive bps=BYTES_PER_SECOND,iops=IOPS.
See the QEMU or libvirt documentation for details.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH 1/1] util/thread-pool: add parameter to limit maximum threads in thread pool
  2018-07-27 10:18 ` Stefan Hajnoczi
@ 2018-07-27 12:49   ` Daniel P. Berrangé
  2018-07-30  8:25   ` felix yao
  1 sibling, 0 replies; 4+ messages in thread
From: Daniel P. Berrangé @ 2018-07-27 12:49 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: FelixYao, kwolf, yaozhenguo, qemu-devel, armbru, mreitz

On Fri, Jul 27, 2018 at 11:18:26AM +0100, Stefan Hajnoczi wrote:
> On Fri, Jul 13, 2018 at 03:48:27PM +0800, FelixYao wrote:
> > Hi all:
> > 
> > Max threads in thread pool is fixed at 64 before which is not
> > propriate in some situations. For public cloud environment,
> > there are lots of VMs in one host machine. We should limit the worker thread
> > numbers for each machine alone based on its ability to prevent impacting
> > each other. Although we can use iotune to limit bandwidth or IOPS of block
> > device. That is not enough. Lots of worker threads still impact other VMs.
> > I think the ability to limit maximum threads in thread pool is necessary.
> > 
> > What's your opinion about this patch? please review it.
> 
> I haven't seen any discussion so here are some thoughts:
> 
> It sounds like you are trying to control I/O resources so that guests do
> not affect each other.  The threads are an implementation detail and not
> the root of the problem.
> 
> For example, if you use -drive aio=native then reads and writes will not
> use the thread pool and you will setting the thread pool size won't
> solve the problem.
> 
> Have you tried QEMU's I/O throttling feature?  You can set bandwidth
> (bps) and IOPS limits, including bursts if you wish to allow temporary
> spikes.  The basic parameters are -drive bps=BYTES_PER_SECOND,iops=IOPS.
> See the QEMU or libvirt documentation for details.

Correct me if I'm wrong, but would using the iothreads feature already
enable us to put a cap on the number of threads used for I/o operations,
but assigning dedicated threads to each device ?

Regards,
Daniel
-- 
|: 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 :|

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH 1/1] util/thread-pool: add parameter to limit maximum threads in thread pool
  2018-07-27 10:18 ` Stefan Hajnoczi
  2018-07-27 12:49   ` Daniel P. Berrangé
@ 2018-07-30  8:25   ` felix yao
  1 sibling, 0 replies; 4+ messages in thread
From: felix yao @ 2018-07-30  8:25 UTC (permalink / raw)
  To: stefanha; +Cc: kwolf, mreitz, armbru, yaozhenguo, QEMU Developers

Hi stefan:
    Thank you very much for your reply. Yes, I want to reduce impact of
resources among virtual machines including I/O resources, vCPU and etc. I
can use CPU pin feature to pin vCPU to dedicated host's physical CPU and
use I/O throttling feature to limit IOPS and bandwidth of VM's disks. But,
When using aio=threads, I/O worker threads are too many(if we have 16 VMs
in one host, there are 1024 threads at worst) and  can't pinned to
dedicated CPU(dynamic creating). The I/O worker threads seem out of
control. I/O worker threads of one VM's disk may impact the running of
other VM. I am not sure how much of the impact.
    You have mentioned aio=native in the mail. Yes, When Using -drive
aio=native, there is no such problem and performance is better. I am
thinking to use it. But, I found a issue report about using Linux AIO on
QEMU, issue link:
https://specs.openstack.org/openstack/nova-specs/specs/mitaka/implemented/libvirt-aio-mode.html
. http://paste.openstack.org/show/480498/. Do you know about this issue?
Could Linux AIO be safely used in QEMU?
    I have  noticed that there is IOthread feature in QEMU(dedicated thread
to handle disk I/O instead of main thread) recently. I can use 'iothreadpin
iothread=xx cpuset=xxx ' in libvirt XML to pin iothreads of one VM to
dedicated host's physical CPU. Worker threads of the VM are also pinned to
that host's physical CPU. This can also solve my problem.




Stefan Hajnoczi <stefanha@gmail.com> 于2018年7月27日周五 下午8:41写道:

> On Fri, Jul 13, 2018 at 03:48:27PM +0800, FelixYao wrote:
> > Hi all:
> >
> > Max threads in thread pool is fixed at 64 before which is not
> > propriate in some situations. For public cloud environment,
> > there are lots of VMs in one host machine. We should limit the worker
> thread
> > numbers for each machine alone based on its ability to prevent impacting
> > each other. Although we can use iotune to limit bandwidth or IOPS of
> block
> > device. That is not enough. Lots of worker threads still impact other
> VMs.
> > I think the ability to limit maximum threads in thread pool is necessary.
> >
> > What's your opinion about this patch? please review it.
>
> I haven't seen any discussion so here are some thoughts:
>
> It sounds like you are trying to control I/O resources so that guests do
> not affect each other.  The threads are an implementation detail and not
> the root of the problem.
>
> For example, if you use -drive aio=native then reads and writes will not
> use the thread pool and you will setting the thread pool size won't
> solve the problem.
>
> Have you tried QEMU's I/O throttling feature?  You can set bandwidth
> (bps) and IOPS limits, including bursts if you wish to allow temporary
> spikes.  The basic parameters are -drive bps=BYTES_PER_SECOND,iops=IOPS.
> See the QEMU or libvirt documentation for details.
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-07-30  8:26 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-13  7:48 [Qemu-devel] [PATCH 1/1] util/thread-pool: add parameter to limit maximum threads in thread pool FelixYao
2018-07-27 10:18 ` Stefan Hajnoczi
2018-07-27 12:49   ` Daniel P. Berrangé
2018-07-30  8:25   ` felix yao

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.