Also, just to confirm: where is the GIT tree located? I was sync'ed to https://github.com/clrkwllms/rt-tests.git But I also see a tree at http://git.kernel.org/pub/scm/linux/kernel/git/clrkwllms/rt-tests.git Thanks On Oct 16, 2012, at 9:11 AM, Bhavesh Davda wrote: > 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 > > > > > > -- 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
