autotest / autotest-client-tests

Autotest client tests
Other
29 stars 84 forks source link

rtc: Also treat RTC_ALM_READ in rtctest on ppc64 #123

Closed ldoktor closed 6 years ago

ldoktor commented 7 years ago

The rtc chip defined in qemu guest on ppc64 does not support alarms at all, therefor RTC_ALM_READ fails with EINVAL. Let's proceed to next test in such case.

@KarMez-Redhat, @clebergnu this addresses the in-guest issue with rtctest.

luckyh commented 7 years ago

The source file comes from kernel tree, so we can have a look at the newest one https://github.com/torvalds/linux/blob/master/tools/testing/selftests/timers/rtctest.c.

ldoktor commented 7 years ago

@luckyh that one contains +1 (dangerous) test, but does not handle the power's minimized RTC. Hmm, sending our fixes upstream could answer some questions... I'll consider doing that.

luckyh commented 7 years ago

@ldoktor indeed, it needs your fix to support the pseries rtc device (the rtc-generic module).

ldoktor commented 6 years ago

@luckyh the upstream fix was applied. Do you want me to send v2 to sync with the latest version, or do you think we should simply go with this bugfix only?

luckyh commented 6 years ago

@ldoktor thanks for your work! then, let's use the latest version.

ldoktor commented 6 years ago

@luckyh updated

ldoktor commented 6 years ago

@luckyh added a second commit to bring back the maxfreq argument, which is only available in Autotest version of the rtctest.

luckyh commented 6 years ago

@ldoktor ok, thanks.

However, the test failed on my x86 workstation:

...
Periodic IRQ rate is 1024Hz.
Counting 20 interrupts at:
2Hz:     1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20
4Hz:     1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20
8Hz:     1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20
16Hz:    1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20
32Hz:    1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20
64Hz:    1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20
128Hz:   1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20
256Hz:   1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20
512Hz:   1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20
1024Hz:  1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20
2048Hz:  1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20
4096Hz:  1 2 3
PIE delta error: 0.000283 should be close to 0.000244

And the result was even worse on a QEMU VM (could not pass with 1024HZ).

According to the first commit, the PIE delta check is newly introduced.

+ if (diff.tv_sec > 0 ||
+     diff.tv_usec > ((1000000L / tmp) * 1.10)) {
+         fprintf(stderr, "\nPIE delta error: %ld.%06ld should be close to 0.%06ld\n",
+                 diff.tv_sec, diff.tv_usec,
+                 (1000000L / tmp));
+         fflush(stdout);
+         exit(-1);
+ }

I guess the problem is this check is too strict, but need to confirm it.

ldoktor commented 6 years ago

Well the check allows 10% which might be too tight for >64Hz. It worked well on my machine with 8192Hz so I kept it there. Do you want me to remove this test for >64Hz, or to put there a min value?

ldoktor commented 6 years ago

.. or should we keep it there and wait for users to report issues, if they use >64Hz? I think that would be the best.

luckyh commented 6 years ago

Ok, let's keep it.

ACK

ldoktor commented 6 years ago

OK, thank you, merging it...