From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755501AbcGHPMY (ORCPT ); Fri, 8 Jul 2016 11:12:24 -0400 Received: from smtprelay0063.hostedemail.com ([216.40.44.63]:51179 "EHLO smtprelay.hostedemail.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1754863AbcGHPMS (ORCPT ); Fri, 8 Jul 2016 11:12:18 -0400 X-Session-Marker: 726F737465647440676F6F646D69732E6F7267 X-Spam-Summary: 2,0,0,,d41d8cd98f00b204,rostedt@goodmis.org,:::::::::::,RULES_HIT:41:355:379:541:599:800:960:973:988:989:1260:1277:1311:1313:1314:1345:1359:1437:1515:1516:1518:1534:1543:1593:1594:1605:1711:1730:1747:1777:1792:2393:2553:2559:2562:2693:2895:2898:3138:3139:3140:3141:3142:3622:3698:3865:3866:3867:3868:3871:3872:3873:3874:4250:4321:4362:5007:6119:6261:7875:7903:8660:10004:10400:10848:10967:11026:11232:11473:11658:11914:12043:12296:12438:12517:12519:12555:12663:12740:13148:13161:13229:13230:13439:14096:14097:14181:14659:14721:21080:21324:30012:30036:30045:30054:30056:30070:30090:30091,0,RBL:none,CacheIP:none,Bayesian:0.5,0.5,0.5,Netcheck:none,DomainCache:0,MSF:not bulk,SPF:fn,MSBL:0,DNSBL:none,Custom_rules:0:0:0,LFtime:3,LUA_SUMMARY:none X-HE-Tag: dirt16_6b019512cc047 X-Filterd-Recvd-Size: 4953 Date: Fri, 8 Jul 2016 11:12:14 -0400 From: Steven Rostedt To: Peter Zijlstra Cc: LKML , Ingo Molnar , Thomas Gleixner , Clark Williams , Andrew Morton Subject: Re: [PATCH] sched/rt: Avoid sending an IPI to a CPU already doing a push Message-ID: <20160708111214.25ef91e6@gandalf.local.home> In-Reply-To: <20160708145153.GX30921@twins.programming.kicks-ass.net> References: <20160624112613.4e98c657@gandalf.local.home> <20160708145153.GX30921@twins.programming.kicks-ass.net> X-Mailer: Claws Mail 3.13.2 (GTK+ 2.24.30; 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: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 8 Jul 2016 16:51:53 +0200 Peter Zijlstra wrote: > On Fri, Jun 24, 2016 at 11:26:13AM -0400, Steven Rostedt wrote: > > The IPI code now needs to call push_tasks() instead of just push_task() as > > it will not be receiving an IPI for each CPU that is requesting a PULL. > > My brain just skidded on that, can you try again with a few more words? Sure. One of those cases where I lose track of knowing what I know, and don't describe enough. The original logic expected each pull request to have its own IPI sent out. Thus, a CPU would only do a single push, knowing that only one CPU opened up (lowered its priority). Although, it still sends another IPI out to the next CPU regardless of a push if a higher task is waiting. This may be solved with a cpupri of next highest waiters. But that's a bit complex for now. Anyway, the old code would have multiple CPUs sending IPIs out to CPUs. One for each CPU that lowers its prio. The new code checks if a CPU is already processing an IPI (via the ipi counter) and will skip sending to that CPU. That means that if two CPUs lower its priority, and only one CPU has multiple RT tasks waiting, it needs to do multiple push_task() calls (which push_tasks() does). Better? I can update the change log. > > > > diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c > > index d5690b722691..165bcfdbd94b 100644 > > --- a/kernel/sched/rt.c > > +++ b/kernel/sched/rt.c > > @@ -100,6 +100,7 @@ void init_rt_rq(struct rt_rq *rt_rq) > > rt_rq->push_flags = 0; > > rt_rq->push_cpu = nr_cpu_ids; > > raw_spin_lock_init(&rt_rq->push_lock); > > + atomic_set(&rt_rq->ipi_count, 0); > > init_irq_work(&rt_rq->push_work, push_irq_work_func); > > #endif > > #endif /* CONFIG_SMP */ > > @@ -1917,6 +1918,10 @@ static int find_next_push_cpu(struct rq *rq) > > break; > > next_rq = cpu_rq(cpu); > > > > + /* If pushing was already started, ignore */ > > + if (atomic_read(&next_rq->rt.ipi_count)) > > + continue; > > + > > /* Make sure the next rq can push to this rq */ > > if (next_rq->rt.highest_prio.next < rq->rt.highest_prio.curr) > > break; > > @@ -1955,6 +1960,7 @@ static void tell_cpu_to_push(struct rq *rq) > > return; > > > > rq->rt.push_flags = RT_PUSH_IPI_EXECUTING; > > + atomic_inc(&cpu_rq(cpu)->rt.ipi_count); > > > > irq_work_queue_on(&rq->rt.push_work, cpu); > > } > > @@ -1974,11 +1980,12 @@ static void try_to_push_tasks(void *arg) > > > > rq = cpu_rq(this_cpu); > > src_rq = rq_of_rt_rq(rt_rq); > > + WARN_ON_ONCE(!atomic_read(&rq->rt.ipi_count)); > > > > again: > > if (has_pushable_tasks(rq)) { > > raw_spin_lock(&rq->lock); > > - push_rt_task(rq); > > + push_rt_tasks(rq); > > Maybe as a comment around here? Sure. > > > raw_spin_unlock(&rq->lock); > > } > > > > @@ -2000,7 +2007,7 @@ again: > > raw_spin_unlock(&rt_rq->push_lock); > > > > if (cpu >= nr_cpu_ids) > > - return; > > + goto out; > > > > /* > > * It is possible that a restart caused this CPU to be > > @@ -2011,7 +2018,10 @@ again: > > goto again; > > > > /* Try the next RT overloaded CPU */ > > + atomic_inc(&cpu_rq(cpu)->rt.ipi_count); > > irq_work_queue_on(&rt_rq->push_work, cpu); > > +out: > > + atomic_dec(&rq->rt.ipi_count); > > } > > I have a vague feeling we're duplicating state, but I can't seem to spot > it, maybe I'm wrong. The above is can be deceiving because there's two atomics close, but subtly different. The inc updates curr_rq(cpu) the dec updates just "rq". > > Looks about right, but could use a comment, this stuff is getting rather > subtle. Agreed. I'll add that and update it. Again, this patch added to 4.6-rt makes it pass rteval with a 200us max on that 80 CPU box. Thanks for looking into this. -- Steve