All of lore.kernel.org
 help / color / mirror / Atom feed
From: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
To: Mel Gorman <mgorman@techsingularity.net>
Cc: mingo@kernel.org, peterz@infradead.org,
	torvalds@linux-foundation.org, tglx@linutronix.de, hpa@zytor.com,
	efault@gmx.de, linux-kernel@vger.kernel.org,
	matt@codeblueprint.co.uk, ggherdovich@suse.cz,
	mpe@ellerman.id.au
Subject: Re: [PATCH] Revert "sched/numa: Delay retrying placement for automatic NUMA balance after wake_affine()"
Date: Wed, 9 May 2018 19:04:51 -0700	[thread overview]
Message-ID: <20180510020451.GB41120@linux.vnet.ibm.com> (raw)
In-Reply-To: <20180509163115.6fnnyeg4vdm2ct4v@techsingularity.net>

* Mel Gorman <mgorman@techsingularity.net> [2018-05-09 17:31:15]:

> This reverts commit 7347fc87dfe6b7315e74310ee1243dc222c68086.
> 
> Srikar Dronamra pointed out that while the commit in question did show
> a performance improvement on ppc64, it did so at the cost of disabling
> active CPU migration by automatic NUMA balancing which was not the intent.
> The issue was that a serious flaw in the logic failed to ever active balance
> if SD_WAKE_AFFINE was disabled on scheduler domains. Even when it's enabled,
> the logic is still bizarre and against the original intent.
> 
> Investigation showed that fixing the patch in either the way he suggested,
> using the correct comparison for jiffies values or introducing a new
> numa_migrate_deferred variable in task_struct all perform similarly to a
> revert with a mix of gains and losses depending on the workload, machine
> and socket count.
> 
> The original intent of the commit was to handle a problem whereby
> wake_affine, idle balancing and automatic NUMA balancing disagree on the
> appropriate placement for a task. This was particularly true for cases where
> a single task was a massive waker of tasks but where wake_wide logic did
> not apply.  This was particularly noticeable when a futex (a barrier) woke
> all worker threads and tried pulling the wakees to the waker nodes. In that
> specific case, it could be handled by tuning MPI or openMP appropriately,
> but the behavior is not illogical and was worth attempting to fix. However,
> the approach was wrong. Given that we're at rc4 and a fix is not obvious,
> it's better to play safe, revert this commit and retry later.
> 
> Signed-off-by: Mel Gorman <mgorman@techsingularity.net>


Reviewed-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>

  reply	other threads:[~2018-05-10  2:05 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-09 16:31 [PATCH] Revert "sched/numa: Delay retrying placement for automatic NUMA balance after wake_affine()" Mel Gorman
2018-05-10  2:04 ` Srikar Dronamraju [this message]
2018-05-11  8:39   ` Mel Gorman
2018-05-12  6:42 ` [tip:sched/urgent] " tip-bot for Mel Gorman

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=20180510020451.GB41120@linux.vnet.ibm.com \
    --to=srikar@linux.vnet.ibm.com \
    --cc=efault@gmx.de \
    --cc=ggherdovich@suse.cz \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=matt@codeblueprint.co.uk \
    --cc=mgorman@techsingularity.net \
    --cc=mingo@kernel.org \
    --cc=mpe@ellerman.id.au \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    /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.