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: [PATCH v3 1/5] hugetlb: add demote hugetlb page sysfs interfaces
To:     Oscar Salvador <osalvador@suse.de>
Cc:     linux-mm@kvack.org, linux-kernel@vger.kernel.org,
        David Hildenbrand <david@redhat.com>,
        Michal Hocko <mhocko@suse.com>, Zi Yan <ziy@nvidia.com>,
        Muchun Song <songmuchun@bytedance.com>,
        Naoya Horiguchi <naoya.horiguchi@linux.dev>,
        David Rientjes <rientjes@google.com>,
        "Aneesh Kumar K . V" <aneesh.kumar@linux.ibm.com>,
        Andrew Morton <akpm@linux-foundation.org>
References: <20211001175210.45968-1-mike.kravetz@oracle.com>
 <20211001175210.45968-2-mike.kravetz@oracle.com>
 <YVr7C8ek1g9hKz4C@localhost.localdomain>
From:   Mike Kravetz <mike.kravetz@oracle.com>
Message-ID: <81c2e7d0-cbdf-56fd-2a59-34c180d5a979@oracle.com>
Date:   Mon, 4 Oct 2021 11:27:36 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.1.1
In-Reply-To: <YVr7C8ek1g9hKz4C@localhost.localdomain>
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?SGhjUGhNR2RTQmdNYk56V0t5bjBDOEYyTm83WC9XbWY0MCtrMkhBMUFSV0Z6?=
 =?utf-8?B?T2JMVWgwbDBWb0MwWFc3Ri9sckpxd2hMaWZlNThlUitFSHU0QmpqTjM0OSsz?=
 =?utf-8?B?QmRDT0UzeldxeWxmWS9ZYUZicDJUSEppRUVNa1JwajV5bWxTdml0aW5YdXp5?=
 =?utf-8?B?K2Q3NE1mSW9OSm9ydnhpenBxQTM1NW1Lc1paZkV4UGNiSEJ3RU9Ja3dFbmd5?=
 =?utf-8?B?QXNXS2lqY3pTdGwwTThoKzNLa3VSYkxlTktJOWZrZFFuZ1ZJYlFvNmYvZnZD?=
 =?utf-8?B?V3hKbFB4bTk5WG1CZlZPMG1kTnhxZG1xZmorY3JiY0JtUndzdEpqcWk3NDA2?=
 =?utf-8?B?RDlRZ3JBc2FoZWcza3o5dDlramV1eHFENGxHTVZXeDZMVHAxblQrTXJQcU1M?=
 =?utf-8?B?NmJuM2UzWnhxQnFEQS9zYng3bXdMZlpMMWg4bFIrbzB6T05yQU1iTHdVeUV5?=
 =?utf-8?B?Y0JoYjdMekdZNjVidGtMa1BtOVByUzZZMElsRHU2R1lxc2ozdHdzeTlMbTVt?=
 =?utf-8?B?MTRMMzFIczRraXRlbjExdjAvek1YbEU4KzQxaUNSTk5XYUJreFBFTXBlRnlS?=
 =?utf-8?B?TE5kVWZWQUhlVzNQVFR0OE1BL1Y0M09TQytRbWFVUm51MWVSeXVyUk9DVFgv?=
 =?utf-8?B?U3JhOXgrNXVjdDVEMW02d0MwVXRkQk5oaWVOOHhRVm5Rd2d4TnZsckh4Y1JD?=
 =?utf-8?B?NXNsbEJCMVFaTWx1eVBiWERUdkxJV2FCVmI4bkhpVVZWRFM4TUJNRXZ3NWd6?=
 =?utf-8?B?US9LRWE3cTkxTkM0VUY4TFV6M2FuZlZac0Jlc2dZT29WRmZSd3BhYXVEdndR?=
 =?utf-8?B?bDVBNGVvRHV4OEVKSTliSEJMM05xUTVreVlrcjkzdkx3ZXJ4d2ZBWFlHU0Rm?=
 =?utf-8?B?c0NyR3FXaGsrVFUrdGZZQ1drajh0UTBSUHZ3UDZ4MGNpZkZoV1RrREV4eUhO?=
 =?utf-8?B?YXlDeE9YNHo2UDJ4cUQ4cWROWGFOWEhXNW9WR2dhRVBqaHJra3VwZWszQTJX?=
 =?utf-8?B?b1dKdk1aMVVoR1lyY0JDdFROYlhmb1BxRnVGcEwzL2ZVcHBBem13azN4ZVNl?=
 =?utf-8?B?bjB6NDFOMXRZUTVRQkxRRVMvSW16QVQxY01zUW56cGovT0xTaFFBNGpsUXFq?=
 =?utf-8?B?SWM3VDRnTzBUQjd3Rit6R25vUVN4RXpCbzhrMHoyRHBNTE9oZEV6c1hmcllk?=
 =?utf-8?B?SFpGSVpLV1pBN29mU0E0c3h6MFJIUkhMR1pzcWl6RTJEcUlNYWtUd2xETWpv?=
 =?utf-8?B?UWkxVjE0azZpVm1qalFjb0xNczFiTmdncVlnL29SOU1yQWwrOHlrNkU0ejZl?=
 =?utf-8?B?Ly9oMUc0SW41U3ZJanZJRVVsQ3MzaVJHSnY5cVZzS3hGdG1ZYkV2aWpTaitN?=
 =?utf-8?B?eW93OHhiUjJ4REhoQnBVM0IwUjFVZER2dVRDYUxvQ0l4aE9xU3RjOEx3Yk0r?=
 =?utf-8?B?NmtxNlVlYkNDWk92TGJVMm9wN2FCMGJIQklPbFNqM2JuRVBOWnQ2cUJYYTJB?=
 =?utf-8?B?SUV3TWcxK01vTDdpM3NZSlpaOWpKMm0zNi9YeDBqZkdDNFR6V25LWE5CUlda?=
 =?utf-8?B?NkRxNjhKM01OaldDYTdmLzR3TlB6ZUJrZHpWOGd2WTJVckNDRnZLcDhVUmtV?=
 =?utf-8?B?MVc4M1h4eTN1MjZwaFdQUkthVjM2eGM5QVphTi82NXA3THBaU1c5TktXdEFO?=
 =?utf-8?B?aWtENE5mZWtSc3h0THdrVHY3OUVmbEFmaHhpVlRBb0J4SWt0WWpocENLazB3?=
 =?utf-8?Q?uUGtcrM+b8RZFxgrO3aJVFT1FwqzaaqX/UtV/4v?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: f2cb515e-64d1-4938-4bef-08d98764a54a
X-MS-Exchange-CrossTenant-AuthSource: BY5PR10MB4196.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Oct 2021 18:27:39.4213
 (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: 8NXwNOJH6s/5zkfkzKvQpQ5tFfXgvgkKVw00z0QumbP+HgIhWgyR5E9SnojuuLhfoQCzJF41DvTsJX56k5w29A==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR10MB3987
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10127 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 suspectscore=0 adultscore=0
 mlxscore=0 phishscore=0 bulkscore=0 mlxlogscore=999 malwarescore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2109230001
 definitions=main-2110040126
X-Proofpoint-GUID: RmXyuI-akyAokuHCCnL7lCZBULp1OC-W
X-Proofpoint-ORIG-GUID: RmXyuI-akyAokuHCCnL7lCZBULp1OC-W
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 10/4/21 6:00 AM, Oscar Salvador wrote:
> On Fri, Oct 01, 2021 at 10:52:06AM -0700, Mike Kravetz wrote:
>> -which function as described above for the default huge page-sized case.
>> +The demote interfaces provide the ability to split a huge page into
>> +smaller huge pages.  For example, the x86 architecture supports both
>> +1GB and 2MB huge pages sizes.  A 1GB huge page can be split into 512
>> +2MB huge pages.  Demote interfaces are not available for the smallest
>> +huge page size.  The demote interfaces are:
>> +
>> +demote_size
>> +        is the size of demoted pages.  When a page is demoted a corresponding
>> +        number of huge pages of demote_size will be created.  By default,
>> +        demote_size is set to the next smaller huge page size.  If there are
>> +        multiple smaller huge page sizes, demote_size can be set to any of
>> +        these smaller sizes.  Only huge page sizes less then the current huge
> 
> s/then/than ?
> 

Thanks

>>  static void __init hugetlb_init_hstates(void)
>>  {
>> -	struct hstate *h;
>> +	struct hstate *h, *h2;
>>  
>>  	for_each_hstate(h) {
>>  		if (minimum_order > huge_page_order(h))
>> @@ -2995,6 +2995,23 @@ static void __init hugetlb_init_hstates(void)
>>  		/* oversize hugepages were init'ed in early boot */
>>  		if (!hstate_is_gigantic(h))
>>  			hugetlb_hstate_alloc_pages(h);
>> +
>> +		/*
>> +		 * Set demote order for each hstate.  Note that
>> +		 * h->demote_order is initially 0.
>> +		 * - We can not demote gigantic pages if runtime freeing
>> +		 *   is not supported.
>> +		 */
>> +		if (!hstate_is_gigantic(h) ||
>> +		    gigantic_page_runtime_supported()) {
> 
> Based on the comment, I think that making the condition explicit looks
> better from my point of view.
> 
> if (hstate_is_gigantic(h) && !gigantic_page_runtime_supported())
>     continue;
> 

Agreed.  In addition, this code will look better when more conditions
are added in subsequent patches.

-- 
Mike Kravetz

>> +			for_each_hstate(h2) {
>> +				if (h2 == h)
>> +					continue;
>> +				if (h2->order < h->order &&
>> +				    h2->order > h->demote_order)
>> +					h->demote_order = h2->order;
>> +			}
>> +		}
>>  	}
>>  	VM_BUG_ON(minimum_order == UINT_MAX);
>>  }
>> @@ -3235,9 +3252,29 @@ static int set_max_huge_pages(struct hstate *h, unsigned long count, int nid,
>>  	return 0;
>>  }
>>