Re: [PATCH v2 6/8] Revert use of clock_gettime for benchmarking

["Daniel P. Berrangé" <berrange@redhat.com>]

On Mon, Oct 21, 2024 at 01:42:29PM +0200, Kevin Wolf wrote:
> Am 19.10.2024 um 21:35 hat Philippe Mathieu-Daudé geschrieben:
> > Hi,
> > 
> > On 18/10/24 10:20, Aleksandar Rakic wrote:
> > > This patch reverts the commit (with SHA
> > > 50290c002c045280f8defad911901e16bfb52884 from
> > > https://github.com/MIPS/gnutools-qemu) that breaks for mingw builds,
> > > where clock_gettime and CLOCK_MONOTONIC are not available.
> 
> What does "not available" mean? I'm sure that we have kept building QEMU
> with mingw in the past five years (the commit you want to revert is from
> 2019), so they must exist in some form?

They exist in the mingw headers

$ grep clock_gettime /usr/i686-w64-mingw32/sys-root/mingw/include/pthread_time.h
int __cdecl WINPTHREAD_API clock_gettime(clockid_t clock_id, struct timespec *tp);

$ grep CLOCK_MONOTONIC /usr/i686-w64-mingw32/sys-root/mingw/include/pthread_time.h
#ifndef CLOCK_MONOTONIC
#define CLOCK_MONOTONIC             1

So I think we need more clarity about what's broken before we can
consider merging this.

> 
> > Isn't get_clock() what we want here?
> > 
> > > Cherry-picked d57c735e1af1ca719dbd0c3a904ad70c9c31cbb7
> > > from https://github.com/MIPS/gnutools-qemu
> > > 
> > > Signed-off-by: Faraz Shahbazker <fshahbazker@wavecomp.com>
> > > Signed-off-by: Aleksandar Rakic <aleksandar.rakic@htecgroup.com>
> > > ---
> > >   qemu-io-cmds.c | 77 +++++++++++++++++++++++++-------------------------
> > >   1 file changed, 39 insertions(+), 38 deletions(-)
> > 
> > Please Cc maintainers (done now):
> > 
> > $ ./scripts/get_maintainer.pl -f qemu-io-cmds.c
> > Kevin Wolf <kwolf@redhat.com> (supporter:Block layer core)
> > Hanna Reitz <hreitz@redhat.com> (supporter:Block layer core)
> 
> Also Alex (CCed) who is the author of the patch you want to revert.
> 
> Kevin
> 
> > > @@ -904,7 +905,7 @@ static const cmdinfo_t readv_cmd = {
> > >   static int readv_f(BlockBackend *blk, int argc, char **argv)
> > >   {
> > > -    struct timespec t1, t2;
> > > +    struct timeval t1, t2;
> > >       bool Cflag = false, qflag = false, vflag = false;
> > >       int c, cnt, ret;
> > >       char *buf;
> > > @@ -964,9 +965,9 @@ static int readv_f(BlockBackend *blk, int argc, char **argv)
> > >           return -EINVAL;
> > >       }
> > > -    clock_gettime(CLOCK_MONOTONIC, &t1);
> > > +    gettimeofday(&t1, NULL);
> > >       ret = do_aio_readv(blk, &qiov, offset, flags, &total);
> > > -    clock_gettime(CLOCK_MONOTONIC, &t2);
> > > +    gettimeofday(&t2, NULL);
> > >       if (ret < 0) {
> > >           printf("readv failed: %s\n", strerror(-ret));
> > 
> 
> 

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 :|