From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1752785AbdEDTEH (ORCPT <rfc822;w@1wt.eu>);
        Thu, 4 May 2017 15:04:07 -0400
Received: from mail.kernel.org ([198.145.29.136]:37202 "EHLO mail.kernel.org"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1756253AbdEDTEA (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
        Thu, 4 May 2017 15:04:00 -0400
Date: Thu, 4 May 2017 15:03:55 -0400
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
Message-ID: <20170504150355.0524c8e8@gandalf.local.home>
In-Reply-To: <20170504184200.3gjfciwqulgwpeln@hirez.programming.kicks-ass.net>
References: <20170424114732.1aac6dc4@gandalf.local.home>
        <20170504153256.fbmglqe2zjo6ika2@hirez.programming.kicks-ass.net>
        <20170504132538.157f8ca7@gandalf.local.home>
        <20170504184200.3gjfciwqulgwpeln@hirez.programming.kicks-ass.net>
X-Mailer: Claws Mail 3.14.0 (GTK+ 2.24.31; x86_64-pc-linux-gnu)
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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