| Message ID | 20211224023755.532189-1-lei.rao@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | net/filter: Optimize filter_send to coroutine | expand |
On 24/12/2021 10:37, Rao, Lei wrote: > This patch is to improve the logic of QEMU main thread sleep code in > qemu_chr_write_buffer() where it can be blocked and can't run other > coroutines during COLO IO stress test. > > Our approach is to put filter_send() in a coroutine. In this way, > filter_send() will call qemu_coroutine_yield() in qemu_co_sleep_ns(), > so that it can be scheduled out and QEMU main thread has opportunity to > run other tasks. > > Signed-off-by: Lei Rao <lei.rao@intel.com> > Signed-off-by: Zhang Chen <chen.zhang@intel.com> > --- > net/filter-mirror.c | 67 ++++++++++++++++++++++++++++++++++++--------- > 1 file changed, 54 insertions(+), 13 deletions(-) > > diff --git a/net/filter-mirror.c b/net/filter-mirror.c > index f20240cc9f..1e9f8b6216 100644 > --- a/net/filter-mirror.c > +++ b/net/filter-mirror.c > @@ -20,6 +20,7 @@ > #include "chardev/char-fe.h" > #include "qemu/iov.h" > #include "qemu/sockets.h" > +#include "block/aio-wait.h" > > #define TYPE_FILTER_MIRROR "filter-mirror" > typedef struct MirrorState MirrorState; > @@ -42,20 +43,21 @@ struct MirrorState { > bool vnet_hdr; > }; > > -static int filter_send(MirrorState *s, > - const struct iovec *iov, > - int iovcnt) > +typedef struct FilterSendCo { > + MirrorState *s; > + char *buf; > + ssize_t size; > + bool done; > + int ret; > +} FilterSendCo; > + > +static int _filter_send(MirrorState *s, > + char *buf, > + ssize_t size) > { > NetFilterState *nf = NETFILTER(s); > int ret = 0; > - ssize_t size = 0; > uint32_t len = 0; > - char *buf; > - > - size = iov_size(iov, iovcnt); > - if (!size) { > - return 0; > - } > > len = htonl(size); > ret = qemu_chr_fe_write_all(&s->chr_out, (uint8_t *)&len, sizeof(len)); > @@ -80,10 +82,7 @@ static int filter_send(MirrorState *s, > } > } > > - buf = g_malloc(size); > - iov_to_buf(iov, iovcnt, 0, buf, size); > ret = qemu_chr_fe_write_all(&s->chr_out, (uint8_t *)buf, size); > - g_free(buf); > if (ret != size) { > goto err; > } > @@ -94,6 +93,48 @@ err: > return ret < 0 ? ret : -EIO; > } > > +static void coroutine_fn filter_send_co(void *opaque) > +{ > + FilterSendCo *data = opaque; > + > + data->ret = _filter_send(data->s, data->buf, data->size); > + data->done = true; > + g_free(data->buf); > + aio_wait_kick(); > +} > + > +static int filter_send(MirrorState *s, > + const struct iovec *iov, > + int iovcnt) > +{ > + ssize_t size = iov_size(iov, iovcnt); > + char *buf = NULL; > + > + if (!size) { > + return 0; > + } > + > + buf = g_malloc(size); > + iov_to_buf(iov, iovcnt, 0, buf, size); > + > + FilterSendCo data = { > + .s = s, > + .size = size, > + .buf = buf, > + .ret = 0, > + }; > + > + Coroutine *co = qemu_coroutine_create(filter_send_co, &data); BTW, does qemu/old gcc complaint such coding style ? int a; a = foo() int b = a; > + qemu_coroutine_enter(co); > + > + while (!data.done) { > + aio_poll(qemu_get_aio_context(), true); > + } > + > + return data.ret; > + redundant newline Otherwise, Reviewed-by: Li Zhijian <lizhijian@fujitsu.com> > +} > + > static void redirector_to_filter(NetFilterState *nf, > const uint8_t *buf, > int len)
On 12/24/2021 6:07 PM, lizhijian@fujitsu.com wrote: > > > On 24/12/2021 10:37, Rao, Lei wrote: >> This patch is to improve the logic of QEMU main thread sleep code in >> qemu_chr_write_buffer() where it can be blocked and can't run other >> coroutines during COLO IO stress test. >> >> Our approach is to put filter_send() in a coroutine. In this way, >> filter_send() will call qemu_coroutine_yield() in qemu_co_sleep_ns(), >> so that it can be scheduled out and QEMU main thread has opportunity to >> run other tasks. >> >> Signed-off-by: Lei Rao <lei.rao@intel.com> >> Signed-off-by: Zhang Chen <chen.zhang@intel.com> >> --- >> net/filter-mirror.c | 67 ++++++++++++++++++++++++++++++++++++--------- >> 1 file changed, 54 insertions(+), 13 deletions(-) >> >> diff --git a/net/filter-mirror.c b/net/filter-mirror.c >> index f20240cc9f..1e9f8b6216 100644 >> --- a/net/filter-mirror.c >> +++ b/net/filter-mirror.c >> @@ -20,6 +20,7 @@ >> #include "chardev/char-fe.h" >> #include "qemu/iov.h" >> #include "qemu/sockets.h" >> +#include "block/aio-wait.h" >> >> #define TYPE_FILTER_MIRROR "filter-mirror" >> typedef struct MirrorState MirrorState; >> @@ -42,20 +43,21 @@ struct MirrorState { >> bool vnet_hdr; >> }; >> >> -static int filter_send(MirrorState *s, >> - const struct iovec *iov, >> - int iovcnt) >> +typedef struct FilterSendCo { >> + MirrorState *s; >> + char *buf; >> + ssize_t size; >> + bool done; >> + int ret; >> +} FilterSendCo; >> + >> +static int _filter_send(MirrorState *s, >> + char *buf, >> + ssize_t size) >> { >> NetFilterState *nf = NETFILTER(s); >> int ret = 0; >> - ssize_t size = 0; >> uint32_t len = 0; >> - char *buf; >> - >> - size = iov_size(iov, iovcnt); >> - if (!size) { >> - return 0; >> - } >> >> len = htonl(size); >> ret = qemu_chr_fe_write_all(&s->chr_out, (uint8_t *)&len, sizeof(len)); >> @@ -80,10 +82,7 @@ static int filter_send(MirrorState *s, >> } >> } >> >> - buf = g_malloc(size); >> - iov_to_buf(iov, iovcnt, 0, buf, size); >> ret = qemu_chr_fe_write_all(&s->chr_out, (uint8_t *)buf, size); >> - g_free(buf); >> if (ret != size) { >> goto err; >> } >> @@ -94,6 +93,48 @@ err: >> return ret < 0 ? ret : -EIO; >> } >> >> +static void coroutine_fn filter_send_co(void *opaque) >> +{ >> + FilterSendCo *data = opaque; >> + >> + data->ret = _filter_send(data->s, data->buf, data->size); >> + data->done = true; >> + g_free(data->buf); >> + aio_wait_kick(); >> +} >> + >> +static int filter_send(MirrorState *s, >> + const struct iovec *iov, >> + int iovcnt) >> +{ >> + ssize_t size = iov_size(iov, iovcnt); >> + char *buf = NULL; >> + >> + if (!size) { >> + return 0; >> + } >> + >> + buf = g_malloc(size); >> + iov_to_buf(iov, iovcnt, 0, buf, size); >> + >> + FilterSendCo data = { >> + .s = s, >> + .size = size, >> + .buf = buf, >> + .ret = 0, >> + }; >> + >> + Coroutine *co = qemu_coroutine_create(filter_send_co, &data); > > BTW, does qemu/old gcc complaint such coding style ? > > int a; > a = foo() > int b = a; There are a lot of codes of this style in QEMU. It is written that we need at least GCC v7.4 to compile QEMU in the configure file. So, I think it is no problem. > > > >> + qemu_coroutine_enter(co); >> + >> + while (!data.done) { >> + aio_poll(qemu_get_aio_context(), true); >> + } >> + >> + return data.ret; >> + > redundant newline will be changed in V2. Thanks, Lei > > Otherwise, > Reviewed-by: Li Zhijian <lizhijian@fujitsu.com> > > > >> +} >> + >> static void redirector_to_filter(NetFilterState *nf, >> const uint8_t *buf, >> int len)
diff --git a/net/filter-mirror.c b/net/filter-mirror.c index f20240cc9f..1e9f8b6216 100644 --- a/net/filter-mirror.c +++ b/net/filter-mirror.c @@ -20,6 +20,7 @@ #include "chardev/char-fe.h" #include "qemu/iov.h" #include "qemu/sockets.h" +#include "block/aio-wait.h" #define TYPE_FILTER_MIRROR "filter-mirror" typedef struct MirrorState MirrorState; @@ -42,20 +43,21 @@ struct MirrorState { bool vnet_hdr; }; -static int filter_send(MirrorState *s, - const struct iovec *iov, - int iovcnt) +typedef struct FilterSendCo { + MirrorState *s; + char *buf; + ssize_t size; + bool done; + int ret; +} FilterSendCo; + +static int _filter_send(MirrorState *s, + char *buf, + ssize_t size) { NetFilterState *nf = NETFILTER(s); int ret = 0; - ssize_t size = 0; uint32_t len = 0; - char *buf; - - size = iov_size(iov, iovcnt); - if (!size) { - return 0; - } len = htonl(size); ret = qemu_chr_fe_write_all(&s->chr_out, (uint8_t *)&len, sizeof(len)); @@ -80,10 +82,7 @@ static int filter_send(MirrorState *s, } } - buf = g_malloc(size); - iov_to_buf(iov, iovcnt, 0, buf, size); ret = qemu_chr_fe_write_all(&s->chr_out, (uint8_t *)buf, size); - g_free(buf); if (ret != size) { goto err; } @@ -94,6 +93,48 @@ err: return ret < 0 ? ret : -EIO; } +static void coroutine_fn filter_send_co(void *opaque) +{ + FilterSendCo *data = opaque; + + data->ret = _filter_send(data->s, data->buf, data->size); + data->done = true; + g_free(data->buf); + aio_wait_kick(); +} + +static int filter_send(MirrorState *s, + const struct iovec *iov, + int iovcnt) +{ + ssize_t size = iov_size(iov, iovcnt); + char *buf = NULL; + + if (!size) { + return 0; + } + + buf = g_malloc(size); + iov_to_buf(iov, iovcnt, 0, buf, size); + + FilterSendCo data = { + .s = s, + .size = size, + .buf = buf, + .ret = 0, + }; + + Coroutine *co = qemu_coroutine_create(filter_send_co, &data); + qemu_coroutine_enter(co); + + while (!data.done) { + aio_poll(qemu_get_aio_context(), true); + } + + return data.ret; + +} + static void redirector_to_filter(NetFilterState *nf, const uint8_t *buf, int len)