|
From: | Longpeng (Mike, Cloud Infrastructure Service Product Dept.) |
Subject: | Re: Fio regression caused by f9fc8932b11f3bcf2a2626f567cb6fdd36a33a94 |
Date: | Thu, 5 May 2022 20:34:40 +0800 |
User-agent: | Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.7.0 |
Hi Stefan, 在 2022/5/5 18:09, Stefan Hajnoczi 写道:
On Tue, May 03, 2022 at 09:43:15AM +0200, Lukáš Doktor wrote:Hello Mike, Paolo, others, in my perf pipeline I noticed a regression bisected to the f9fc8932b11f3bcf2a2626f567cb6fdd36a33a94 - "thread-posix: remove the posix semaphore support" commit and I'd like to ask you to verify it might have caused that and eventually consider fixing it. The regression is visible, reproducible and clearly bisectable to this commit with the following 2 scenarios:I can't parse the commit message for f9fc8932b11f3bcf2a2626f567cb6fdd36a33a94, so it's not 100% clear to me why it was necessary to remove sem_*() calls.
We can find the previous discussion here: [1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg870174.html [2] https://www.mail-archive.com/qemu-devel@nongnu.org/msg870409.htmlBecause sem_timedwait() only supports absolute time and it would be affected
if the system time is changing. Another reason to remove sem_*() is to make the code much neater.
util/thread-pool.c uses qemu_sem_*() to notify worker threads when work becomes available. It makes sense that this operation is performance-critical and that's why the benchmark regressed. Maybe thread-pool.c can use qemu_cond_*() instead of qemu_sem_*(). That avoids the extra mutex (we already have pool->lock) and counter (we already have pool->request_list)?1. fio write 4KiB using the nbd ioengine on localhost 2. fio read 4KiB using #cpu jobs and iodepth=8 on a rotational disk using qcow2 image and default virt-install <disk type="file" device="disk"> <driver name="qemu" type="qcow2"/> <source file="/var/lib/libvirt/images/RHEL-8.4.0-20210503.1-virtlab506.DefaultLibvirt0.qcow2"/> <target dev="vda" bus="virtio"/> </disk> but smaller regressions can be seen under other scenarios as well since this commit. You can find the report from bisections here: https://ldoktor.github.io/tmp/RedHat-virtlab506/v7.0.0/RedHat-virtlab506-f9fc8932b11f3bcf2a2626f567cb6fdd36a33a94-RHEL-8.4.0-20210503.1-1.html https://ldoktor.github.io/tmp/RedHat-virtlab506/v7.0.0/RedHat-virtlab506-f9fc8932b11f3bcf2a2626f567cb6fdd36a33a94-RHEL-8.4.0-20210503.1-2.html Regards, Lukáš
[Prev in Thread] | Current Thread | [Next in Thread] |