Re: [PATCH] crypto: fix bogus error benchmarking pbkdf on fast machines

[Daniel P . Berrangé]

On Wed, Jan 22, 2025 at 10:19:41PM +0300, Michael Tokarev wrote:
> 08.01.2025 21:43, Daniel P. Berrangé пишет:
> > We're seeing periodic reports of errors like:
> > 
> > $ qemu-img create -f luks --object secret,data=123456,id=sec0 \
> >                    -o key-secret=sec0 luks-info.img 1M
> >    Formatting 'luks-info.img', fmt=luks size=1048576 key-secret=sec0
> >    qemu-img: luks-info.img: Unable to get accurate CPU usage
> > 
> > This error message comes from a recent attempt to workaround a
> > kernel bug with measuring rusage in long running processes:
> > 
> >    commit c72cab5ad9f849bbcfcf4be7952b8b8946cc626e
> >    Author: Tiago Pasqualini <tiago.pasqualini@canonical.com>
> >    Date:   Wed Sep 4 20:52:30 2024 -0300
> > 
> >      crypto: run qcrypto_pbkdf2_count_iters in a new thread
> > 
> > Unfortunately this has a subtle bug on machines which are very fast.
> > 
> > On the first time around the loop, the 'iterations' value is quite
> > small (1 << 15), and so will run quite fast. Testing has shown that
> > some machines can complete this benchmarking task in as little as
> > 7 milliseconds.
> > 
> > Unfortunately the 'getrusage' data is not updated at the time of
> > the 'getrusage' call, it is done asynchronously by the schedular.
> > The 7 millisecond completion time for the benchmark is short
> > enough that 'getrusage' sometimes reports 0 accumulated execution
> > time.
> > 
> > As a result the 'delay_ms == 0' sanity check in the above commit
> > is triggering non-deterministically on such machines.
> > 
> > The benchmarking loop intended to run multiple times, increasing
> > the 'iterations' value until the benchmark ran for > 500 ms, but
> > the sanity check doesn't allow this to happen.
> > 
> > To fix it, we keep a loop counter and only run the sanity check
> > after we've been around the loop more than 5 times. At that point
> > the 'iterations' value is high enough that even with infrequent
> > updates of 'getrusage' accounting data on fast machines, we should
> > see a non-zero value.
> > 
> > Reported-by: Thomas Huth <thuth@redhat.com>
> > Reported-by: Richard W.M. Jones <rjones@redhat.com>
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> 
> Is this a qemu-stable material (9.2)?

Yes, please include it.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|