Re: Fio regression caused by f9fc8932b11f3bcf2a2626f567cb6fdd36a33a94

[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.

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áš

signature.asc
Description: PGP signature