From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751217AbdIKIE1 convert rfc822-to-8bit (ORCPT ); Mon, 11 Sep 2017 04:04:27 -0400 Received: from mout.gmx.net ([212.227.15.15]:62373 "EHLO mout.gmx.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750994AbdIKIEY (ORCPT ); Mon, 11 Sep 2017 04:04:24 -0400 Message-ID: <1505117005.6062.68.camel@gmx.de> Subject: Re: [lkp-robot] [sched/fair] 6d46bd3d97: netperf.Throughput_tps -11.3% regression From: Mike Galbraith To: Joel Fernandes Cc: kernel test robot , LKML , Peter Zijlstra , Josef Bacik , Juri Lelli , Brendan Jackman , Dietmar Eggemann , Matt Fleming , Rik van Riel , Ingo Molnar , lkp@01.org Date: Mon, 11 Sep 2017 10:03:25 +0200 In-Reply-To: References: <20170827010226.19703-1-joelaf@google.com> <20170910134021.GB29265@yexl-desktop> <1505098524.18240.42.camel@gmx.de> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.20.5 Mime-Version: 1.0 Content-Transfer-Encoding: 8BIT X-Provags-ID: V03:K0:0r8S2ZaB++t+IkwV5B1GJ/+OeVt/gaZghqJ0ijscxfRxJ1ZEqXY AgNnkueupETIKjn5SC2Q323YuC2wHH90tO1mXRPbiVr0BXjJZyN68plPa9qJo6l8xlwpDz4 cxv/3aVPfNvJxDLmSwcApNk2zMjuNqT4oNMTUxIQYTVWDEr6VIwWSI26AK21n4rgsXi9nz1 eahab6MHxwDaszYy6tYQA== X-UI-Out-Filterresults: notjunk:1;V01:K0:MAMfeyhWmGk=:vDteyp/YCqaqHJXOPBPxIq vO/B/X4E6W1EsN2Mo1Wd+ma0f0LUrzRfO6Tn2TRH1EOXAn4IDqPli7SkPqB5+BX+xZstAJinX U/kxtSzGxEHaa9bn384X2pH2JeusoZx7n8vT/pJdfX9SmkpPvlPY2qTx1aZCUY1D9CHYABhTm 7wf9hSLqprVrjxamA5LuQppZBzA82+AZNUyf9r/0pBIXKxMKY49Mz2+I14L+kbaplEuGGyzHp lQ3E1ZC/H2eWxu1VFtI7Z5jrJKaYhjrDZM4dOUJ+JsAmRJ7YNflhAbOj8FtkI/TNOkJaMO3HF pZ+tKNceejuqyH541LmUzAy1AiEEOIFZvA+Ew+EC8Iu2u+YL+IgKjYNhD5CSJKOhQYHQnJrXf piHa6084FKHOBN2iiQIqeo8diUFrjvFc06MB+TmPxgHnmPXt3P2xfG/KTmE6lSPIy0tF7NV6l bSs86RWuB68X0jyvYNLSgA2SS10WOp0smGjw4j9AreZVuoQlYtQJ2kgp9wQJb2jqT0gc9ir4h T46ws1k4PX0mri8Msc+hY7Sfsbq4sRm6jFBAPoPlc/StYdE3Fh/gvhN9vqR4zQl+9+p0b7CwL OBl3OV2UYcCsoQjYTsYk+z1MVQQgkQ9F2ZSPwjF4o0RhPstd0px4QMdzHnCgFgvm6RMffAePU yaAngaPb5I++f3CVjXqZXi+mn8xzks6z/Yr2rxPLcwb1F58Ak15SgjecyBBfcGYLuLkJgiChO 7ZwyrhB6+Mw3q8yXIQLM1Oe/avh0eZtQwSTvvwYegLu3jtYO4WmqaMnmbpyGmbgY+P/RnEE/V A0kWB3eMhpfcvF0+Mfc6w6xaCgTNXWXIiqq9GHTfTqNTmd0rnI= Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, 2017-09-10 at 23:32 -0700, Joel Fernandes wrote: > Hi Mike, > Thanks a lot for sharing the history of this. > > On Sun, Sep 10, 2017 at 7:55 PM, Mike Galbraith wrote: > > On Sun, 2017-09-10 at 09:53 -0700, Joel Fernandes wrote: > >> > >> Anyone know what in the netperf test triggers use of the sync flag? > > > > homer:..kernel/linux-master # git grep wake_up_interruptible_sync_poll net > > net/core/sock.c: wake_up_interruptible_sync_poll(&wq->wait, POLLIN | POLLPRI | > > net/core/sock.c: wake_up_interruptible_sync_poll(&wq->wait, POLLOUT | > > net/sctp/socket.c: wake_up_interruptible_sync_poll(&wq->wait, POLLIN | > > net/smc/smc_rx.c: wake_up_interruptible_sync_poll(&wq->wait, POLLIN | POLLPRI | > > net/tipc/socket.c: wake_up_interruptible_sync_poll(&wq->wait, POLLOUT | > > net/tipc/socket.c: wake_up_interruptible_sync_poll(&wq->wait, POLLIN | > > net/unix/af_unix.c: wake_up_interruptible_sync_poll(&wq->wait, > > net/unix/af_unix.c: wake_up_interruptible_sync_poll(&u->peer_wait, > > > > The same as metric tons of other stuff. > > > > Once upon a time, we had avg_overlap to help decide whether to wake > > core affine or not, on top of the wake_affine() imbalance constraint, > > but instrumentation showed it to be too error prone, so it had to die. > > These days, an affine wakeup generally means cache affine, and the > > sync hint gives you a wee bit more chance of migration near to tasty > > hot data being approved. > > > > The sync hint was born back in the bad old days, when communicating > > tasks not sharing L2 may as well have been talking over two tin cans > > and a limp string. These days, things are oodles better, but truly > > synchronous stuff could still benefit from core affinity (up to hugely > > for very fast/light stuff) if it weren't for all the caveats that can > > lead to tossing concurrency opportunities out the window. > > Cool, thanks. For this test I suspect its the other way? I think the > reason why regresses is that the 'nr_running < 2' check is too weak of > a check to prevent sync in all bad situations ('bad' being pulling a > task to a crowded CPU). Could we maybe be having a situation for this > test where if the blocked load a CPU is high (many tasks recently were > running on it and went to sleep), then the nr_running < 2 is a false > positive and in such a scenario we listened to the sync flag when we > shouldn't have? nr_running has ~little to do with synchronous baton passing, or rather with overlap being too small to overcome costs (L2 miss/cstate..).  If you run TCP_RR, pipe-test or ilk (tiny ball ping-pong), you should see gorgeous numbers, and may find yourself thinking sync is a wonder elixir for network-land, but 'taint necessarily so in the real world. The only way to win the "Rob Peter to pay Paul" game is through the use of knowledge, acquisition of which is expensive.  Try resurrecting the cheap avg_overlap, you'll love it.. for a little while. > To make the load check more meaningful, I am thinking if using > wake_affine()'s balance check is a better thing to do than the > 'nr_running < 2' check I used in this patch. Then again, since commit > 3fed382b46baac ("sched/numa: Implement NUMA node level wake_affine()", > wake_affine() doesn't do balance check for CPUs within a socket so > probably bringing back something like the *old* wake_affine that > checked load between different CPUs within a socket is needed to avoid > a potentially disastrous sync decision? The commit I refer to was > added with the reason that select_idle_sibling was selecting cores > anywhere within a socket, but with my patch we're more specifically > selecting the waker's CPU on passing the sync flag. Could you share > your thoughts about this? Mucking about with wake_affine() in core affinity context would be a step backward methinks.  The sync hint has not meant CPU affine for quite some time now. If you can come up with a sync hint modification that is win/win, super, but I suspect you'll find it a lot easier to create a new 'really really' flag or make your governor more responsive. -Mike