Re: [PATCH v2 2/2] monitor: increase amount of data for monitor to read
On 24.11.2020 14:03, Vladimir Sementsov-Ogievskiy wrote:
23.11.2020 18:44, Andrey Shinkevich wrote:
QMP and HMP monitors read one byte at a time from the socket or stdin,
which is very inefficient. With 100+ VMs on the host, this results in
multiple extra system calls and CPU overuse.
This patch increases the amount of read data up to 4096 bytes that fits
the buffer size on the channel level.
A JSON little parser is introduced to throttle QMP commands read from
the buffer so that incoming requests do not overflow the monitor input
queue.
Suggested-by: Denis V. Lunev
Signed-off-by: Andrey Shinkevich
Can't we just increase qmp queue instead? It seems a lot simpler:
With the OOB compatibility disabled, the monitor queues one QMP command
at most. It was made for the backward compatibility as stated in the
comment before pushing a command into the queue. To keep that concept
functional, the monitor should track the end of a single QMP command. It
allows the dispatcher handling the command and send a response to client
in time.
With the patch below, the monitor queue will be filled with QMP commands
as many as they will be found in the input buffer. The first command
execution {"execute":"qmp_capabilities"} takes more time and queue will
be filled at full. Then the dispatcher starts execution of other
commands in the monitor queue. The process becomes synchronious. In this
case, we need neither thread nor the queue.
Andrey
diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 348bfad3d5..7e721eee3f 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -8,7 +8,7 @@
typedef struct MonitorHMP MonitorHMP;
typedef struct MonitorOptions MonitorOptions;
-#define QMP_REQ_QUEUE_LEN_MAX 8
+#define QMP_REQ_QUEUE_LEN_MAX 4096
extern QemuOptsList qemu_mon_opts;
diff --git a/monitor/monitor.c b/monitor/monitor.c
index 84222cd130..1588f00306 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -566,7 +566,7 @@ int monitor_can_read(void *opaque)
{
Monitor *mon = opaque;
- return !qatomic_mb_read(>suspend_cnt);
+ return !qatomic_mb_read(>suspend_cnt) ? 4096 : 0;
}
- with this patch tests pass and performance is even better.
Re: [PATCH v2 2/2] monitor: increase amount of data for monitor to read
24.11.2020 14:03, Vladimir Sementsov-Ogievskiy wrote:
23.11.2020 18:44, Andrey Shinkevich wrote:
QMP and HMP monitors read one byte at a time from the socket or stdin,
which is very inefficient. With 100+ VMs on the host, this results in
multiple extra system calls and CPU overuse.
This patch increases the amount of read data up to 4096 bytes that fits
the buffer size on the channel level.
A JSON little parser is introduced to throttle QMP commands read from
the buffer so that incoming requests do not overflow the monitor input
queue.
Suggested-by: Denis V. Lunev
Signed-off-by: Andrey Shinkevich
Can't we just increase qmp queue instead? It seems a lot simpler:
diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 348bfad3d5..7e721eee3f 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -8,7 +8,7 @@
typedef struct MonitorHMP MonitorHMP;
typedef struct MonitorOptions MonitorOptions;
-#define QMP_REQ_QUEUE_LEN_MAX 8
+#define QMP_REQ_QUEUE_LEN_MAX 4096
extern QemuOptsList qemu_mon_opts;
diff --git a/monitor/monitor.c b/monitor/monitor.c
index 84222cd130..1588f00306 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -566,7 +566,7 @@ int monitor_can_read(void *opaque)
{
Monitor *mon = opaque;
- return !qatomic_mb_read(>suspend_cnt);
+ return !qatomic_mb_read(>suspend_cnt) ? 4096 : 0;
}
- with this patch tests pass and performance is even better.
Suddenly I found, that this patch ^^ was sent a year ago:
https://patchew.org/QEMU/20190610105906.28524-1-dplotni...@virtuozzo.com/
some questions were asked, so I think we should start from it.
--
Best regards,
Vladimir
Re: [PATCH v2 2/2] monitor: increase amount of data for monitor to read
23.11.2020 18:44, Andrey Shinkevich wrote:
QMP and HMP monitors read one byte at a time from the socket or stdin,
which is very inefficient. With 100+ VMs on the host, this results in
multiple extra system calls and CPU overuse.
This patch increases the amount of read data up to 4096 bytes that fits
the buffer size on the channel level.
A JSON little parser is introduced to throttle QMP commands read from
the buffer so that incoming requests do not overflow the monitor input
queue.
Suggested-by: Denis V. Lunev
Signed-off-by: Andrey Shinkevich
Can't we just increase qmp queue instead? It seems a lot simpler:
diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 348bfad3d5..7e721eee3f 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -8,7 +8,7 @@
typedef struct MonitorHMP MonitorHMP;
typedef struct MonitorOptions MonitorOptions;
-#define QMP_REQ_QUEUE_LEN_MAX 8
+#define QMP_REQ_QUEUE_LEN_MAX 4096
extern QemuOptsList qemu_mon_opts;
diff --git a/monitor/monitor.c b/monitor/monitor.c
index 84222cd130..1588f00306 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -566,7 +566,7 @@ int monitor_can_read(void *opaque)
{
Monitor *mon = opaque;
-return !qatomic_mb_read(>suspend_cnt);
+return !qatomic_mb_read(>suspend_cnt) ? 4096 : 0;
}
- with this patch tests pass and performance is even better.
--
Best regards,
Vladimir
[PATCH v2 2/2] monitor: increase amount of data for monitor to read
QMP and HMP monitors read one byte at a time from the socket or stdin,
which is very inefficient. With 100+ VMs on the host, this results in
multiple extra system calls and CPU overuse.
This patch increases the amount of read data up to 4096 bytes that fits
the buffer size on the channel level.
A JSON little parser is introduced to throttle QMP commands read from
the buffer so that incoming requests do not overflow the monitor input
queue.
Suggested-by: Denis V. Lunev
Signed-off-by: Andrey Shinkevich
---
chardev/char-fd.c | 35 +--
chardev/char-socket.c | 42 +++---
chardev/char.c | 41 +
include/chardev/char.h | 15 +++
monitor/monitor.c | 2 +-
5 files changed, 129 insertions(+), 6 deletions(-)
diff --git a/chardev/char-fd.c b/chardev/char-fd.c
index 1cd62f2..15bc8f4 100644
--- a/chardev/char-fd.c
+++ b/chardev/char-fd.c
@@ -33,6 +33,8 @@
#include "chardev/char-fd.h"
#include "chardev/char-io.h"
+#include "monitor/monitor-internal.h"
+
/* Called with chr_write_lock held. */
static int fd_chr_write(Chardev *chr, const uint8_t *buf, int len)
{
@@ -45,8 +47,12 @@ static gboolean fd_chr_read(QIOChannel *chan, GIOCondition
cond, void *opaque)
{
Chardev *chr = CHARDEV(opaque);
FDChardev *s = FD_CHARDEV(opaque);
+CharBackend *be = chr->be;
+Monitor *mon = (Monitor *)be->opaque;
int len;
uint8_t buf[CHR_READ_BUF_LEN];
+uint8_t *cursor;
+int load, size, pos;
ssize_t ret;
len = sizeof(buf);
@@ -62,10 +68,35 @@ static gboolean fd_chr_read(QIOChannel *chan, GIOCondition
cond, void *opaque)
if (ret == 0) {
remove_fd_in_watch(chr);
qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
+chr->json_thl = (const JSONthrottle){0};
return FALSE;
}
-if (ret > 0) {
-qemu_chr_be_write(chr, buf, ret);
+if (ret < 0) {
+return TRUE;
+}
+load = ret;
+cursor = buf;
+
+while (load > 0) {
+size = load;
+if (monitor_is_qmp(mon)) {
+/* Find the end position of a JSON command in the input buffer */
+pos = qemu_chr_end_position((const char *) cursor, size,
+>json_thl);
+if (pos >= 0) {
+size = pos + 1;
+}
+}
+
+qemu_chr_be_write(chr, cursor, size);
+cursor += size;
+load -= size;
+
+if (load > 0) {
+while (qatomic_mb_read(>suspend_cnt)) {
+g_usleep(40);
+}
+}
}
return TRUE;
diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 213a4c8..30ad1d4 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -38,6 +38,8 @@
#include "chardev/char-io.h"
#include "qom/object.h"
+#include "monitor/monitor-internal.h"
+
/***/
/* TCP Net console */
@@ -522,7 +524,11 @@ static gboolean tcp_chr_read(QIOChannel *chan,
GIOCondition cond, void *opaque)
{
Chardev *chr = CHARDEV(opaque);
SocketChardev *s = SOCKET_CHARDEV(opaque);
+CharBackend *be = chr->be;
+Monitor *mon = (Monitor *)be->opaque;
uint8_t buf[CHR_READ_BUF_LEN];
+uint8_t *cursor;
+int load, pos;
int len, size;
if ((s->state != TCP_CHARDEV_STATE_CONNECTED) ||
@@ -537,12 +543,42 @@ static gboolean tcp_chr_read(QIOChannel *chan,
GIOCondition cond, void *opaque)
if (size == 0 || (size == -1 && errno != EAGAIN)) {
/* connection closed */
tcp_chr_disconnect(chr);
-} else if (size > 0) {
+chr->json_thl = (const JSONthrottle){0};
+return TRUE;
+}
+if (size < 0) {
+return TRUE;
+}
+load = size;
+cursor = buf;
+
+while (load > 0) {
+size = load;
+if (monitor_is_qmp(mon)) {
+/* Find the end position of a JSON command in the input buffer */
+pos = qemu_chr_end_position((const char *) cursor, size,
+>json_thl);
+if (pos >= 0) {
+size = pos + 1;
+}
+}
+len = size;
+
if (s->do_telnetopt) {
-tcp_chr_process_IAC_bytes(chr, s, buf, );
+tcp_chr_process_IAC_bytes(chr, s, cursor, );
}
if (size > 0) {
-qemu_chr_be_write(chr, buf, size);
+qemu_chr_be_write(chr, cursor, size);
+cursor += size;
+load -= size;
+} else {
+cursor += len;
+load -= len;
+}
+if (load > 0) {
+while (qatomic_mb_read(>suspend_cnt)) {
+g_usleep(40);
+}
}
}
diff --git a/chardev/char.c b/chardev/char.c
index aa42821..75c7bc7 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -1178,6 +1178,47 @@ GSource
