linux-kernel - Re: [PATCH v2 2/2] mm: migrate: Allocate the node

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 11 Nov 2021 19:20:11 +0800
From:   Baolin Wang <baolin.wang@...ux.alibaba.com>
To:     "Huang, Ying" <ying.huang@...el.com>
Cc:     akpm@...ux-foundation.org, dave.hansen@...ux.intel.com,
        ziy@...dia.com, osalvador@...e.de, shy828301@...il.com,
        zhongjiang-ali@...ux.alibaba.com, xlpang@...ux.alibaba.com,
        linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] mm: migrate: Allocate the node_demotion structure
 dynamically



On 2021/11/11 16:51, Huang, Ying wrote:
> Baolin Wang <baolin.wang@...ux.alibaba.com> writes:
> 
>> For the worst case (MAX_NUMNODES=1024), the node_demotion structure can
>> consume 32k bytes, which appears too large, so we can change to allocate
>> node_demotion dynamically at initialization time. Meanwhile allocating
>> the target demotion nodes array dynamically to select a suitable size
>> according to the MAX_NUMNODES.
>>
>> Signed-off-by: Baolin Wang <baolin.wang@...ux.alibaba.com>
>> ---
>>   mm/migrate.c | 38 +++++++++++++++++++++++++++++---------
>>   1 file changed, 29 insertions(+), 9 deletions(-)
>>
>> diff --git a/mm/migrate.c b/mm/migrate.c
>> index 126e9e6..0145b38 100644
>> --- a/mm/migrate.c
>> +++ b/mm/migrate.c
>> @@ -1152,10 +1152,11 @@ static int __unmap_and_move(struct page *page, struct page *newpage,
>>   #define DEFAULT_DEMOTION_TARGET_NODES 15
>>   struct demotion_nodes {
>>   	unsigned short nr;
>> -	short nodes[DEFAULT_DEMOTION_TARGET_NODES];
>> +	short nodes[];
>>   };
>>   
>> -static struct demotion_nodes node_demotion[MAX_NUMNODES] __read_mostly;
>> +static struct demotion_nodes *node_demotion[MAX_NUMNODES] __read_mostly;
>> +static unsigned short target_nodes_max;
> 
> I think we can use something as below,
> 
>    #if MAX_NUMNODES < DEFAULT_DEMOTION_TARGET_NODES
>    #define DEMOTION_TARGET_NODES   (MAX_NUMNODES - 1)
>    #else
>    #define DEMOTION_TARGET_NODES   DEFAULT_DEMOTION_TARGET_NODES
>    #endif

Yes, looks better.

> 
>    static struct demotion_nodes *node_demotion;
> 
> Then we can allocate nr_node_ids * sizeof(struct demotion_nodes) for node_demotion.

Yeah, this is simple. The reason I want to declare the structure like 
"struct demotion_nodes *node_demotion[MAX_NUMNODES]" is that, we can 
validate the non-possible nodes which are invalid to demote memory, and 
in case the node_demotion[nid] is failed to be allocated which can be 
validated, though this is unlikely. However, I agree with you to keep 
things simple now and can be merged into patch 1. Will do in next 
version. Thanks.