* [PATCH 0/2] [rt-tests] Two minor fixes of cyclictest @ 2010-03-07 20:39 Carsten Emde 2010-03-07 20:39 ` [PATCH 1/2] do-not-allow-prio-less-than-one-in-cyclictest.patch Carsten Emde 2010-03-07 20:39 ` [PATCH 2/2] remove-incorrect-options-from-smp-help-message-in-cyclictest.patch Carsten Emde 0 siblings, 2 replies; 9+ messages in thread From: Carsten Emde @ 2010-03-07 20:39 UTC (permalink / raw) To: Clark Williams; +Cc: RT-users These patches prevent a priority < 1 and fix an incorrect help message in cyclictest. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] do-not-allow-prio-less-than-one-in-cyclictest.patch 2010-03-07 20:39 [PATCH 0/2] [rt-tests] Two minor fixes of cyclictest Carsten Emde @ 2010-03-07 20:39 ` Carsten Emde 2010-03-07 22:37 ` John Kacur 2010-03-07 20:39 ` [PATCH 2/2] remove-incorrect-options-from-smp-help-message-in-cyclictest.patch Carsten Emde 1 sibling, 1 reply; 9+ messages in thread From: Carsten Emde @ 2010-03-07 20:39 UTC (permalink / raw) To: Clark Williams; +Cc: RT-users, Carsten Emde [-- Attachment #1: do-not-allow-prio-below-one-in-cyclictest.patch --] [-- Type: text/plain, Size: 808 bytes --] If not in SMP testing mode, the priority may go below 1, if the specified priority is lower than the number of threads, e.g. # cyclictest -p2 -t3 T: 0 (21970) P: 2 [..] T: 1 (21971) P: 1 [..] T: 2 (21972) P: 0 [..] Do not allow priority to go below 1. Signed-off-by: Carsten Emde <C.Emde@osadl.org> Index: rt-tests/src/cyclictest/cyclictest.c =================================================================== --- rt-tests.orig/src/cyclictest/cyclictest.c +++ rt-tests/src/cyclictest/cyclictest.c @@ -1304,7 +1304,7 @@ int main(int argc, char **argv) } par->prio = priority; - if (!sameprio) + if (priority > 1 && !sameprio) priority--; if (priority && policy <= 1) par->policy = SCHED_FIFO; else if (priority && policy == 2) par->policy = SCHED_RR; ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] do-not-allow-prio-less-than-one-in-cyclictest.patch 2010-03-07 20:39 ` [PATCH 1/2] do-not-allow-prio-less-than-one-in-cyclictest.patch Carsten Emde @ 2010-03-07 22:37 ` John Kacur 2010-03-09 18:02 ` David Sommerseth 0 siblings, 1 reply; 9+ messages in thread From: John Kacur @ 2010-03-07 22:37 UTC (permalink / raw) To: Carsten Emde; +Cc: Clark Williams, RT-users On Sun, Mar 7, 2010 at 9:39 PM, Carsten Emde <C.Emde@osadl.org> wrote: > If not in SMP testing mode, the priority may go below 1, if the specified > priority is lower than the number of threads, e.g. > # cyclictest -p2 -t3 > T: 0 (21970) P: 2 [..] > T: 1 (21971) P: 1 [..] > T: 2 (21972) P: 0 [..] > > Do not allow priority to go below 1. > > Signed-off-by: Carsten Emde <C.Emde@osadl.org> > > Index: rt-tests/src/cyclictest/cyclictest.c > =================================================================== > --- rt-tests.orig/src/cyclictest/cyclictest.c > +++ rt-tests/src/cyclictest/cyclictest.c > @@ -1304,7 +1304,7 @@ int main(int argc, char **argv) > } > > par->prio = priority; > - if (!sameprio) > + if (priority > 1 && !sameprio) > priority--; > if (priority && policy <= 1) par->policy = SCHED_FIFO; > else if (priority && policy == 2) par->policy = SCHED_RR; > > -- I'm not sure about this, why not allow a priority below 1? The code below properly sets the third thead to SCHED_OTHER. I could imagine wanting to test that too. If you don't want to go below 1 then just set a higher prio, p3 in the scenario you showed. Thanks. John -- 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] 9+ messages in thread
* Re: [PATCH 1/2] do-not-allow-prio-less-than-one-in-cyclictest.patch 2010-03-07 22:37 ` John Kacur @ 2010-03-09 18:02 ` David Sommerseth 2010-03-09 18:12 ` John Kacur 0 siblings, 1 reply; 9+ messages in thread From: David Sommerseth @ 2010-03-09 18:02 UTC (permalink / raw) To: John Kacur; +Cc: Carsten Emde, RT-users On 07/03/10 23:37, John Kacur wrote: > On Sun, Mar 7, 2010 at 9:39 PM, Carsten Emde <C.Emde@osadl.org> wrote: >> If not in SMP testing mode, the priority may go below 1, if the specified >> priority is lower than the number of threads, e.g. >> # cyclictest -p2 -t3 >> T: 0 (21970) P: 2 [..] >> T: 1 (21971) P: 1 [..] >> T: 2 (21972) P: 0 [..] >> >> Do not allow priority to go below 1. >> >> Signed-off-by: Carsten Emde <C.Emde@osadl.org> >> >> Index: rt-tests/src/cyclictest/cyclictest.c >> =================================================================== >> --- rt-tests.orig/src/cyclictest/cyclictest.c >> +++ rt-tests/src/cyclictest/cyclictest.c >> @@ -1304,7 +1304,7 @@ int main(int argc, char **argv) >> } >> >> par->prio = priority; >> - if (!sameprio) >> + if (priority > 1 && !sameprio) >> priority--; >> if (priority && policy <= 1) par->policy = SCHED_FIFO; >> else if (priority && policy == 2) par->policy = SCHED_RR; >> >> -- > > I'm not sure about this, why not allow a priority below 1? The code > below properly sets the third thead to SCHED_OTHER. > I could imagine wanting to test that too. If you don't want to go > below 1 then just set a higher prio, p3 in the scenario you > showed. Maybe I'm misreading and misunderstanding the patch ... but I believe the if statement should say: if (priority > 0 && !sameprio) priority--; Just to avoid the situation the commit log says, priority to go below 0. kind regards, David Sommerseth ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] do-not-allow-prio-less-than-one-in-cyclictest.patch 2010-03-09 18:02 ` David Sommerseth @ 2010-03-09 18:12 ` John Kacur 2010-03-09 18:14 ` David Sommerseth 0 siblings, 1 reply; 9+ messages in thread From: John Kacur @ 2010-03-09 18:12 UTC (permalink / raw) To: David Sommerseth; +Cc: Carsten Emde, RT-users On Tue, Mar 9, 2010 at 7:02 PM, David Sommerseth <davids@redhat.com> wrote: > On 07/03/10 23:37, John Kacur wrote: >> On Sun, Mar 7, 2010 at 9:39 PM, Carsten Emde <C.Emde@osadl.org> wrote: >>> If not in SMP testing mode, the priority may go below 1, if the specified >>> priority is lower than the number of threads, e.g. >>> # cyclictest -p2 -t3 >>> T: 0 (21970) P: 2 [..] >>> T: 1 (21971) P: 1 [..] >>> T: 2 (21972) P: 0 [..] >>> >>> Do not allow priority to go below 1. >>> >>> Signed-off-by: Carsten Emde <C.Emde@osadl.org> >>> >>> Index: rt-tests/src/cyclictest/cyclictest.c >>> =================================================================== >>> --- rt-tests.orig/src/cyclictest/cyclictest.c >>> +++ rt-tests/src/cyclictest/cyclictest.c >>> @@ -1304,7 +1304,7 @@ int main(int argc, char **argv) >>> } >>> >>> par->prio = priority; >>> - if (!sameprio) >>> + if (priority > 1 && !sameprio) >>> priority--; >>> if (priority && policy <= 1) par->policy = SCHED_FIFO; >>> else if (priority && policy == 2) par->policy = SCHED_RR; >>> >>> -- >> >> I'm not sure about this, why not allow a priority below 1? The code >> below properly sets the third thead to SCHED_OTHER. >> I could imagine wanting to test that too. If you don't want to go >> below 1 then just set a higher prio, p3 in the scenario you >> showed. > > Maybe I'm misreading and misunderstanding the patch ... but I believe > the if statement should say: > > if (priority > 0 && !sameprio) > priority--; > > Just to avoid the situation the commit log says, priority to go below 0. > > > kind regards, > > David Sommerseth The code in general there was buggy, so I reverted the patch that caused the problems in the first place. (assuming Clark merges it). Now the code will read (as it did before) if (priority && !histogram && !smp && !numa) priority--; -- 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] 9+ messages in thread
* Re: [PATCH 1/2] do-not-allow-prio-less-than-one-in-cyclictest.patch 2010-03-09 18:12 ` John Kacur @ 2010-03-09 18:14 ` David Sommerseth 0 siblings, 0 replies; 9+ messages in thread From: David Sommerseth @ 2010-03-09 18:14 UTC (permalink / raw) To: John Kacur; +Cc: Carsten Emde, RT-users -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 09/03/10 19:12, John Kacur wrote: > On Tue, Mar 9, 2010 at 7:02 PM, David Sommerseth <davids@redhat.com> wrote: >> On 07/03/10 23:37, John Kacur wrote: >>> On Sun, Mar 7, 2010 at 9:39 PM, Carsten Emde <C.Emde@osadl.org> wrote: >>>> If not in SMP testing mode, the priority may go below 1, if the specified >>>> priority is lower than the number of threads, e.g. >>>> # cyclictest -p2 -t3 >>>> T: 0 (21970) P: 2 [..] >>>> T: 1 (21971) P: 1 [..] >>>> T: 2 (21972) P: 0 [..] >>>> >>>> Do not allow priority to go below 1. >>>> >>>> Signed-off-by: Carsten Emde <C.Emde@osadl.org> >>>> >>>> Index: rt-tests/src/cyclictest/cyclictest.c >>>> =================================================================== >>>> --- rt-tests.orig/src/cyclictest/cyclictest.c >>>> +++ rt-tests/src/cyclictest/cyclictest.c >>>> @@ -1304,7 +1304,7 @@ int main(int argc, char **argv) >>>> } >>>> >>>> par->prio = priority; >>>> - if (!sameprio) >>>> + if (priority > 1 && !sameprio) >>>> priority--; >>>> if (priority && policy <= 1) par->policy = SCHED_FIFO; >>>> else if (priority && policy == 2) par->policy = SCHED_RR; >>>> >>>> -- >>> >>> I'm not sure about this, why not allow a priority below 1? The code >>> below properly sets the third thead to SCHED_OTHER. >>> I could imagine wanting to test that too. If you don't want to go >>> below 1 then just set a higher prio, p3 in the scenario you >>> showed. >> >> Maybe I'm misreading and misunderstanding the patch ... but I believe >> the if statement should say: >> >> if (priority > 0 && !sameprio) >> priority--; >> >> Just to avoid the situation the commit log says, priority to go below 0. >> >> >> kind regards, >> >> David Sommerseth > > The code in general there was buggy, so I reverted the patch that > caused the problems in the first place. > (assuming Clark merges it). > > Now the code will read (as it did before) > > if (priority && !histogram && !smp && !numa) > priority--; That looks better ... and guess what, I just stumbled upon those patches right now in the mailing list ... the troubles of catching up after holidays :) kind regards, David Sommerseth -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAkuWkAAACgkQIIWEatLf4HeRiQCeMLgoLJHvzHwS+8yRroGBOAAX zXwAnAmXlY4AFgIGRAzi+33nNwukXdCF =cPhd -----END PGP SIGNATURE----- ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] remove-incorrect-options-from-smp-help-message-in-cyclictest.patch 2010-03-07 20:39 [PATCH 0/2] [rt-tests] Two minor fixes of cyclictest Carsten Emde 2010-03-07 20:39 ` [PATCH 1/2] do-not-allow-prio-less-than-one-in-cyclictest.patch Carsten Emde @ 2010-03-07 20:39 ` Carsten Emde 2010-03-07 23:10 ` John Kacur 1 sibling, 1 reply; 9+ messages in thread From: Carsten Emde @ 2010-03-07 20:39 UTC (permalink / raw) To: Clark Williams; +Cc: RT-users, Carsten Emde [-- Attachment #1: remove-incorrect-options-from-smp-help-message-in-cyclictest.patch --] [-- Type: text/plain, Size: 1210 bytes --] The help message of cyclictest's -S option says that it equals -a -t -n -m -d0. In reality, it only equals -a -t -n. Signed-off-by: Carsten Emde <C.Emde@osadl.org> Index: rt-tests/src/cyclictest/cyclictest.c =================================================================== --- rt-tests.orig/src/cyclictest/cyclictest.c +++ rt-tests/src/cyclictest/cyclictest.c @@ -777,8 +777,8 @@ static void display_help(int error) "-W --wakeuprt rt task wakeup tracing (used with -b)\n" "-y POLI --policy=POLI policy of realtime thread (1:FIFO, 2:RR)\n" " format: --policy=fifo(default) or --policy=rr\n" - "-S --smp Standard SMP testing (equals -a -t -n -m -d0)\n" - " same priority on all threads.\n" + "-S --smp Standard SMP testing: options -a -t -n and\n" + " same priority of all threads\n" "-U --numa Standard NUMA testing (similar to SMP option)\n" " thread data structures allocated from local node\n", tracers ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] remove-incorrect-options-from-smp-help-message-in-cyclictest.patch 2010-03-07 20:39 ` [PATCH 2/2] remove-incorrect-options-from-smp-help-message-in-cyclictest.patch Carsten Emde @ 2010-03-07 23:10 ` John Kacur 2010-03-07 23:49 ` John Kacur 0 siblings, 1 reply; 9+ messages in thread From: John Kacur @ 2010-03-07 23:10 UTC (permalink / raw) To: Carsten Emde; +Cc: Clark Williams, RT-users On Sun, Mar 7, 2010 at 9:39 PM, Carsten Emde <C.Emde@osadl.org> wrote: > The help message of cyclictest's -S option says that it equals -a -t -n -m -d0. > In reality, it only equals -a -t -n. > > Signed-off-by: Carsten Emde <C.Emde@osadl.org> > > Index: rt-tests/src/cyclictest/cyclictest.c > =================================================================== > --- rt-tests.orig/src/cyclictest/cyclictest.c > +++ rt-tests/src/cyclictest/cyclictest.c > @@ -777,8 +777,8 @@ static void display_help(int error) > "-W --wakeuprt rt task wakeup tracing (used with -b)\n" > "-y POLI --policy=POLI policy of realtime thread (1:FIFO, 2:RR)\n" > " format: --policy=fifo(default) or --policy=rr\n" > - "-S --smp Standard SMP testing (equals -a -t -n -m -d0)\n" > - " same priority on all threads.\n" > + "-S --smp Standard SMP testing: options -a -t -n and\n" > + " same priority of all threads\n" > "-U --numa Standard NUMA testing (similar to SMP option)\n" > " thread data structures allocated from local node\n", > tracers > > -- Wow, you're right. I wonder if the intention here was to set -m and -d0 for -S, and if we instead need a patch to do exactly that if the user selects -S. Certainly a default of -d0 makes sense for smp, and it probably doesn't make sense to let the user override it either. However, I'm not sure if -m by default is a good choice to make for -S. I suggest we make -d0 the default for -S, and then see if anyone has an opinion about whether -m should be the default for -S. My vote is no, so then we would need to correct the documentation in that case. Thanks John -- 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] 9+ messages in thread
* Re: [PATCH 2/2] remove-incorrect-options-from-smp-help-message-in-cyclictest.patch 2010-03-07 23:10 ` John Kacur @ 2010-03-07 23:49 ` John Kacur 0 siblings, 0 replies; 9+ messages in thread From: John Kacur @ 2010-03-07 23:49 UTC (permalink / raw) To: Carsten Emde; +Cc: Clark Williams, RT-users On Mon, Mar 8, 2010 at 12:10 AM, John Kacur <jkacur@redhat.com> wrote: > On Sun, Mar 7, 2010 at 9:39 PM, Carsten Emde <C.Emde@osadl.org> wrote: >> The help message of cyclictest's -S option says that it equals -a -t -n -m -d0. >> In reality, it only equals -a -t -n. >> >> Signed-off-by: Carsten Emde <C.Emde@osadl.org> >> >> Index: rt-tests/src/cyclictest/cyclictest.c >> =================================================================== >> --- rt-tests.orig/src/cyclictest/cyclictest.c >> +++ rt-tests/src/cyclictest/cyclictest.c >> @@ -777,8 +777,8 @@ static void display_help(int error) >> "-W --wakeuprt rt task wakeup tracing (used with -b)\n" >> "-y POLI --policy=POLI policy of realtime thread (1:FIFO, 2:RR)\n" >> " format: --policy=fifo(default) or --policy=rr\n" >> - "-S --smp Standard SMP testing (equals -a -t -n -m -d0)\n" >> - " same priority on all threads.\n" >> + "-S --smp Standard SMP testing: options -a -t -n and\n" >> + " same priority of all threads\n" >> "-U --numa Standard NUMA testing (similar to SMP option)\n" >> " thread data structures allocated from local node\n", >> tracers >> >> -- > > Wow, you're right. I wonder if the intention here was to set -m and > -d0 for -S, and if we instead need a patch to do exactly > that if the user selects -S. > > Certainly a default of -d0 makes sense for smp, and it probably > doesn't make sense to let the user override it either. > However, I'm not sure if -m by default is a good choice to make for > -S. I suggest we make -d0 the default for -S, and > then see if anyone has an opinion about whether -m should be the > default for -S. My vote is no, so then we would need > to correct the documentation in that case. > Okay, looking at the history of cyclictest in git, 77abded073c5d11d118112c46c22a84ddc29e771 indicates that the intention was to set just -t -a and -n, so in that case Acked-by: John Kacur <jkacur@redhat.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] 9+ messages in thread
end of thread, other threads:[~2010-03-09 18:14 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-03-07 20:39 [PATCH 0/2] [rt-tests] Two minor fixes of cyclictest Carsten Emde 2010-03-07 20:39 ` [PATCH 1/2] do-not-allow-prio-less-than-one-in-cyclictest.patch Carsten Emde 2010-03-07 22:37 ` John Kacur 2010-03-09 18:02 ` David Sommerseth 2010-03-09 18:12 ` John Kacur 2010-03-09 18:14 ` David Sommerseth 2010-03-07 20:39 ` [PATCH 2/2] remove-incorrect-options-from-smp-help-message-in-cyclictest.patch Carsten Emde 2010-03-07 23:10 ` John Kacur 2010-03-07 23:49 ` 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).