* Re: cyclictest overflow instance tracking -- irc continued [not found] <507CAF63.7080703@am.sony.com> @ 2012-10-16 10:56 ` John Kacur 2012-10-16 16:11 ` Bhavesh Davda 0 siblings, 1 reply; 4+ messages in thread From: John Kacur @ 2012-10-16 10:56 UTC (permalink / raw) To: Frank Rowand; +Cc: John Kacur, bhavesh, linux-rt-users 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 > > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: cyclictest overflow instance tracking -- irc continued 2012-10-16 10:56 ` cyclictest overflow instance tracking -- irc continued John Kacur @ 2012-10-16 16:11 ` Bhavesh Davda 2012-10-16 16:14 ` Bhavesh Davda 0 siblings, 1 reply; 4+ messages in thread From: Bhavesh Davda @ 2012-10-16 16:11 UTC (permalink / raw) To: John Kacur; +Cc: Frank Rowand, linux-rt-users@vger.kernel.org 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@vmware.com -- To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: cyclictest overflow instance tracking -- irc continued 2012-10-16 16:11 ` Bhavesh Davda @ 2012-10-16 16:14 ` Bhavesh Davda 2012-10-16 16:23 ` John Kacur 0 siblings, 1 reply; 4+ messages in thread From: Bhavesh Davda @ 2012-10-16 16:14 UTC (permalink / raw) To: John Kacur; +Cc: Frank Rowand, linux-rt-users@vger.kernel.org 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@vmware.com > > > > > > -- Bhavesh Davda bhavesh@vmware.com -- To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: cyclictest overflow instance tracking -- irc continued 2012-10-16 16:14 ` Bhavesh Davda @ 2012-10-16 16:23 ` John Kacur 0 siblings, 0 replies; 4+ messages in thread From: John Kacur @ 2012-10-16 16:23 UTC (permalink / raw) To: Bhavesh Davda; +Cc: John Kacur, Frank Rowand, linux-rt, Clark Williams On Tue, 16 Oct 2012, Bhavesh Davda wrote: > Also, just to confirm: where is the GIT tree located? > > I was sync'ed to https://github.com/clrkwllms/rt-tests.git The above one was a temporary place while the problems with kernel.org were on going. > > But I also see a tree at http://git.kernel.org/pub/scm/linux/kernel/git/clrkwllms/rt-tests.git The above one is the correct one. Thanks John ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-10-16 16:21 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <507CAF63.7080703@am.sony.com>
2012-10-16 10:56 ` cyclictest overflow instance tracking -- irc continued John Kacur
2012-10-16 16:11 ` Bhavesh Davda
2012-10-16 16:14 ` Bhavesh Davda
2012-10-16 16:23 ` John Kacur
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).