Date: Wed, 19 Oct 2016 17:21:02 -0700
From: Andrew Morton <akpm@linux-foundation.org>
To: Manfred Spraul <manfred@colorfullife.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Davidlohr Bueso <dave@stgolabs.net>,
        Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@elte.hu>,
        "H. Peter Anvin" <hpa@zytor.com>, 1vier1@web.de,
        torvalds@linux-foundation.org, xiaolong.ye@intel.com,
        felixh@informatik.uni-bremen.de
Subject: Re: [lkp] [ipc/sem.c]  5864a2fd30:  aim9.shared_memory.ops_per_sec
 -13.0%
Message-Id: <20161019172102.d04a40c4e2f9d8054aa7ec78@linux-foundation.org>
In-Reply-To: <1476851896-3590-1-git-send-email-manfred@colorfullife.com>
References: <20161017022504.GG22605@yexl-desktop>
        <1476851896-3590-1-git-send-email-manfred@colorfullife.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1819
Lines: 48

On Wed, 19 Oct 2016 06:38:14 +0200 Manfred Spraul <manfred@colorfullife.com> wrote:

> Hi,
> 
> as discussed before:
> The root cause for the performance regression is the smp_mb() that was
> added into the fast path.
> 
> I see two options:
> 1) switch to full spin_lock()/spin_unlock() for the rare codepath,
>   then the fast path doesn't need the smp_mb() anymore.
> 
> 2) confirm that no arch needs the smp_mb(), then remove it.
>   - powerpc is ok after commit
>      6262db7c088b ("powerpc/spinlock: Fix spin_unlock_wait()")
>   - arm is ok after commit
>      d86b8da04dfa ("arm64: spinlock: serialise spin_unlock_wait against concurrent lockers")
>   - for x86 is ok after commit
>      2c6100227116 ("locking/qspinlock: Fix spin_unlock_wait() some more")
>   - for the remaining SMP architectures, I don't have a status.
> 
> I would prefer the approach 1:
> The memory ordering provided by spin_lock()/spin_unlock() is clear.
> 
> Thus:
> Attached are patches for approach 1:
> 
> - Patch 1 replaces spin_unlock_wait() with spin_lock()/spin_unlock() and
>   removes all memory barriers that are then unnecessary.
> 
> - Patch 2 adds the hysteresis code: This makes the rare codepath
>   extremely rare.
>   It also corrects some wrong comments, e.g. regarding switching
>   from global lock to per-sem lock (we "must' switch, not we "can"
>   switch as written right now).
> 
> The patches passed stress-testing.
> 
> What do you think?

Are you able to confirm that the performance issues are fixed?

> My initial idea was to aim for 4.10, then we have more time to decide.

I suppose I can slip these into -next and see what the effect is upon
the Intel test results.  But a) I don't know if they test linux-next(?)
and b) I don't know where the test results are published, assuming they
are published(?).