On Mon, Sep 13, 2021 at 04:35:24PM +0100, Stefan Hajnoczi wrote: > It was reported that enabling SafeStack reduces IOPS significantly > (>25%) with the following fio benchmark on virtio-blk using a NVMe host > block device: > > # fio --rw=randrw --bs=4k --iodepth=64 --runtime=1m --direct=1 \ > --filename=/dev/vdb --name=job1 --ioengine=libaio --thread \ > --group_reporting --numjobs=16 --time_based \ > --output=/tmp/fio_result > > Serge Guelton and I found that SafeStack is not really at fault, it just > increases the cost of coroutine creation. This fio workload exhausts the > coroutine pool and coroutine creation becomes a bottleneck. Previous > work by Honghao Wang also pointed to excessive coroutine creation. > > Creating new coroutines is expensive due to allocating new stacks with > mmap(2) and mprotect(2). Currently there are thread-local and global > pools that recycle old Coroutine objects and their stacks but the > hardcoded size limit of 64 for thread-local pools and 128 for the global > pool is insufficient for the fio benchmark shown above. > > This patch changes the coroutine pool algorithm to a simple thread-local > pool without a maximum size limit. Threads periodically shrink the pool > down to a size sufficient for the maximum observed number of coroutines. > > The global pool is removed by this patch. It can help to hide the fact > that local pools are easily exhausted, but it's doesn't fix the root > cause. I don't think there is a need for a global pool because QEMU's > threads are long-lived, so let's keep things simple. > > Performance of the above fio benchmark is as follows: > > Before After > IOPS 60k 97k > > Memory usage varies over time as needed by the workload: > > VSZ (KB) RSS (KB) > Before fio 4705248 843128 > During fio 5747668 (+ ~100 MB) 849280 > After fio 4694996 (- ~100 MB) 845184 > > This confirms that coroutines are indeed being freed when no longer > needed. > > Thanks to Serge Guelton for working on identifying the bottleneck with > me! > > Reported-by: Tingting Mao <ti...@redhat.com> > Cc: Serge Guelton <sguel...@redhat.com> > Cc: Honghao Wang <wanghong...@bytedance.com> > Cc: Paolo Bonzini <pbonz...@redhat.com> > Cc: Daniele Buono <dbu...@linux.vnet.ibm.com> > Signed-off-by: Stefan Hajnoczi <stefa...@redhat.com> > --- > v2: > * Retained minimum pool size of 64 to keep latency low for threads that > perform I/O infrequently and to prevent possible regressions [Daniele] > --- > include/qemu/coroutine-pool-timer.h | 36 ++++++++++++++++ > include/qemu/coroutine.h | 7 +++ > iothread.c | 6 +++ > util/coroutine-pool-timer.c | 35 +++++++++++++++ > util/main-loop.c | 5 +++ > util/qemu-coroutine.c | 66 ++++++++++++++++------------- > util/meson.build | 1 + > 7 files changed, 126 insertions(+), 30 deletions(-) > create mode 100644 include/qemu/coroutine-pool-timer.h > create mode 100644 util/coroutine-pool-timer.c
Applied to my block-next tree: https://gitlab.com/stefanha/qemu/commits/block-next Stefan
signature.asc
Description: PGP signature