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;
Subject: Re: [v3] hugetlbfs: Extend the definition of hugepages parameter to
 support node allocation
To:     yaozhenguo <yaozhenguo1@gmail.com>
Cc:     corbet@lwn.net, akpm@linux-foundation.org, yaozhenguo@jd.com,
        willy@infradead.org, linux-kernel@vger.kernel.org,
        linux-doc@vger.kernel.org,
        Linux Memory Management List <linux-mm@kvack.org>
References: <20210902065902.81943-1-yaozhenguo1@gmail.com>
From:   Mike Kravetz <mike.kravetz@oracle.com>
Message-ID: <d445cda6-873b-5be5-cda1-1f886bce700c@oracle.com>
Date:   Wed, 8 Sep 2021 16:13:21 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.1.1
In-Reply-To: <20210902065902.81943-1-yaozhenguo1@gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?cTdJWEx6NmhpUUdQK3VUKy9jT3QyS0ljcWVqVCtQMDhRZXlsSmU3ZEhuUE5w?=
 =?utf-8?B?VHh6eVpBaFRzS2FZUndpM1RIQVQ0clpGcTgrRXg0dkVkbE4yeUN2cUV3REY4?=
 =?utf-8?B?UVhsYWh1UnBLQ25UMmhHSnpZOE1nVGRnVldSS015bzdXb2c2ZktmTTdUNEpt?=
 =?utf-8?B?S01RY0thdkl0VGJIVHpOaStmLzRJM3cwdmpzbmFyV3lDVmczWllnTWdzVE5j?=
 =?utf-8?B?cnJFOHZYY0xtcE1Fd25XaWZJeThIODhyL3pIcTJOU2laTG90V0tWWmorYzZv?=
 =?utf-8?B?S0JVM0dWenNzQ2w1QWFVRGNtRWNtNEpEZ2gwSVZkaEVPTnMrRGlMeW1pVHhz?=
 =?utf-8?B?OG9DY0lBb2dsUElGVmJvdmVxak5TUGFpQzlpaWRGZFdVc2lKelUxZTNBdXNp?=
 =?utf-8?B?ZC8xYnoxaVAzS3ZtcFhPQ042YWFacDN2WHNxczh0Ri8rbjIraDcvZnA5aEhF?=
 =?utf-8?B?eGtQT3ZHVndHcUt3NzdHc2dUQmtwaEpYRnBuOVAwRXRKbysvdGthSzZBUDZa?=
 =?utf-8?B?S2QvQUh6NkhCVWlXVUw2REIzbG9STVkrTXdDL0kwZzY0Y2tGZ0NGZEFqQzdU?=
 =?utf-8?B?c3FyeEpkMjhwT1MybytqRjk0ajBHWVJuN0Fid09RQ0o2UUFiVzJNTm14aVdL?=
 =?utf-8?B?ZkM2Qlh4TFJCK1VOSEplT1hvcHdPbTJlMW5Hd0M2U291QngycmhuSFdUVm5Z?=
 =?utf-8?B?OXFDNG1rdWR2M1VoTDluSG4rdVNiNTFITVJQWnVpcjBjbEQ1MkY2NU1HWUNL?=
 =?utf-8?B?eUdSejFMUzlEUXh4U3J2NVJyU1JKcy9UNXFOQVUzZlI5NlgwekpJZml0Nmxi?=
 =?utf-8?B?aHZUU25hOXV0SnlZKzR1RmdxYTdiQWk5M1hYTTVMdHZrRS8vbDlZSHhINXFM?=
 =?utf-8?B?QWlZSmlER2FrUURWWTY1eXNtR01xd2wxWnY2YTRBUmFiM3VSbVpGM2FuMUpz?=
 =?utf-8?B?MU9kMmpneGw1ZTI3cVBEbEdGQ2pwSzZJcms5bzRKbTNmdkRUcUJPbUI2c1V5?=
 =?utf-8?B?eGtBRnFDbXRwMXQxY09aaWRWbGNOb3JzdE5sUFNNdWZxTSsxTzQ2YTdNeDJI?=
 =?utf-8?B?dTE0ajQrSE1qb3RXL0JVREJMdWhsU2FwRmhNWUl6NW5zNUp3OUxUWkplc2dK?=
 =?utf-8?B?a1Y4aGZLUkIyMFo0elBPRVZ4UkxhdnhHSkdMUm83RUl4UlF2R1NDb3didjJR?=
 =?utf-8?B?Rzl3cDdndWk5OXJUVXJhUzd0S3BYc1JzRGswWGdad2UwazBvOHVOdXY0RkFy?=
 =?utf-8?B?OUhTRVZkTEp3bG5lTnU1bWpZQ3k0bDFFSzI0d0F1NHcrb3Njd2JvcmYxODdo?=
 =?utf-8?B?Z1dxQXVHRDEvWnpLM3ZZS0ErYTl6UjAzN1BvQURVU0RKVitUaXJPcmdxaHBF?=
 =?utf-8?B?azB6M3RYaXlvYlB4SEZEZmw4dWZVNGRta0drdm1TdmFNcXlOZkZOYmRIRDBQ?=
 =?utf-8?B?QmJTRlZMRjdHMXFLM0xnUVIyaG5jd2pac2tYNlNBa0d0aHNaaVJ3UzhBUXdJ?=
 =?utf-8?B?ejZ5eXZqUndZVWcwNFlSVUNNbG9SSGtaNzNVdGo0VHlpR3J4Q0hNSkYwNXlv?=
 =?utf-8?B?UlVvTEYyeWhhb1JKZUY5NVRYWW5MSjhUL1l4UEZURUtGY3duU3gwd1RDNnJx?=
 =?utf-8?B?UEg2WUZMTG5acUVQSytoamNWbkNvZjJPL1pxbVI2SSszZU1uT1ZNV2xVNWta?=
 =?utf-8?B?NG1qQ3A0NS9pWkxLbmpIZFFOaW9mYzl4U0UxeS9NTEFxMjQ3bFgzKzZIRnQy?=
 =?utf-8?Q?ZG4AfsynjOhzKfctAJeJduSjgLSv78bIjGBzz6K?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: c23e951e-2331-4c18-79c6-08d9731e4150
X-MS-Exchange-CrossTenant-AuthSource: BY5PR10MB4196.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Sep 2021 23:13:23.4220
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: KrQ0F3mjg7LtGjf/lR+AaDbz8q0SPWCC7P3X8rvMzfZfwbc06LyAsjDeTuCHCE+IRILk5JfWVHF5BTU8FwqeIQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR10MB5456
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10101 signatures=668682
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 mlxlogscore=999 bulkscore=0
 suspectscore=0 mlxscore=0 phishscore=0 malwarescore=0 adultscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2109030001
 definitions=main-2109080144
X-Proofpoint-ORIG-GUID: q_CZjxJXI03qDIzNeZoIq7HrQwer__eD
X-Proofpoint-GUID: q_CZjxJXI03qDIzNeZoIq7HrQwer__eD
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 9/1/21 11:59 PM, yaozhenguo wrote:
> We can specify the number of hugepages to allocate at boot. But the
> hugepages is balanced in all nodes at present. In some scenarios,
> we only need hugepages in one node. For example: DPDK needs hugepages
> which is in the same node as NIC. if DPDK needs four hugepages of 1G
> size in node1 and system has 16 numa nodes. We must reserve 64 hugepages
> in kernel cmdline. But, only four hugepages are used. The others should
> be free after boot.If the system memory is low(for example: 64G), it will
                    ^ add space after . please
> be an impossible task. So, extend hugepages kernel parameter to specify
> node number of hugepages to allocate at boot.
> For example add following parameter:
> 
> hugepagesz=1G hugepages=0:1,1:3
> 
> It will allocate 1 hugepages in node0 and 3 hugepages in node1.
> 
> Signed-off-by: yaozhenguo <yaozhenguo1@gmail.com>

Thank you for continuing to work this!  This kernel command line parsing
code is tricky, and there is still something missing in your changes.
I also added some suggestions to change working of documentation.

> ---
> v3:	1. Skip gigantic hugepages allocation if hugetlb_cma is enabled.
> 	2. Fix wrong behavior for parameter: hugepagesz=2M hugepages=2 hugepages=5.
> 	3. Update hugetlbpage.rst.
> 	4. Fix side effects which v2 brings in.
> 	5. add cond_resched in hugetlb_hstate_alloc_pages_onenode.
> ---
>  .../admin-guide/kernel-parameters.txt         |   8 +-
>  Documentation/admin-guide/mm/hugetlbpage.rst  |  12 +-
>  include/linux/hugetlb.h                       |   1 +
>  mm/hugetlb.c                                  | 116 ++++++++++++++++--
>  4 files changed, 126 insertions(+), 11 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index bdb22006f..64a128924 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -1588,9 +1588,11 @@
>  			the number of pages of hugepagesz to be allocated.
>  			If this is the first HugeTLB parameter on the command
>  			line, it specifies the number of pages to allocate for
> -			the default huge page size.  See also
> -			Documentation/admin-guide/mm/hugetlbpage.rst.
> -			Format: <integer>
> +			the default huge page size. If using node format, It
> +			specifies numbers of hugepage in a specific node.

Perhaps rewrite as?
			the default huge page size. If using node format, the
			number of pages to allocate per-node can be specified.  

> +			See also Documentation/admin-guide/mm/hugetlbpage.rst.
> +			Format: <integer> or (node format)
> +				<node>:<numbers>[,<node>:<numbers>]

Perhaps node format should be written as?
				<node>:<integer>[,<node>:<integer>]

>  
>  	hugepagesz=
>  			[HW] The size of the HugeTLB pages.  This is used in
> diff --git a/Documentation/admin-guide/mm/hugetlbpage.rst b/Documentation/admin-guide/mm/hugetlbpage.rst
> index 8abaeb144..bc5f674ff 100644
> --- a/Documentation/admin-guide/mm/hugetlbpage.rst
> +++ b/Documentation/admin-guide/mm/hugetlbpage.rst
> @@ -128,7 +128,9 @@ hugepages
>  	implicitly specifies the number of huge pages of default size to
>  	allocate.  If the number of huge pages of default size is implicitly
>  	specified, it can not be overwritten by a hugepagesz,hugepages
> -	parameter pair for the default size.
> +	parameter pair for the default size. This parameter also has node
> +	format. It specifies numbers of hugepage in a specific node when
> +	using node format.

Perhaps rewrite as?

	parameter pair for the default size. This parameter also has a
	node format.  The node format specifies the number of huge pages
	to allocate on specific nodes.
>  
>  	For example, on an architecture with 2M default huge page size::
>  
> @@ -138,6 +140,14 @@ hugepages
>  	indicating that the hugepages=512 parameter is ignored.  If a hugepages
>  	parameter is preceded by an invalid hugepagesz parameter, it will
>  	be ignored.
> +
> +	Node format example::
> +
> +		hugepagesz=2M hugepages=0:1,1:2
> +
> +	It will allocate 1 2M hugepages in node0 and 2 2M hugepages in node1.
> +	If the node number exceeds the maximum node, the parameter will be
> +	ignored.

Perhaps rewrite as?

	It will allocate 1 2M hugepage on node0 and 2 2M hugepages on node1.
	If the node number is invalid,  the parameter will be ignored.

>  default_hugepagesz
>  	Specify the default huge page size.  This parameter can
>  	only be specified once on the command line.  default_hugepagesz can
<snip>
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
<snip>
> @@ -3580,6 +3641,10 @@ static int __init hugetlb_init(void)
>  				default_hstate_max_huge_pages;
>  		}
>  	}
> +	for (i = 0; i < nodes_weight(node_states[N_MEMORY]); i++)
> +		if (default_hugepages_in_node[i] > 0)
> +			default_hstate.max_huge_pages_node[i] =
> +					default_hugepages_in_node[i];

Logic like the above for loop also needs to be added to the routine
default_hugepagesz_setup.  See the code block starting with:

	if (default_hstate_max_huge_pages) {

The kernel command line parameters,
	hugepages=0:1,1:3 default_hugepagesz=1G

should allocate 1 1G page on node 0 and 3 1G pages on node 1.  With this
patch it will allocate 2 1G pages on node 0 and 2 1G pages on node 1.
-- 
Mike Kravetz