Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1635852 > unrolled thread
| Started by | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| First post | 2017-05-04 17:40 +0200 |
| Last post | 2017-05-06 10:00 +0200 |
| Articles | 11 — 3 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [PATCH tip/sched/core v2] sched/rt: Simplify the IPI rt balancing logic Peter Zijlstra <peterz@infradead.org> - 2017-05-04 17:40 +0200
Re: [PATCH tip/sched/core v2] sched/rt: Simplify the IPI rt balancing logic Steven Rostedt <rostedt@goodmis.org> - 2017-05-04 19:30 +0200
Re: [PATCH tip/sched/core v2] sched/rt: Simplify the IPI rt balancing logic Peter Zijlstra <peterz@infradead.org> - 2017-05-04 20:50 +0200
Re: [PATCH tip/sched/core v2] sched/rt: Simplify the IPI rt balancing logic Steven Rostedt <rostedt@goodmis.org> - 2017-05-04 21:10 +0200
Re: [PATCH tip/sched/core v2] sched/rt: Simplify the IPI rt balancing logic Mike Galbraith <efault@gmx.de> - 2017-05-05 06:30 +0200
Re: [PATCH tip/sched/core v2] sched/rt: Simplify the IPI rt balancing logic Mike Galbraith <efault@gmx.de> - 2017-05-05 07:20 +0200
Re: [PATCH tip/sched/core v2] sched/rt: Simplify the IPI rt balancing logic Peter Zijlstra <peterz@infradead.org> - 2017-05-05 13:10 +0200
Re: [PATCH tip/sched/core v2] sched/rt: Simplify the IPI rt balancing logic Steven Rostedt <rostedt@goodmis.org> - 2017-05-05 14:10 +0200
Re: [PATCH tip/sched/core v2] sched/rt: Simplify the IPI rt balancing logic Peter Zijlstra <peterz@infradead.org> - 2017-05-05 19:50 +0200
Re: [PATCH tip/sched/core v2] sched/rt: Simplify the IPI rt balancing logic Steven Rostedt <rostedt@goodmis.org> - 2017-05-05 21:00 +0200
Re: [PATCH tip/sched/core v2] sched/rt: Simplify the IPI rt balancing logic Peter Zijlstra <peterz@infradead.org> - 2017-05-06 10:00 +0200
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2017-05-04 17:40 +0200 |
| Subject | Re: [PATCH tip/sched/core v2] sched/rt: Simplify the IPI rt balancing logic |
| Message-ID | <tDr1f-5lB-13@gated-at.bofh.it> |
On Mon, Apr 24, 2017 at 11:47:32AM -0400, Steven Rostedt wrote:
> static int rto_next_cpu(struct rq *rq)
> {
> int cpu;
>
> /*
> + * When starting the IPI RT pushing, the rto_cpu is set to nr_cpu_ids
> + * or greater. rt_next_cpu() will simply return the first CPU found in
> + * the rto_mask.
> + *
> + * If rto_next_cpu() is called with rto_cpu less than nr_cpu_ids, it
> + * will return the next CPU found in the rto_mask.
> + *
> + * If there are no more CPUs left in the rto_mask, then a check is made
> + * against rto_loop and rto_loop_next. rto_loop is only updated with
> + * the rto_lock held, but any CPU may increment the rto_loop_next
> + * without any locking.
> */
> +again:
> + if (rq->rd->rto_cpu >= nr_cpu_ids) {
> cpu = cpumask_first(rq->rd->rto_mask);
> + rq->rd->rto_cpu = cpu;
> + /* If cpu is nr_cpu_ids, then there is no overloaded rqs */
> + return cpu;
> }
>
> + cpu = cpumask_next(rq->rd->rto_cpu, rq->rd->rto_mask);
> + rq->rd->rto_cpu = cpu;
>
> + if (cpu < nr_cpu_ids)
> + return cpu;
>
> + if (rq->rd->rto_loop == atomic_read(&rq->rd->rto_loop_next))
> + return cpu;
>
> + rq->rd->rto_loop = atomic_read(&rq->rd->rto_loop_next);
> + goto again;
> +}
I think you want to write that as:
struct root_domain *rd = rq->rd;
int cpu, next;
/* comment */
for (;;) {
if (rd->rto_cpu >= nr_cpu_ids) {
cpu = cpumask_first(rd->rto_mask);
rd->rto_cpu = cpu;
return cpu;
}
cpu = cpumask_next(rd->rto_mask);
rd->rto_cpu = cpu;
if (cpu < nr_cpu_ids)
break;
// rd->rto_cpu = -1;
/*
* ACQUIRE ensures we see the @rto_mask changes
* made prior to the @next value observed.
*
* Matches WMB in rt_set_overload().
*/
next = atomic_read_acquire(&rd->rto_loop_next);
if (rd->rto_loop == next)
break;
rd->rto_loop = next;
}
return cpu;
And I don't fully understand the whole rto_cpu >= nr_cpus_ids thing,
can't you simply reset the thing to -1 and always use cpumask_next()?
As per the // comment above?
> +static inline bool rto_start_trylock(atomic_t *v)
> +{
> + return !atomic_cmpxchg(v, 0, 1);
Arguably this could be: !atomic_cmpxchg_acquire(v, 0, 1);
> }
>
> +static inline void rto_start_unlock(atomic_t *v)
> +{
> + atomic_set_release(v, 0);
> +}
>
> static void tell_cpu_to_push(struct rq *rq)
> {
> + int cpu = nr_cpu_ids;
>
> + /* Keep the loop going if the IPI is currently active */
> + atomic_inc_return(&rq->rd->rto_loop_next);
Since rt_set_overload() already provides a WMB, we don't need an
ordered primitive here and atomic_inc() is fine.
>
> + /* Only one CPU can initiate a loop at a time */
> + if (!rto_start_trylock(&rq->rd->rto_loop_start))
> return;
>
> + raw_spin_lock(&rq->rd->rto_lock);
> +
> + /*
> + * The rto_cpu is updated under the lock, if it has a valid cpu
> + * then the IPI is still running and will continue due to the
> + * update to loop_next, and nothing needs to be done here.
> + * Otherwise it is finishing up and an ipi needs to be sent.
> + */
> + if (rq->rd->rto_cpu >= nr_cpu_ids)
// if (rq->rd->rto_cpu < 0)
> + cpu = rto_next_cpu(rq);
>
> + raw_spin_unlock(&rq->rd->rto_lock);
> +
> + rto_start_unlock(&rq->rd->rto_loop_start);
> +
> + if (cpu < nr_cpu_ids)
> + irq_work_queue_on(&rq->rd->rto_push_work, cpu);
> }
>
> /* Called from hardirq context */
> +void rto_push_irq_work_func(struct irq_work *work)
> {
> + struct rq *rq;
> int this_cpu;
> int cpu;
>
> + this_cpu = smp_processor_id();
> rq = cpu_rq(this_cpu);
rq = this_rq();
>
> + /*
> + * We do not need to grab the lock to check for has_pushable_tasks.
> + * When it gets updated, a check is made if a push is possible.
> + */
> if (has_pushable_tasks(rq)) {
> raw_spin_lock(&rq->lock);
> + push_rt_tasks(rq);
> raw_spin_unlock(&rq->lock);
> }
>
> + raw_spin_lock(&rq->rd->rto_lock);
>
> + /* Pass the IPI to the next rt overloaded queue */
> + cpu = rto_next_cpu(rq);
>
> + raw_spin_unlock(&rq->rd->rto_lock);
>
> if (cpu >= nr_cpu_ids)
> return;
>
> /* Try the next RT overloaded CPU */
> + irq_work_queue_on(&rq->rd->rto_push_work, cpu);
> }
[toc] | [next] | [standalone]
| From | Steven Rostedt <rostedt@goodmis.org> |
|---|---|
| Date | 2017-05-04 19:30 +0200 |
| Message-ID | <tDsJH-6Ay-1@gated-at.bofh.it> |
| In reply to | #1635852 |
On Thu, 4 May 2017 17:32:56 +0200
Peter Zijlstra <peterz@infradead.org> wrote:
> On Mon, Apr 24, 2017 at 11:47:32AM -0400, Steven Rostedt wrote:
> > static int rto_next_cpu(struct rq *rq)
> > {
> > int cpu;
> >
> > /*
> > + * When starting the IPI RT pushing, the rto_cpu is set to nr_cpu_ids
> > + * or greater. rt_next_cpu() will simply return the first CPU found in
> > + * the rto_mask.
> > + *
> > + * If rto_next_cpu() is called with rto_cpu less than nr_cpu_ids, it
> > + * will return the next CPU found in the rto_mask.
> > + *
> > + * If there are no more CPUs left in the rto_mask, then a check is made
> > + * against rto_loop and rto_loop_next. rto_loop is only updated with
> > + * the rto_lock held, but any CPU may increment the rto_loop_next
> > + * without any locking.
> > */
> > +again:
> > + if (rq->rd->rto_cpu >= nr_cpu_ids) {
> > cpu = cpumask_first(rq->rd->rto_mask);
> > + rq->rd->rto_cpu = cpu;
> > + /* If cpu is nr_cpu_ids, then there is no overloaded rqs */
> > + return cpu;
> > }
> >
> > + cpu = cpumask_next(rq->rd->rto_cpu, rq->rd->rto_mask);
> > + rq->rd->rto_cpu = cpu;
> >
> > + if (cpu < nr_cpu_ids)
> > + return cpu;
> >
> > + if (rq->rd->rto_loop == atomic_read(&rq->rd->rto_loop_next))
> > + return cpu;
> >
> > + rq->rd->rto_loop = atomic_read(&rq->rd->rto_loop_next);
> > + goto again;
> > +}
>
> I think you want to write that as:
>
> struct root_domain *rd = rq->rd;
> int cpu, next;
>
> /* comment */
> for (;;) {
> if (rd->rto_cpu >= nr_cpu_ids) {
If we go with your change, then this needs to be:
if (rd->rto_cpu < 0) {
> cpu = cpumask_first(rd->rto_mask);
> rd->rto_cpu = cpu;
> return cpu;
> }
>
> cpu = cpumask_next(rd->rto_mask);
cpumask_next() requires two parameters.
> rd->rto_cpu = cpu;
>
> if (cpu < nr_cpu_ids)
> break;
>
> // rd->rto_cpu = -1;
>
> /*
> * ACQUIRE ensures we see the @rto_mask changes
> * made prior to the @next value observed.
> *
> * Matches WMB in rt_set_overload().
> */
> next = atomic_read_acquire(&rd->rto_loop_next);
>
> if (rd->rto_loop == next)
> break;
>
> rd->rto_loop = next;
> }
>
> return cpu;
>
> And I don't fully understand the whole rto_cpu >= nr_cpus_ids thing,
> can't you simply reset the thing to -1 and always use cpumask_next()?
> As per the // comment above?
>
> > +static inline bool rto_start_trylock(atomic_t *v)
> > +{
> > + return !atomic_cmpxchg(v, 0, 1);
>
> Arguably this could be: !atomic_cmpxchg_acquire(v, 0, 1);
Yes agreed. But if you remember, at the time I was basing this off of
tip/sched/core, which didn't have atomic_cmpxchg_acquire() available.
>
> > }
> >
> > +static inline void rto_start_unlock(atomic_t *v)
> > +{
> > + atomic_set_release(v, 0);
> > +}
> >
>
> > static void tell_cpu_to_push(struct rq *rq)
> > {
> > + int cpu = nr_cpu_ids;
> >
> > + /* Keep the loop going if the IPI is currently active */
> > + atomic_inc_return(&rq->rd->rto_loop_next);
>
> Since rt_set_overload() already provides a WMB, we don't need an
> ordered primitive here and atomic_inc() is fine.
Agree, I mentioned this in my previous reply. It was leftover from
previous versions of the patch. I believe I also needed a memory
barrier with this and the check for rto_loop_start. Can't remember if
that was the case, but it doesn't matter now as loop_start is now
updated with a cmpxchg.
>
> >
> > + /* Only one CPU can initiate a loop at a time */
> > + if (!rto_start_trylock(&rq->rd->rto_loop_start))
> > return;
> >
> > + raw_spin_lock(&rq->rd->rto_lock);
> > +
> > + /*
> > + * The rto_cpu is updated under the lock, if it has a valid cpu
> > + * then the IPI is still running and will continue due to the
> > + * update to loop_next, and nothing needs to be done here.
> > + * Otherwise it is finishing up and an ipi needs to be sent.
> > + */
> > + if (rq->rd->rto_cpu >= nr_cpu_ids)
> // if (rq->rd->rto_cpu < 0)
This can be done, I was just being a bit more conservative and having
rto_cpu have less states (valid CPU or nr_cpu_ids). With a -1, we have
to manually set it to that. But I'm fine with doing it that way too.
This went through several iterations. There were times where using a -1
wasn't so simple.
>
> > + cpu = rto_next_cpu(rq);
> >
> > + raw_spin_unlock(&rq->rd->rto_lock);
> > +
> > + rto_start_unlock(&rq->rd->rto_loop_start);
> > +
> > + if (cpu < nr_cpu_ids)
> > + irq_work_queue_on(&rq->rd->rto_push_work, cpu);
> > }
> >
> > /* Called from hardirq context */
> > +void rto_push_irq_work_func(struct irq_work *work)
> > {
> > + struct rq *rq;
> > int this_cpu;
> > int cpu;
> >
> > + this_cpu = smp_processor_id();
> > rq = cpu_rq(this_cpu);
>
> rq = this_rq();
Heh, sure. I guess I was just keeping it with the previous logic.
Thanks for the review. I'll spin up a new patch. Unfortunately, I no
longer have access to the behemoth machine. I'll only be testing this
on 4 cores now, or 8 with HT.
-- Steve
>
> >
> > + /*
> > + * We do not need to grab the lock to check for has_pushable_tasks.
> > + * When it gets updated, a check is made if a push is possible.
> > + */
> > if (has_pushable_tasks(rq)) {
> > raw_spin_lock(&rq->lock);
> > + push_rt_tasks(rq);
> > raw_spin_unlock(&rq->lock);
> > }
> >
> > + raw_spin_lock(&rq->rd->rto_lock);
> >
> > + /* Pass the IPI to the next rt overloaded queue */
> > + cpu = rto_next_cpu(rq);
> >
> > + raw_spin_unlock(&rq->rd->rto_lock);
> >
> > if (cpu >= nr_cpu_ids)
> > return;
> >
> > /* Try the next RT overloaded CPU */
> > + irq_work_queue_on(&rq->rd->rto_push_work, cpu);
> > }
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2017-05-04 20:50 +0200 |
| Message-ID | <tDtZ8-7l4-9@gated-at.bofh.it> |
| In reply to | #1635909 |
On Thu, May 04, 2017 at 01:25:38PM -0400, Steven Rostedt wrote:
> > I think you want to write that as:
> >
> > struct root_domain *rd = rq->rd;
> > int cpu, next;
> >
> > /* comment */
> > for (;;) {
> > if (rd->rto_cpu >= nr_cpu_ids) {
>
> If we go with your change, then this needs to be:
>
> if (rd->rto_cpu < 0) {
>
> > cpu = cpumask_first(rd->rto_mask);
> > rd->rto_cpu = cpu;
> > return cpu;
> > }
No you can leave it out entirely.
> >
> > cpu = cpumask_next(rd->rto_mask);
>
> cpumask_next() requires two parameters.
Indeed it does:
cpu = cpumask_next(rd->rto_cpu, rd->rto_mask);
will be cpumask_first() when rto_cpu == -1, see for example
for_each_cpu().
> > > +static inline bool rto_start_trylock(atomic_t *v)
> > > +{
> > > + return !atomic_cmpxchg(v, 0, 1);
> >
> > Arguably this could be: !atomic_cmpxchg_acquire(v, 0, 1);
>
> Yes agreed. But if you remember, at the time I was basing this off of
> tip/sched/core, which didn't have atomic_cmpxchg_acquire() available.
No that's the try_cmpxchg stuff, the _acquire stuff is long in.
> Thanks for the review. I'll spin up a new patch. Unfortunately, I no
> longer have access to the behemoth machine. I'll only be testing this
> on 4 cores now, or 8 with HT.
I have something with 144 CPUs in or thereabout, if you have the
testcase handy I can give it a spin.
[toc] | [prev] | [next] | [standalone]
| From | Steven Rostedt <rostedt@goodmis.org> |
|---|---|
| Date | 2017-05-04 21:10 +0200 |
| Message-ID | <tDuiu-7H9-19@gated-at.bofh.it> |
| In reply to | #1635965 |
On Thu, 4 May 2017 20:42:00 +0200
Peter Zijlstra <peterz@infradead.org> wrote:
> On Thu, May 04, 2017 at 01:25:38PM -0400, Steven Rostedt wrote:
> > > I think you want to write that as:
> > >
> > > struct root_domain *rd = rq->rd;
> > > int cpu, next;
> > >
> > > /* comment */
> > > for (;;) {
> > > if (rd->rto_cpu >= nr_cpu_ids) {
> >
> > If we go with your change, then this needs to be:
> >
> > if (rd->rto_cpu < 0) {
> >
> > > cpu = cpumask_first(rd->rto_mask);
> > > rd->rto_cpu = cpu;
> > > return cpu;
> > > }
>
> No you can leave it out entirely.
>
> > >
> > > cpu = cpumask_next(rd->rto_mask);
> >
> > cpumask_next() requires two parameters.
>
> Indeed it does:
>
> cpu = cpumask_next(rd->rto_cpu, rd->rto_mask);
>
> will be cpumask_first() when rto_cpu == -1, see for example
> for_each_cpu().
OK, I'll have to take a look.
>
> > > > +static inline bool rto_start_trylock(atomic_t *v)
> > > > +{
> > > > + return !atomic_cmpxchg(v, 0, 1);
> > >
> > > Arguably this could be: !atomic_cmpxchg_acquire(v, 0, 1);
> >
> > Yes agreed. But if you remember, at the time I was basing this off of
> > tip/sched/core, which didn't have atomic_cmpxchg_acquire() available.
>
> No that's the try_cmpxchg stuff, the _acquire stuff is long in.
OK, I was confused with the try stuff, as I remember that was what you
suggested before.
>
> > Thanks for the review. I'll spin up a new patch. Unfortunately, I no
> > longer have access to the behemoth machine. I'll only be testing this
> > on 4 cores now, or 8 with HT.
>
> I have something with 144 CPUs in or thereabout, if you have the
> testcase handy I can give it a spin.
My test case is two fold. It basically just involves running rteval.
One is to run it on latest mainline to make sure it doesn't crash. The
other is to backport it to the latest -rt patch, and see how well it
helps with latency.
To get rteval:
$ git clone git://git.kernel.org/pub/scm/linux/kernel/git/clrkwllms/rteval.git
$ cd rteval
$ git checkout origin/v2/master
-- Steve
[toc] | [prev] | [next] | [standalone]
| From | Mike Galbraith <efault@gmx.de> |
|---|---|
| Date | 2017-05-05 06:30 +0200 |
| Message-ID | <tDD2p-5aE-1@gated-at.bofh.it> |
| In reply to | #1635978 |
> To get rteval: > > $ git clone git://git.kernel.org/pub/scm/linux/kernel/git/clrkwllms/rteval.git > $ cd rteval > $ git checkout origin/v2/master ImportError: No module named ethtool Where does one find the ethtool bits it seems to depend upon? -Mike
[toc] | [prev] | [next] | [standalone]
| From | Mike Galbraith <efault@gmx.de> |
|---|---|
| Date | 2017-05-05 07:20 +0200 |
| Message-ID | <tDDON-5Im-7@gated-at.bofh.it> |
| In reply to | #1636154 |
On Fri, 2017-05-05 at 06:26 +0200, Mike Galbraith wrote: > > To get rteval: > > > > $ git clone > > git://git.kernel.org/pub/scm/linux/kernel/git/clrkwllms/rteval.git > > $ cd rteval > > $ git checkout origin/v2/master > > ImportError: No module named ethtool > > Where does one find the ethtool bits it seems to depend upon? Nevermind, google found a tarball. -Mike
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2017-05-05 13:10 +0200 |
| Message-ID | <tDJhw-Vh-17@gated-at.bofh.it> |
| In reply to | #1635978 |
On Thu, May 04, 2017 at 03:03:55PM -0400, Steven Rostedt wrote: > My test case is two fold. It basically just involves running rteval. > > One is to run it on latest mainline to make sure it doesn't crash. The > other is to backport it to the latest -rt patch, and see how well it > helps with latency. > > To get rteval: > > $ git clone git://git.kernel.org/pub/scm/linux/kernel/git/clrkwllms/rteval.git > $ cd rteval > $ git checkout origin/v2/master Blergh, that thing wants a gazillion things installed. Can't I run something simple like rt-migrate-test with some arguments to stress the box out?
[toc] | [prev] | [next] | [standalone]
| From | Steven Rostedt <rostedt@goodmis.org> |
|---|---|
| Date | 2017-05-05 14:10 +0200 |
| Message-ID | <tDKdz-1yR-5@gated-at.bofh.it> |
| In reply to | #1636314 |
On Fri, 5 May 2017 13:05:29 +0200 Peter Zijlstra <peterz@infradead.org> wrote: > On Thu, May 04, 2017 at 03:03:55PM -0400, Steven Rostedt wrote: > > My test case is two fold. It basically just involves running rteval. > > > > One is to run it on latest mainline to make sure it doesn't crash. The > > other is to backport it to the latest -rt patch, and see how well it > > helps with latency. > > > > To get rteval: > > > > $ git clone git://git.kernel.org/pub/scm/linux/kernel/git/clrkwllms/rteval.git > > $ cd rteval > > $ git checkout origin/v2/master > > Blergh, that thing wants a gazillion things installed. Blame Clark and friends ;-) > > Can't I run something simple like rt-migrate-test with some arguments to > stress the box out? Actually what rteval does is basically 3 things. It runs cyclictest, hackbench in a loop and a kernel build in a loop. Note, rteval binds the the hackbench and kernel builds to nodes. That is, if you have 4 nodes, it will run four instances of loops of both hackbench and kernel builds in each of the nodes. This is because hackbench and access to the filesystem across nodes with a stress test can cause exorbitant latency due to cross node memory access on spin locks. I usually run cyclictest with: cyclictest -p80 -i250 -n -a -t -q -d 0 Although I think rteval does it slightly different. Like adding --numa to it. Clark, want to explain more? -- Steve
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2017-05-05 19:50 +0200 |
| Message-ID | <tDPwC-4T3-11@gated-at.bofh.it> |
| In reply to | #1636336 |
On Fri, May 05, 2017 at 08:02:38AM -0400, Steven Rostedt wrote: > Actually what rteval does is basically 3 things. It runs cyclictest, > hackbench in a loop and a kernel build in a loop. Of those, only cyclictest uses RT tasks and would end up poking at the bits you just changed. So just running cyclictest should lock up a ~120 CPU machine?
[toc] | [prev] | [next] | [standalone]
| From | Steven Rostedt <rostedt@goodmis.org> |
|---|---|
| Date | 2017-05-05 21:00 +0200 |
| Message-ID | <tDQCm-5yw-25@gated-at.bofh.it> |
| In reply to | #1636514 |
On Fri, 5 May 2017 19:39:49 +0200 Peter Zijlstra <peterz@infradead.org> wrote: > On Fri, May 05, 2017 at 08:02:38AM -0400, Steven Rostedt wrote: > > Actually what rteval does is basically 3 things. It runs cyclictest, > > hackbench in a loop and a kernel build in a loop. > > Of those, only cyclictest uses RT tasks and would end up poking at the > bits you just changed. > > So just running cyclictest should lock up a ~120 CPU machine? The other tools tend to trigger RT kernel threads as well, which causes migration. cyclictest tasks don't migrate, but they do cause other tasks to want to move around. You can try rt-migrate-test too, because I used that to trigger some bugs in previous versions. -- Steve
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2017-05-06 10:00 +0200 |
| Message-ID | <tE2Nb-5f8-5@gated-at.bofh.it> |
| In reply to | #1636645 |
On Fri, May 05, 2017 at 02:59:16PM -0400, Steven Rostedt wrote:
> On Fri, 5 May 2017 19:39:49 +0200
> Peter Zijlstra <peterz@infradead.org> wrote:
>
> > On Fri, May 05, 2017 at 08:02:38AM -0400, Steven Rostedt wrote:
> > > Actually what rteval does is basically 3 things. It runs cyclictest,
> > > hackbench in a loop and a kernel build in a loop.
> >
> > Of those, only cyclictest uses RT tasks and would end up poking at the
> > bits you just changed.
> >
> > So just running cyclictest should lock up a ~120 CPU machine?
>
> The other tools tend to trigger RT kernel threads as well, which causes
> migration. cyclictest tasks don't migrate, but they do cause other
> tasks to want to move around.
There aren't that many RT threads on a !RT kernel. All my laptop has for
example are:
$ ps -faxo pid,class,comm | grep -v TS
PID CLS COMMAND
9 FF \_ migration/0
10 FF \_ watchdog/0
11 FF \_ watchdog/1
12 FF \_ migration/1
16 FF \_ watchdog/2
17 FF \_ migration/2
21 FF \_ watchdog/3
22 FF \_ migration/3
714 FF \_ irq/48-iwlwifi
24254 FF \_ irq/47-mei_me
2444 B \_ baloo_file
> You can try rt-migrate-test too, because I used that to trigger some
> bugs in previous versions.
Just running rt-migrate-test 'works' on the 144 CPU box. But I ran it
while it was otherwise idle. That is, it ran in reasonable time and no
lockup messages were produced.
I can try and run it together with cyclictest, but over all this sounds
like we're missing a usable test-case. If I have a spare moment (lol) I
might poke at rt-migrate-test to see if I can make it more aggressive or
something.
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web