[LTP] [PATCH] enable fsx-linux.c running on VxWorks user space and add measurement of execution time

[Wang, Xin]

> Hi!
> > I come from Wind River. We and our customers used fsx-linux for file
> > system testing on VxWorks user space and total execution time also
> > received more attention in the testing.  So I upstream the change of
> > VxWorks compatibility parts on this patch, wish to enlarge the test
> > support of fsx-linux.  There is a point need to explain, because
> > current VxWorks doesn't support CLOCK_MONOTONIC_RAW ,
> CLOCK_MONOTONIC
> > is still used by VxWorks when calling clock_gettime.  When
> > CLOCK_MONOTONIC_RAW is support by VxWorks in the future, this part
> > will be removed.
> 
> Actually Andrea started to clean up and rewrite the test so that it uses LTP API
> and he will send a patch soon, can you please send a new patch once that
> rewrite is finished?
> 
> A few comments below.

Thanks for your review. One question abort msync part in below.
> 
> > Signed-off-by: Xin.Wang@windriver.com
> > ---
> >  testcases/kernel/fs/fsx-linux/fsx-linux.c | 44
> > ++++++++++++++++++++---
> >  1 file changed, 40 insertions(+), 4 deletions(-)
> >
> > diff --git a/testcases/kernel/fs/fsx-linux/fsx-linux.c
> > b/testcases/kernel/fs/fsx-linux/fsx-linux.c
> > index 64c27a0f5..2064fac16 100644
> > --- a/testcases/kernel/fs/fsx-linux/fsx-linux.c
> > +++ b/testcases/kernel/fs/fsx-linux/fsx-linux.c
> > @@ -39,8 +39,10 @@
> >
> >  #include <sys/types.h>
> >  #include <sys/stat.h>
> > -#if defined(_UWIN) || defined(__linux__)
> > +#if defined(_UWIN) || defined(__linux__) || defined(__VXWORKS__) #if
> > +!defined(__VXWORKS__)
> >  #include <sys/param.h>
> > +#endif
> >  #include <limits.h>
> >  #include <time.h>
> >  #include <string.h>
> > @@ -849,7 +851,12 @@ void domapwrite(unsigned offset, unsigned size)
> >               gettimeofday(&t, NULL);
> >               prt("       %lu.%06lu memcpy done\n", t.tv_sec, t.tv_usec);
> >       }
> > -     if (msync(p, map_size, 0) != 0) {
> > +#ifdef MS_SYNC
> > +     if (msync(p, map_size, MS_SYNC) != 0) #else
> > +     if (msync(p, map_size, 0) != 0)
> > +#endif
> 
> It would be better to define MS_SYNC to 0 on VXWORKS instead.

Because only MS_ASYNC, MS_SYNC and MS_INVALIDATE are defined for flag argument of msync in POSIX. Is it right to set 0 directly in the case? 
Flag set 0 will be identified EINVAL in VxWorks.

> 
> > +     {
> >               prterr("domapwrite: msync");
> >               report_failure(203);
> >       }
> > @@ -1115,11 +1122,16 @@ int main(int argc, char **argv)
> >       int i, style, ch;
> >       char *endp;
> >       int dirpath = 0;
> > -
> > +     struct timespec time_start, time_end, time_diff;
> > +
> >       goodfile[0] = 0;
> >       logfile[0] = 0;
> >
> > +#if defined(__VXWORKS__)
> > +     page_size = (int)sysconf(_SC_PAGESIZE);
> 
> We can just use sysconf() everywhere instead.
> 
> > +#else
> >       page_size = getpagesize();
> > +#endif
> >       page_mask = page_size - 1;
> >
> >       setvbuf(stdout, NULL, _IOLBF, 0);       /* line buffered stdout */
> > @@ -1267,9 +1279,12 @@ int main(int argc, char **argv)
> >       signal(SIGUSR1, cleanup);
> >       signal(SIGUSR2, cleanup);
> >
> > +#if defined(__VXWORKS__)
> > +     srand(seed);
> > +#else
> >       initstate(seed, state, 256);
> >       setstate(state);
> > -
> > +#endif
> >       open_test_files(argv, argc);
> >
> >       strncat(goodfile, dirpath ? basename(fname) : fname, 256); @@
> > -1336,12 +1351,33 @@ int main(int argc, char **argv)
> >       } else
> >               check_trunc_hack();
> >
> > +#if defined(__VXWORKS__)
> > +     clock_gettime(CLOCK_MONOTONIC, &time_start); #else
> > +     clock_gettime(CLOCK_MONOTONIC_RAW, &time_start); #endif
> 
> And here it would be cleaner to define the clock to use instead of ifdefing each
> clock_gettime call. I.e.:
> 
> #if defined(__VXWORKS__)
> # define CLOCK_ID CLOCK_MONOTONIC
> #else
> # define CLOCK_ID CLOCK_MONOTONIC_RAW
> #endif
> 
> >       while (numops == -1 || numops--)
> >               test();
> >
> >       close_test_files();
> > +#if defined(__VXWORKS__)
> > +     clock_gettime(CLOCK_MONOTONIC, &time_end); #else
> > +     clock_gettime(CLOCK_MONOTONIC_RAW, &time_end); #endif
> > +
> >       prt("All operations completed A-OK!\n");
> >
> > +     if (time_end.tv_nsec < time_start.tv_nsec) {
> > +             time_end.tv_nsec += 1000000000;
> > +             time_end.tv_sec -= 1;
> > +     }
> > +     time_diff.tv_sec = time_end.tv_sec - time_start.tv_sec;
> > +     time_diff.tv_nsec = time_end.tv_nsec - time_start.tv_nsec;
> 
> LTP has functions for operations with timespec, here you should use
> tst_timespec_diff()
> 
> > +     prt("Elapsed Test Time %lu.%09lu\n",
> > +        (unsigned long)time_diff.tv_sec, time_diff.tv_nsec);
> > +
> >       if (tf_buf)
> >               free(tf_buf);
> >
> > --
> > 2.34.1
> >
> >
> > --
> > Mailing list info: https://lists.linux.it/listinfo/ltp
> 
> --
> Cyril Hrubis
> chrubis@suse.cz