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

[Andrea Cervesato]

Hi,

On 10/13/23 09:05, Wang, Xin wrote:
> Hello,
>> Hi,
>>
>> On 10/11/23 10:47, Cyril Hrubis wrote:
>>> 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?
>> My 2 cents: I would go the other way around, since I'm busy with different
>> things and it won't be available that soon.
> Thank you, I could wait for fsx-linux rewrite then send a new patch. Do you have a schedule for fsx-linux rewrite?
I can't give a schedule at the moment, because I'm busy with other 
things. It might take a few months, considering review process as well.
>>> A few comments 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.
>>>
>>>> +    {
>>>>               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
>> The whole CLOCK_MONOTONIC_RAW problem is mostly related with test
>> logging messages, which is something we will get rid in the fsx-linux rewrite.
>>
>> Regards,
>> Andrea Cervesato
>>
>>
>> --
>> Mailing list info: https://lists.linux.it/listinfo/ltp

Regards,
Andrea Cervesato