Path: csiph.com!news.mixmin.net!aioe.org!bofh.it!news.nic.it!robomod From: Vaidyanathan Srinivasan Newsgroups: linux.kernel Subject: Re: [patch 10/15] sched/migration: Move calc_load_migrate() into CPU_DYING Date: Tue, 12 Jul 2016 21:00:01 +0200 Message-ID: References: X-Ibm-Helo: d23dlp01.au.ibm.com X-Ibm-Mailfrom: svaidy@linux.vnet.ibm.com X-Ibm-Rcptto: linux-kernel@vger.kernel.org Reply-To: svaidy@linux.vnet.ibm.com MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline User-Agent: Mutt/1.5.24 (2015-08-30) X-Tm-As-Mml: disable X-Content-Scanned: Fidelis XPS MAILER X-Cbid: 16071218-0012-0000-0000-000001A9CAD4 X-Ibm-Av-Detection: SAVI=unused REMOTE=unused XFE=unused X-Cbparentid: 16071218-0013-0000-0000-000005909DB0 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2016-07-12_09:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1604210000 definitions=main-1607120167 Sender: robomod@news.nic.it List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Approved: robomod@news.nic.it Lines: 103 Organization: linux.* mail to news gateway X-Original-Cc: Anton Blanchard , LKML , Peter Zijlstra , Ingo Molnar , rt@linutronix.de, Michael Ellerman , shreyas@linux.vnet.ibm.com X-Original-Date: Wed, 13 Jul 2016 00:19:39 +0530 X-Original-Message-ID: <20160712184939.GC13229@drishya.in.ibm.com> X-Original-References: <20160310115406.940706476@linutronix.de> <20160310120025.328739226@linutronix.de> <20160712143745.430733e7@kryten> X-Original-Sender: linux-kernel-owner@vger.kernel.org Xref: csiph.com linux.kernel:1441658 * Thomas Gleixner [2016-07-12 18:33:56]: > Anton, > > On Tue, 12 Jul 2016, Anton Blanchard wrote: > > > It really does not matter when we fold the load for the outgoing cpu. > > > It's almost dead anyway, so there is no harm if we fail to fold the > > > few microseconds which are required for going fully away. > > > > We are seeing the load average shoot up when hot unplugging CPUs (+1 > > for every CPU we offline) on ppc64. This reproduces on bare metal as > > well as inside a KVM guest. A bisect points at this commit. > > > > As an example, a completely idle box with 128 CPUS and 112 hot > > unplugged: > > > > # uptime > > 04:35:30 up 1:23, 2 users, load average: 112.43, 122.94, 125.54 > > Yes, it's an off by one as we now call that from the task which is tearing > down the cpu. Does the patch below fix it? Hi Thomas, Yes this patch fixes the issue. I was able to recreate the problem and also verify with this patch on 4.7.0-rc7. > > Thanks, > > tglx > > 8<---------------------- > > Subject: sched/migration: Correct off by one in load migration > From: Thomas Gleixner > > The move of calc_load_migrate() from CPU_DEAD to CPU_DYING did not take into > account that the function is now called from a thread running on the outgoing > CPU. As a result a cpu unplug leakes a load of 1 into the global load > accounting mechanism. > > Fix it by adjusting for the currently running thread which calls > calc_load_migrate(). > > Fixes: e9cd8fa4fcfd: "sched/migration: Move calc_load_migrate() into CPU_DYING" > Reported-by: Anton Blanchard Tested-by: Vaidyanathan Srinivasan > Signed-off-by: Thomas Gleixner > > --- > diff --git a/kernel/sched/core.c b/kernel/sched/core.c > index 51d7105f529a..97ee9ac7e97c 100644 > --- a/kernel/sched/core.c > +++ b/kernel/sched/core.c > @@ -5394,13 +5394,15 @@ void idle_task_exit(void) > /* > * Since this CPU is going 'away' for a while, fold any nr_active delta > * we might have. Assumes we're called after migrate_tasks() so that the > - * nr_active count is stable. > + * nr_active count is stable. We need to take the teardown thread which > + * is calling this into account, so we hand in adjust = 1 to the load > + * calculation. > * > * Also see the comment "Global load-average calculations". > */ > static void calc_load_migrate(struct rq *rq) > { > - long delta = calc_load_fold_active(rq); > + long delta = calc_load_fold_active(rq, 1); > if (delta) > atomic_long_add(delta, &calc_load_tasks); > } > diff --git a/kernel/sched/loadavg.c b/kernel/sched/loadavg.c > index b0b93fd33af9..a2d6eb71f06b 100644 > --- a/kernel/sched/loadavg.c > +++ b/kernel/sched/loadavg.c > @@ -78,11 +78,11 @@ void get_avenrun(unsigned long *loads, unsigned long offset, int shift) > loads[2] = (avenrun[2] + offset) << shift; > } > > -long calc_load_fold_active(struct rq *this_rq) > +long calc_load_fold_active(struct rq *this_rq, long adjust) > { > long nr_active, delta = 0; > > - nr_active = this_rq->nr_running; > + nr_active = this_rq->nr_running - adjust; > nr_active += (long)this_rq->nr_uninterruptible; if (nr_active != this_rq->calc_load_active) { delta = nr_active - this_rq->calc_load_active; this_rq->calc_load_active = nr_active; } return delta; Does the above calculation hold good even if we send adjust=1 and bump down nr_active? Tested ok though :) --Vaidy