What changed.
A general cleanup of test.
A man page added.
More comments.
Addressing issues, see below.
On 4/1/20 4:02 PM, Thomas Gleixner wrote:
>
> The exact purpose of this is?
>
>> +
>> +#define CHECK_LOST_TIME() \
>> + do { \
>> + if (d >= dt_min) { \
This is the update_buckets call, if time difference is greater than a threshold it is added to a bucket.
> Aside of having a \ at the last line, macros which rely on variable
> names at the call site are broken to begin with. What's so magic to hide
> this in a macro instead of writing a proper function?
This was changed to a function.
>> +static inline uint64_t tsc(void)
>> +{
>> + uint64_t ret = 0;
>> + uint32_t l, h;
>> +
>> + __asm__ __volatile__("lfence");
>> + __asm__ __volatile__("rdtsc" : "=a"(l), "=d"(h));
>> + ret = ((uint64_t)h << 32) | l;
>> + return ret;
>> +}
> Having x86 specific code in a generic test suite is a non starter.
If-def added to build of x86, else provide a runtime error.
> + char *freq[2] = {
> + "cpuinfo_cur_freq",
> + /* assumes a busy wait will be run at the max freq */
> Assumptions in tools which are meant to provide useful output are really
> not useful at all.
These frequencies are only starting points, the test recalculates the frequency at runtime.
>> + if (f) {
>> + fscanf(f, "%lu", &fs);
> That definitely has never seen a 32bit compile.
Changed to use PRIu64 macro and verified on i386 compile.
> +
> + if (fs == (uint64_t) -1) {
> So here you have a typecast but at the place where this is initialized
> this is not required, right?
Typecast removed.
>
>> + return 0;
> As the above is completely unreadable gibberish I can only assume that I
> wasted time staring at a well done April 1st joke :)
Rewrite + comments should make the understanding the main loop easier.
Tom
