Re: Fio regression caused by f9fc8932b11f3bcf2a2626f567cb6fdd36a33a94

[Longpeng (Mike, Cloud Infrastructure Service Product Dept.)]

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


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