Message ID | 20201016132245.73378-1-tommi.t.rantala@nokia.com |
---|---|
State | Superseded |
Headers | show |
Series | selftests: intel_pstate: ftime() is deprecated | expand |
Hi Tommi, On 10/16/20 7:22 AM, Tommi Rantala wrote: > Use clock_gettime() instead of deprecated ftime(). > > aperf.c: In function ‘main’: > aperf.c:58:2: warning: ‘ftime’ is deprecated [-Wdeprecated-declarations] > 58 | ftime(&before); > | ^~~~~ > In file included from aperf.c:9: > /usr/include/sys/timeb.h:39:12: note: declared here > 39 | extern int ftime (struct timeb *__timebuf) > | ^~~~~ > Thanks for the fix. One comment below > Signed-off-by: Tommi Rantala <tommi.t.rantala@nokia.com> > --- > tools/testing/selftests/intel_pstate/aperf.c | 17 ++++++++++++----- > 1 file changed, 12 insertions(+), 5 deletions(-) > > diff --git a/tools/testing/selftests/intel_pstate/aperf.c b/tools/testing/selftests/intel_pstate/aperf.c > index f6cd03a87493..eea9dbab459b 100644 > --- a/tools/testing/selftests/intel_pstate/aperf.c > +++ b/tools/testing/selftests/intel_pstate/aperf.c > @@ -10,6 +10,7 @@ > #include <sched.h> > #include <errno.h> > #include <string.h> > +#include <time.h> > #include "../kselftest.h" > > void usage(char *name) { > @@ -22,7 +23,7 @@ int main(int argc, char **argv) { > long long tsc, old_tsc, new_tsc; > long long aperf, old_aperf, new_aperf; > long long mperf, old_mperf, new_mperf; > - struct timeb before, after; > + struct timespec before, after; > long long int start, finish, total; > cpu_set_t cpuset; > > @@ -55,7 +56,10 @@ int main(int argc, char **argv) { > return 1; > } > > - ftime(&before); > + if (clock_gettime(CLOCK_MONOTONIC, &before) < 0) { > + perror("clock_gettime"); > + return 1; > + } > pread(fd, &old_tsc, sizeof(old_tsc), 0x10); > pread(fd, &old_aperf, sizeof(old_mperf), 0xe7); > pread(fd, &old_mperf, sizeof(old_aperf), 0xe8); > @@ -64,7 +68,10 @@ int main(int argc, char **argv) { > sqrt(i); > } > > - ftime(&after); > + if (clock_gettime(CLOCK_MONOTONIC, &after) < 0) { > + perror("clock_gettime"); > + return 1; > + } > pread(fd, &new_tsc, sizeof(new_tsc), 0x10); > pread(fd, &new_aperf, sizeof(new_mperf), 0xe7); > pread(fd, &new_mperf, sizeof(new_aperf), 0xe8); > @@ -73,8 +80,8 @@ int main(int argc, char **argv) { > aperf = new_aperf-old_aperf; > mperf = new_mperf-old_mperf; > > - start = before.time*1000 + before.millitm; > - finish = after.time*1000 + after.millitm; > + start = before.tv_sec*1000 + before.tv_nsec/1000000L; > + finish = after.tv_sec*1000 + after.tv_nsec/1000000L; Why not use timespec dNSEC_PER_MSEC define from include/vdso/time64.h? thanks, -- Shuah
On Tue, 2020-10-27 at 14:08 -0600, Shuah Khan wrote: > > > @@ -73,8 +80,8 @@ int main(int argc, char **argv) { > > aperf = new_aperf-old_aperf; > > mperf = new_mperf-old_mperf; > > > > - start = before.time*1000 + before.millitm; > > - finish = after.time*1000 + after.millitm; > > + start = before.tv_sec*1000 + before.tv_nsec/1000000L; > > + finish = after.tv_sec*1000 + after.tv_nsec/1000000L; > > Why not use timespec dNSEC_PER_MSEC define from include/vdso/time64.h? Hi, If the define was available in the UAPI headers, then certainly would make sense to use it. But I would not mess with the kernel internal headers here. -Tommi
On 10/28/20 1:36 AM, Rantala, Tommi T. (Nokia - FI/Espoo) wrote: > On Tue, 2020-10-27 at 14:08 -0600, Shuah Khan wrote: >> >>> @@ -73,8 +80,8 @@ int main(int argc, char **argv) { >>> aperf = new_aperf-old_aperf; >>> mperf = new_mperf-old_mperf; >>> >>> - start = before.time*1000 + before.millitm; >>> - finish = after.time*1000 + after.millitm; >>> + start = before.tv_sec*1000 + before.tv_nsec/1000000L; >>> + finish = after.tv_sec*1000 + after.tv_nsec/1000000L; >> >> Why not use timespec dNSEC_PER_MSEC define from include/vdso/time64.h? > > Hi, > > If the define was available in the UAPI headers, then certainly would make > sense to use it. But I would not mess with the kernel internal headers here. Suggested the wrong file while looking up the define. I was thinking linux/time64.h However it isn't part of headers_install, so can't use that one. Considering the number of places NSEC_PER_MSEC is hard coded and defined in headers e.g: tools/include/linux/time64.h, probably should be included in timespec block in time.h Not something to worry about for this patch. Please add a NSEC_PER_MSEC define for now in this file. thanks, -- Shuah
diff --git a/tools/testing/selftests/intel_pstate/aperf.c b/tools/testing/selftests/intel_pstate/aperf.c index f6cd03a87493..eea9dbab459b 100644 --- a/tools/testing/selftests/intel_pstate/aperf.c +++ b/tools/testing/selftests/intel_pstate/aperf.c @@ -10,6 +10,7 @@ #include <sched.h> #include <errno.h> #include <string.h> +#include <time.h> #include "../kselftest.h" void usage(char *name) { @@ -22,7 +23,7 @@ int main(int argc, char **argv) { long long tsc, old_tsc, new_tsc; long long aperf, old_aperf, new_aperf; long long mperf, old_mperf, new_mperf; - struct timeb before, after; + struct timespec before, after; long long int start, finish, total; cpu_set_t cpuset; @@ -55,7 +56,10 @@ int main(int argc, char **argv) { return 1; } - ftime(&before); + if (clock_gettime(CLOCK_MONOTONIC, &before) < 0) { + perror("clock_gettime"); + return 1; + } pread(fd, &old_tsc, sizeof(old_tsc), 0x10); pread(fd, &old_aperf, sizeof(old_mperf), 0xe7); pread(fd, &old_mperf, sizeof(old_aperf), 0xe8); @@ -64,7 +68,10 @@ int main(int argc, char **argv) { sqrt(i); } - ftime(&after); + if (clock_gettime(CLOCK_MONOTONIC, &after) < 0) { + perror("clock_gettime"); + return 1; + } pread(fd, &new_tsc, sizeof(new_tsc), 0x10); pread(fd, &new_aperf, sizeof(new_mperf), 0xe7); pread(fd, &new_mperf, sizeof(new_aperf), 0xe8); @@ -73,8 +80,8 @@ int main(int argc, char **argv) { aperf = new_aperf-old_aperf; mperf = new_mperf-old_mperf; - start = before.time*1000 + before.millitm; - finish = after.time*1000 + after.millitm; + start = before.tv_sec*1000 + before.tv_nsec/1000000L; + finish = after.tv_sec*1000 + after.tv_nsec/1000000L; total = finish - start; printf("runTime: %4.2f\n", 1.0*total/1000);
Use clock_gettime() instead of deprecated ftime(). aperf.c: In function ‘main’: aperf.c:58:2: warning: ‘ftime’ is deprecated [-Wdeprecated-declarations] 58 | ftime(&before); | ^~~~~ In file included from aperf.c:9: /usr/include/sys/timeb.h:39:12: note: declared here 39 | extern int ftime (struct timeb *__timebuf) | ^~~~~ Signed-off-by: Tommi Rantala <tommi.t.rantala@nokia.com> --- tools/testing/selftests/intel_pstate/aperf.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-)