All of lore.kernel.org
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: Peter Zijlstra <peterz@infradead.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@kernel.org>,
	Clark Williams <williams@redhat.com>,
	Daniel Bristot de Oliveira <bristot@redhat.com>,
	John Kacur <jkacur@redhat.com>, Scott Wood <swood@redhat.com>
Subject: Re: [PATCH tip/sched/core v2] sched/rt: Simplify the IPI rt balancing logic
Date: Thu, 4 May 2017 15:03:55 -0400	[thread overview]
Message-ID: <20170504150355.0524c8e8@gandalf.local.home> (raw)
In-Reply-To: <20170504184200.3gjfciwqulgwpeln@hirez.programming.kicks-ass.net>

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

  reply	other threads:[~2017-05-04 19:04 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-24 15:47 [PATCH tip/sched/core v2] sched/rt: Simplify the IPI rt balancing logic Steven Rostedt
2017-05-04 14:41 ` Peter Zijlstra
2017-05-04 15:01   ` Steven Rostedt
2017-05-04 15:32 ` Peter Zijlstra
2017-05-04 17:25   ` Steven Rostedt
2017-05-04 18:42     ` Peter Zijlstra
2017-05-04 19:03       ` Steven Rostedt [this message]
2017-05-05  4:26         ` Mike Galbraith
2017-05-05  5:16           ` Mike Galbraith
2017-05-05 11:05         ` Peter Zijlstra
2017-05-05 12:02           ` Steven Rostedt
2017-05-05 17:39             ` Peter Zijlstra
2017-05-05 18:59               ` Steven Rostedt
2017-05-06  7:52                 ` Peter Zijlstra
2017-05-08  1:25 ` [lkp-robot] [sched/rt] bebe5811a9: hackbench.throughput -6% regression kernel test robot
2017-10-10 11:02 ` [tip:sched/core] sched/rt: Simplify the IPI based RT balancing logic tip-bot for Steven Rostedt (Red Hat)
2018-01-19  9:23   ` Pavan Kondeti
2018-01-19 15:03     ` Steven Rostedt
2018-01-19 15:44       ` Pavan Kondeti
2018-01-19 15:53         ` Steven Rostedt
2018-01-19 17:46       ` Pavan Kondeti
2018-01-19 18:11         ` Steven Rostedt
2018-01-19 18:12           ` Steven Rostedt
2018-01-19 18:57             ` Pavan Kondeti
2018-01-19 19:51               ` Steven Rostedt
2018-01-20  4:56                 ` Pavan Kondeti
2018-01-19 18:54           ` Pavan Kondeti
2018-02-06 11:54     ` [tip:sched/urgent] sched/rt: Use container_of() to get root domain in rto_push_irq_work_func() tip-bot for Steven Rostedt (VMware)
2018-02-07  4:14       ` Steven Rostedt
2018-02-06 11:54     ` [tip:sched/urgent] sched/rt: Up the root domain ref count when passing it around via IPIs tip-bot for Steven Rostedt (VMware)
2018-02-07  4:15       ` Steven Rostedt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170504150355.0524c8e8@gandalf.local.home \
    --to=rostedt@goodmis.org \
    --cc=bristot@redhat.com \
    --cc=jkacur@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=swood@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=williams@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.