Sorry about that John, Frank! Don't know how that happened…
Please look for two separate patches against the right ToT: one for whitespace cleanup, and the other with this feature, in the next few hours…
Thanks
- Bhavesh
On Oct 16, 2012, at 3:56 AM, John Kacur wrote:
>
>
> On Mon, 15 Oct 2012, Frank Rowand wrote:
>
> Thanks for your further review Frank. Bhavesh - I think the right thing to
> do here, is to ask you to respin your patch. Can you make sure you are
> applying and testing against the latest. (0.84). Also, please put any
> white space clean-ups in a separate patch to make it easier to read the
> important patch. Oh, and send these in a new thread. If people could
> preface their mail with [PATCH RT-TESTS] that would also help.
>
> Thank You.
>
> John Kacur
>
>>
>> 16:13 jkacur frowand_bot: would you like a Reviewed-by: credit for the cyclictest histogram overflow instance tracking patch?
>> 16:13 jkacur can I just add that or is the reviewer supposed to request it?
>> 17:02 frowand jkacur: sure, you can add my reviewed-by (at least for the first version, i didn't really look at the second version in detail)
>>
>> For the first version, it was a drive-by review, just obvious stuff.
>>
>> Now that I look at the second version a little closer (but still not in detail), it
>> looks like he did some white space clean up, which should probably be a separate patch.
>>
>> It also looks like he created his patch against an old version of cyclictest,
>> because he has:
>>
>> @@ -154,6 +155,7 @@ struct thread_stat {
>> long redmax;
>> long cycleofmax;
>> long hist_overflow;
>> + long num_outliers;
>> };
>>
>>
>> but struct thread_stat currently has another field:
>>
>> long redmax;
>> long cycleofmax;
>> long hist_overflow;
>> unsigned long now_after_next;
>> };
>>
>> and also the line numbers in the patch are off.
>>
>>> from #linux-rt:
>>>
>>> 17:07 jkacur frowand: hmm, the patch doesn't appear to be working for me
>>> 17:07 jkacur I'll look at it more tomorrow
>>> 17:08 jkacur Thread 0: 18446744073709551615 00000 00001 00002 00003 00004 00005 00006 00007 00008 # 02841 others
>>> 17:08 jkacur is an example of broken output
>>
>> 18446744073709551615 is 0xffffffffffffffff (or -1LL)
>>
>> It looks like in timerthread(), the line:
>>
>> + stat->outliers[stat->num_outliers++] = stat->cycles - 1;
>>
>> is picking up stat->cycles == zero. Looking at the current version of cyclictest, for the
>> first event cycles should already be one at this point. But given that the line numbers in
>> the patch are not consistent with the current cyclictest, I don't know where that chunk
>> actually got inserted.
>>
>>
>> -Frank
>>
>>
--
Bhavesh Davda
bhavesh@xxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
