RE: [PATCH 1/2][RFC] Add README_cyclicload

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




-----Original Message-----
From: John Kacur [mailto:jkacur@xxxxxxxxx] On Behalf Of John Kacur
Sent: Wednesday, August 15, 2012 2:05 AM
To: Clark Williams
Cc: Jain Priyanka-B32167; John Kacur; Frank Rowand; linux-rt-users@xxxxxxxxxxxxxxx; dvhart@xxxxxxxxxxxxxxx; rostedt@xxxxxxxxxxx; tglx@xxxxxxxxxxxxx; Srivastava Rajan-B34330; Aggrwal Poonam-B10812
Subject: Re: [PATCH 1/2][RFC] Add README_cyclicload



On Tue, 14 Aug 2012, Clark Williams wrote:

> 
> My comments inline.
> 
> On Tue, 14 Aug 2012 10:15:28 +0000
> Jain Priyanka-B32167 <B32167@xxxxxxxxxxxxx> wrote:
> 
> > 
> > 
> > -----Original Message-----
> > From: linux-rt-users-owner@xxxxxxxxxxxxxxx 
> > [mailto:linux-rt-users-owner@xxxxxxxxxxxxxxx] On Behalf Of Jain 
> > Priyanka-B32167
> > Sent: Tuesday, August 14, 2012 2:06 PM
> > To: John Kacur; Frank Rowand
> > Cc: Clark Williams; linux-rt-users@xxxxxxxxxxxxxxx; 
> > dvhart@xxxxxxxxxxxxxxx; rostedt@xxxxxxxxxxx; tglx@xxxxxxxxxxxxx; 
> > Srivastava Rajan-B34330; Aggrwal Poonam-B10812
> > Subject: RE: [PATCH 1/2][RFC] Add README_cyclicload
> > 
> > 
> > Thanks John and Frank for going through it.
> > 
> > As Frank has suggested to make it separate application and  as John also pointed out that it is breaking some of cyclic-test features, and also the targeted use-case is different, so I think it's better to maintain it as separate tool. Please comments on this. 
> 
> What I'd *really* like to do is pull the common routines out of 
> cyclictest.c and put them in separate object files, so that we could 
> create cyclictest-like-tests such as cyclicload without having to cram 
> the new logic into cyclictest.  As Frank mentioned, cyclictest is 
> complicated enough and somewhat fragile, so adding new logic tends to 
> just add new bugs as well.
> 
> John (and Frank and the test of the CC list) what do you think?
> 

Yes, that is what we want to do, and we've started doing so, although it is still pretty bare bones. This code is in src/lib. We should keep enhancing this.

But it's too much of course to ask someone to do this who just wants to get a new tool into the code. So, I would require this.

1. The new tool doesn't break cyclictest, this is rule number one.
2. The new tool does what it claims to do, and is useful.
3. Doesn't unncessarily complicate cyclictest.

I think you automatically lower the bar if you can do this separately because you won't be breaking cyclictest. Of course, it doesn't seem very smart to just go and duplicate a lot of code though, it might be good enough for a prototype. Is a prototype good enough for rttests though?

I think the answer to that depends on meeting requirement number 2.

My first impression running cyclicload was, that it needs a bit of work.
One example of many, using the command line for smp he gave in his README I got

(gdb) r  -p 99 -c 1 -d 0 -x 40 -X 30 -q -D 600 -S Program received signal SIGFPE, Arithmetic exception.
[Switching to Thread 0x7ffff7fcc700 (LWP 21506)]
0x00000000004042c5 in timerthread (param=0x60c490)
    at src/cyclictest/cyclictest.c:1032
1032					((stat->done_t1 * 
100)/stat->num_t1))/
glibc-2.15-51.fc17.x86_64 numactl-libs-2.0.7-6.fc17.x86_64
(gdb) print stat->num_t1
$1 = 0
 
It has to be at a certain level before we're interested in reviewing it more closely. I'm not sure if Jain was just showing us a prototype or he thought it was ready to go in?

[Priyanka]: It is a RFC version . I am working on to make it a robust one. I will send the polished version soon. My major concern for the polished version is to propose this as separate tool or as part of cyclictest . Also, as you have mentioned above that cyclic-load need a bit of work, can you please mentioned those rework required liked the one you highlighted. It will help me in correcting the tool faster. Thanks a lot, Looking forward for your support.

I'm not willing to review it until it's polished some more.

 --->o snip


--
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


[Index of Archives]     [RT Stable]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]

  Powered by Linux