Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: nHBUuQEVn+REe69ix/w6Y+RgMnAPAbaKWe9WX8iCsDzGziJLFhKKv3iEfSlil+0nxkxfWJCLiZ
 UBw4J3U9S0qA==
IronPort-SDR: ecSZn7beoVqP/TqvZcl1OzTC3kykreZ5ip1xx5+wWc0Jw2P8qE+3tzgCOSM0m0R26e/CQZhdM9
 SEdPvw7ke0+g==
From:   "Huang\, Ying" <ying.huang@intel.com>
To:     Phil Auld <pauld@redhat.com>
Cc:     Peter Zijlstra <peterz@infradead.org>, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org,
        Andrew Morton <akpm@linux-foundation.org>,
        Ingo Molnar <mingo@redhat.com>, Mel Gorman <mgorman@suse.de>,
        Johannes Weiner <hannes@cmpxchg.org>,
        "Matthew Wilcox \(Oracle\)" <willy@infradead.org>,
        Dave Hansen <dave.hansen@intel.com>,
        Andi Kleen <ak@linux.intel.com>,
        Michal Hocko <mhocko@suse.com>,
        David Rientjes <rientjes@google.com>
Subject: Re: [RFC -V2] autonuma: Migrate on fault among multiple bound nodes
References: <20200922065401.376348-1-ying.huang@intel.com>
        <20200922125049.GA10420@lorien.usersys.redhat.com>
Date:   Wed, 23 Sep 2020 13:44:12 +0800
In-Reply-To: <20200922125049.GA10420@lorien.usersys.redhat.com> (Phil Auld's
        message of "Tue, 22 Sep 2020 08:51:53 -0400")
Message-ID: <87o8lxoxn7.fsf@yhuang-dev.intel.com>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain; charset=ascii
Precedence: bulk

Phil Auld <pauld@redhat.com> writes:

> Hi,
>
> On Tue, Sep 22, 2020 at 02:54:01PM +0800 Huang Ying wrote:
>> Now, AutoNUMA can only optimize the page placement among the NUMA nodes if the
>> default memory policy is used.  Because the memory policy specified explicitly
>> should take precedence.  But this seems too strict in some situations.  For
>> example, on a system with 4 NUMA nodes, if the memory of an application is bound
>> to the node 0 and 1, AutoNUMA can potentially migrate the pages between the node
>> 0 and 1 to reduce cross-node accessing without breaking the explicit memory
>> binding policy.
>> 
>> So in this patch, if mbind(.mode=MPOL_BIND, .flags=MPOL_MF_LAZY) is used to bind
>> the memory of the application to multiple nodes, and in the hint page fault
>> handler both the faulting page node and the accessing node are in the policy
>> nodemask, the page will be tried to be migrated to the accessing node to reduce
>> the cross-node accessing.
>>
>
> Do you have any performance numbers that show the effects of this on
> a workload?

I have done some simple test to confirm that NUMA balancing works in the
target configuration.

As for performance numbers, it's exactly same as that of the original
NUMA balancing in a different configuration.  Between without memory
binding and with memory bound to all NUMA nodes.

>
>> [Peter Zijlstra: provided the simplified implementation method.]
>> 
>> Questions:
>> 
>> Sysctl knob kernel.numa_balancing can enable/disable AutoNUMA optimizing
>> globally.  But for the memory areas that are bound to multiple NUMA nodes, even
>> if the AutoNUMA is enabled globally via the sysctl knob, we still need to enable
>> AutoNUMA again with a special flag.  Why not just optimize the page placement if
>> possible as long as AutoNUMA is enabled globally?  The interface would look
>> simpler with that.
>
>
> I agree. I think it should try to do this if globally enabled.

Thanks!

>> 
>> Signed-off-by: "Huang, Ying" <ying.huang@intel.com>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: Ingo Molnar <mingo@redhat.com>
>> Cc: Mel Gorman <mgorman@suse.de>
>> Cc: Rik van Riel <riel@redhat.com>
>> Cc: Johannes Weiner <hannes@cmpxchg.org>
>> Cc: "Matthew Wilcox (Oracle)" <willy@infradead.org>
>> Cc: Dave Hansen <dave.hansen@intel.com>
>> Cc: Andi Kleen <ak@linux.intel.com>
>> Cc: Michal Hocko <mhocko@suse.com>
>> Cc: David Rientjes <rientjes@google.com>
>> ---
>>  mm/mempolicy.c | 17 +++++++++++------
>>  1 file changed, 11 insertions(+), 6 deletions(-)
>> 
>> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
>> index eddbe4e56c73..273969204732 100644
>> --- a/mm/mempolicy.c
>> +++ b/mm/mempolicy.c
>> @@ -2494,15 +2494,19 @@ int mpol_misplaced(struct page *page, struct vm_area_struct *vma, unsigned long
>>  		break;
>>  
>>  	case MPOL_BIND:
>> -
>>  		/*
>> -		 * allows binding to multiple nodes.
>> -		 * use current page if in policy nodemask,
>> -		 * else select nearest allowed node, if any.
>> -		 * If no allowed nodes, use current [!misplaced].
>> +		 * Allows binding to multiple nodes.  If both current and
>> +		 * accessing nodes are in policy nodemask, migrate to
>> +		 * accessing node to optimize page placement. Otherwise,
>> +		 * use current page if in policy nodemask, else select
>> +		 * nearest allowed node, if any.  If no allowed nodes, use
>> +		 * current [!misplaced].
>>  		 */
>> -		if (node_isset(curnid, pol->v.nodes))
>> +		if (node_isset(curnid, pol->v.nodes)) {
>> +			if (node_isset(thisnid, pol->v.nodes))
>> +				goto moron;
>
> Nice label :)

OK.  Because quite some people pay attention to this.  I will rename all
"moron" to "mopron" as suggested by Matthew.  Although MPOL_F_MORON is
defined in include/uapi/linux/mempolicy.h, it is explicitly marked as
internal flags.

Best Regards,
Huang, Ying

>>  			goto out;
>> +		}
>>  		z = first_zones_zonelist(
>>  				node_zonelist(numa_node_id(), GFP_HIGHUSER),
>>  				gfp_zone(GFP_HIGHUSER),
>> @@ -2516,6 +2520,7 @@ int mpol_misplaced(struct page *page, struct vm_area_struct *vma, unsigned long
>>  
>>  	/* Migrate the page towards the node whose CPU is referencing it */
>>  	if (pol->flags & MPOL_F_MORON) {
>> +moron:
>>  		polnid = thisnid;
>>  
>>  		if (!should_numa_migrate_memory(current, page, curnid, thiscpu))
>> -- 
>> 2.28.0
>> 
>
>
> Cheers,
> Phil