On Mon, Jul 19, 2010 at 1:32 AM, Caspar Zhang <[email protected]> wrote:
>
> ----- "Garrett Cooper" <[email protected]> wrote:
>
>> On Sun, Jul 18, 2010 at 11:55 PM, Caspar Zhang <[email protected]>
>> wrote:
>> >
>> > ----- "Garrett Cooper" <[email protected]> wrote:
>> >
>> >>
>> >>     The fact that the test is using time(3) to calculate the
>> elapsed
>> >> period instead of clock_gettime is dubious at best, because the
>> >> precision of time is in seconds. Maybe it should be converted to
>> >> clock_gettime calls for improved precision?
>> >
>> > Use clock_gettime instead of time(3), patch attached.
>>
>> Looks better, but being strict it's still buggy. POSIX defines struct
>> timespec as:
>>
>> The <time.h> header shall declare the structure timespec, which has
>> at
>> least the following members:
>>
>> time_t  tv_sec    Seconds.
>> long    tv_nsec   Nanoseconds.
>>
>> So you're losing precision in one case, and time_t can be variable
>> (the spec for types.h is rather fluid):
>>
>> time_t and clock_t shall be integer or real-floating types.
>>
>> The (struct timeval *) casts are also unnecessary because you're
>> getting the address of a struct timeval object (which is in both
>> cases
>> struct timeval*).
>>
>> > Another question, since POSIX.1-2008 marks gettimeofday() as
>> obsolete
>> > (man gettimeofday), should the usec test part be changed to
>> > clock_getime as well?
>>
>> Sure, if you want.
>
> Updated patch, FYI.
>
> Thanks,
> Caspar
>
>
>
> Hi all,
>
>   1. In my recent tests, not all desired time be equal to exact execution
>   time in sec test part, use clock_gettime to improve precision.
>   2. gettimeofday is marked as obsolete in POSIX.1-2008 according to
>   man gettimeofday, so change gettimeofday to clock_gettime in nsec test
>   part.
>
>   Signed-off-by: Caspar Zhang <[email protected]>
>
> diff -Naur a/testcases/kernel/syscalls/pselect/Makefile 
> b/testcases/kernel/syscalls/pselect/Makefile
> --- a/testcases/kernel/syscalls/pselect/Makefile        2010-04-01 
> 14:23:10.000000000 +0800
> +++ b/testcases/kernel/syscalls/pselect/Makefile        2010-07-19 
> 14:54:38.984482305 +0800
> @@ -21,6 +21,7 @@
>  include $(top_srcdir)/include/mk/testcases.mk
>  include $(abs_srcdir)/../utils/newer_64.mk
>
> +LDLIBS                 += -lpthread -lrt
>  %_64: CPPFLAGS += -D_FILE_OFFSET_BITS=64
>
>  include $(top_srcdir)/include/mk/generic_leaf_target.mk
> diff -Naur a/testcases/kernel/syscalls/pselect/pselect01.c 
> b/testcases/kernel/syscalls/pselect/pselect01.c
> --- a/testcases/kernel/syscalls/pselect/pselect01.c     2010-04-01 
> 14:23:10.000000000 +0800
> +++ b/testcases/kernel/syscalls/pselect/pselect01.c     2010-07-19 
> 16:08:32.402482397 +0800
> @@ -61,14 +61,12 @@
>
>  int main()
>  {
> -       int ret_pselect, total_sec, fd, total_nsec;
> +       int ret_pselect, fd;
>        fd_set readfds;
> -       struct timespec tv;
>        int retval;
> -       time_t t;
> -       unsigned start, end;
> -       struct timeval tv_start, tv_end;
> -       int real_usec;
> +       struct timespec tv, tv_start, tv_end;
> +       long real_nsec, total_nsec;
> +       int real_sec, total_sec;
>
>        setup();
>
> @@ -103,17 +101,20 @@
>                tst_resm(TINFO,
>                         "Testing basic pselect sanity,Sleeping for %jd secs",
>                         (intmax_t)tv.tv_sec);
> -               start = time(&t);
> +               clock_gettime(CLOCK_REALTIME, &tv_start);
>                retval =
> -                   pselect(0, &readfds, NULL, NULL, (struct timespec *)&tv,
> +                   pselect(0, &readfds, NULL, NULL, &tv,
>                            NULL);
> -               end = time(&t);
> +               clock_gettime(CLOCK_REALTIME, &tv_end);
>
> -               if (((end - start) == total_sec) || ((end - start) == 
> total_sec + 1))
> +               /* do a rounding */
> +               real_sec = (int)(0.5 + (tv_end.tv_sec - tv_start.tv_sec +
> +                               1e-9 * (tv_end.tv_nsec - tv_start.tv_nsec)));
> +               if (real_sec == total_sec)
>                        tst_resm(TPASS, "Sleep time was correct");
>                else
>                        tst_resm(TFAIL, "Sleep time was incorrect:%d != %d",
> -                                total_sec, (end - start));
> +                                total_sec, real_sec);
>        }
>
>  #ifdef DEBUG
> @@ -129,19 +130,19 @@
>                tst_resm(TINFO,
>                         "Testing basic pselect sanity,Sleeping for %ld nano 
> secs",
>                         tv.tv_nsec);
> -               gettimeofday(&tv_start, NULL);
> +               clock_gettime(CLOCK_REALTIME, &tv_start);
>                retval =
>                    pselect(0, &readfds, NULL, NULL, &tv,
>                            NULL);
> -               gettimeofday(&tv_end, NULL);
> +               clock_gettime(CLOCK_REALTIME, &tv_end);
>
>                /* Changed total_sec compare to an at least vs an exact 
> compare */
>
> -               real_usec = (tv_end.tv_sec - tv_start.tv_sec) * 1e6 +
> -                       tv_end.tv_usec - tv_start.tv_usec;
> +               real_nsec = (tv_end.tv_sec - tv_start.tv_sec) * 1e9 +
> +                       tv_end.tv_nsec - tv_start.tv_nsec;
>
>                /* allow 10% error*/
> -               if (abs(real_usec - tv.tv_nsec / 1000) < 0.1 * total_nsec / 
> 1000)
> +               if (abs(real_nsec - tv.tv_nsec) < 0.1 * total_nsec)
>                        tst_resm(TPASS, "Sleep time was correct");
>                else {
>                        tst_resm(TWARN,
> @@ -150,8 +151,8 @@
>                                 "due to the limitation of the way it 
> calculates the");
>                        tst_resm(TWARN, "system call execution time.");
>                        tst_resm(TFAIL,
> -                               "Sleep time was incorrect:%d usec vs expected 
> %d usec",
> -                                real_usec, total_nsec / 1000);
> +                               "Sleep time was incorrect:%ld nsec vs 
> expected %ld nsec",
> +                                real_nsec, total_nsec);
>                }
>        }
>        cleanup();

ACK (and thanks for the hard work)!
-Garrett

------------------------------------------------------------------------------
This SF.net email is sponsored by Sprint
What will you do first with EVO, the first 4G phone?
Visit sprint.com/first -- http://p.sf.net/sfu/sprint-com-first
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

Reply via email to