From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1755406AbdJIUXk (ORCPT <rfc822;w@1wt.eu>);
        Mon, 9 Oct 2017 16:23:40 -0400
Received: from mga06.intel.com ([134.134.136.31]:29937 "EHLO mga06.intel.com"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1754076AbdJIUXg (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
        Mon, 9 Oct 2017 16:23:36 -0400
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.42,501,1500966000"; 
   d="scan'208";a="1180296009"
Subject: Re: [PATCH] page_alloc.c: inline __rmqueue()
To: Aaron Lu <aaron.lu@intel.com>, linux-mm <linux-mm@kvack.org>,
        lkml <linux-kernel@vger.kernel.org>
References: <20171009054434.GA1798@intel.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
        Andi Kleen <ak@linux.intel.com>, Huang Ying <ying.huang@intel.com>,
        Tim Chen <tim.c.chen@linux.intel.com>, Kemi Wang <kemi.wang@intel.com>
From: Dave Hansen <dave.hansen@intel.com>
Message-ID: <3a46edcf-88f8-e4f4-8b15-3c02620308e4@intel.com>
Date: Mon, 9 Oct 2017 13:23:34 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.3.0
MIME-Version: 1.0
In-Reply-To: <20171009054434.GA1798@intel.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 10/08/2017 10:44 PM, Aaron Lu wrote:
> __rmqueue() is called by rmqueue_bulk() and rmqueue() under zone->lock
> and that lock can be heavily contended with memory intensive applications.

What does "memory intensive" mean?  I'd probably just say: "The two
__rmqueue() call sites are in very hot page allocator paths."

> Since __rmqueue() is a small function, inline it can save us some time.
> With the will-it-scale/page_fault1/process benchmark, when using nr_cpu
> processes to stress buddy:

Please include a description of the test and a link to the source.

> On a 2 sockets Intel-Skylake machine:
>       base          %change       head
>      77342            +6.3%      82203        will-it-scale.per_process_ops

What's the unit here?  That seems ridiculously low for page_fault1.
It's usually in the millions.

> On a 4 sockets Intel-Skylake machine:
>       base          %change       head
>      75746            +4.6%      79248        will-it-scale.per_process_ops

It's probably worth noting the reason that this is _less_ beneficial on
a larger system.

I'd also just put this in text rather than wasting space in tables like
that.  It took me a few minutes to figure out what the table was trying
top say.  This is one of those places where LKP output is harmful.

Why not just say:

	This patch improved the benchmark by 6.3% on a 2-socket system
	and 4.6% on a 4-socket system.

> This patch adds inline to __rmqueue().

How much text bloat does this cost?