Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <9d64809f-db8c-0a3e-1ae9-d4a8ab79041e@oracle.com>
Date:   Tue, 10 May 2022 17:39:40 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.8.0
Subject: Re: [PATCH v10 4/4] mm: hugetlb_vmemmap: add hugetlb_optimize_vmemmap
 sysctl
Content-Language: en-US
From:   Mike Kravetz <mike.kravetz@oracle.com>
To:     Muchun Song <songmuchun@bytedance.com>, corbet@lwn.net,
        akpm@linux-foundation.org, mcgrof@kernel.org,
        keescook@chromium.org, yzaikin@google.com, osalvador@suse.de,
        david@redhat.com, masahiroy@kernel.org
Cc:     linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-mm@kvack.org, duanxiongchun@bytedance.com, smuchun@gmail.com
References: <20220509062703.64249-1-songmuchun@bytedance.com>
 <20220509062703.64249-5-songmuchun@bytedance.com>
 <970166e0-f70e-dd2a-c764-af23a8425f87@oracle.com>
In-Reply-To: <970166e0-f70e-dd2a-c764-af23a8425f87@oracle.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?Zjd4R2UxYldiZnBzaUdWWmJvV1E1VGFobU9rS0ptWktpbHV4RFRJazVaR1hY?=
 =?utf-8?B?azNvcFh4YVl5bmtEMGVPVFpSWG1oM2MvRnZFeXVGNnRGOUt5cFVPSWl1Wmky?=
 =?utf-8?B?eWU5a1cvdW1BYmE2VmVHNUxWeUNRWHkyNXVYZEthblFwakxNYnVlZlgrcy9V?=
 =?utf-8?B?dGVuOW1ySWVEc3lIWGFqTEFLUlVZcGJ6N2RRWmhtZTlOd2xTM2V0bEFKTVpu?=
 =?utf-8?B?S2FucmE5ZTRPcjZ5em43MXNITitubThQa2R2ejg3cnpIUXNoT2Y0QTdqK1hs?=
 =?utf-8?B?RGpQMTl4TmFITVd4WmxzUi9qWXlZQ0VRc2Q4dUxNUmVSN0ZvZSt0cUJ4MFEr?=
 =?utf-8?B?WmI5Wk5hVVJsVk5jNDFLMjVyY0dlbGdLT0tUL05EQjM2amd3a1RiYTh0UUEx?=
 =?utf-8?B?STUyTVc3UVdSaG82ckNibnRNcXdrbXorMzRPalQrc245K0tUWHlaVkFCTEM3?=
 =?utf-8?B?NmRLbVVlSVVDYmNieXkwTW1zRlh3N2NtWVJacGNZVG1oeURleDczcG9EV2hm?=
 =?utf-8?B?akNUZUhQd1V6aTRPeCtUNml6YlhQaUYzV051eCtJUWMySVZxaEVKQ1M3OG4w?=
 =?utf-8?B?dFZqQTlSQ3ZabmVyeEVyd0xnbnFycDJsNVJQVzVvN3JST1FIa3FUNkM0K0Fi?=
 =?utf-8?B?TzlheFFFZW1obkpWMTgwMHd3d1piNnB2Nm1MYmExVExzVlJaTWg5YzhscjdF?=
 =?utf-8?B?dkRxUFMxdWNsdGVVSGZHNXVWZndrM3AyOWpOb2J5ZUMrVHNuY2ZXelRhWmMw?=
 =?utf-8?B?RmxMR2F1QnhMeEdyVnh0UlFwTGJQOGJzREtFZ2dXZzNQSnZWQkQ4QWFTWjBr?=
 =?utf-8?B?N0EyRXByb01ZSTZ6VWltSXB3T2xxV3BOeFhxZjZXeG81ZzVucks3MnpqRVpw?=
 =?utf-8?B?MHRkUjFjVE1zM0NEVFJ5QXYzV3dWcjE2N0FYMUJ6YkxyaG44UWMraThKTkt0?=
 =?utf-8?B?ZjhtdUlNOFVCazdJcFV0Q3Jydld1cDlkSmt6NHZBZm5Ubm54RTBEelRHRGwr?=
 =?utf-8?B?dVQySS9jOER5MkE3WTFTMGJOU1EvMFdDbHNQTjFOVDhqcWJwb1ZBOEUxSjQy?=
 =?utf-8?B?MVl0MHJrKzA3a1dKRm81eHJQcm1TbnEwZSs1MW1VT2IzZzNtenV3MVdYdldv?=
 =?utf-8?B?NEVwSXUyTitJYks3OW9oek9iUVVJRmU4dW1yTWRnTGVianJVNUlkNW1XTG5F?=
 =?utf-8?B?a2NrdzUyVUlNRlRuVDFtMzhrSWFIcmVrWFM5eHlHN21BNU9FRXN5YmNYZ1gx?=
 =?utf-8?B?cUJqVmtobXp6MnJsY210Z3prSVBySjArc0xyT2ZVZFdadGo3SkROcTl5WkxQ?=
 =?utf-8?B?czBCdmx3RVVVVFRIUE5aS1kwNkZvdncrNnpGbE5xSUY3aFUvMmoxSGJVQmZQ?=
 =?utf-8?B?Rm1ENlB1aVB6WjUzd1FpdVk3YzE2SDBxSEsvSU5NN0dyRlNVUG8wdVh0V0c2?=
 =?utf-8?B?SG1VRzV0RFl6OVB0enRCWktyTkxBYnJUVnBEc1NWcFRrVFQ0WW1NMjV5NlZC?=
 =?utf-8?B?QUlrQmpQSFZ5c3ZINW5udXgwVWdsaVJOK0JqbW00UUlDeFFTVlBWLzZBR0Nm?=
 =?utf-8?B?VGlCV3JYQTY3K2RrcTFPUVBmRy81ZmVzbDZuZGsrQU1aQTZWTDEzaHdUSDRn?=
 =?utf-8?B?OVhrWHFWRFYvbUhMT3JvRGtnS0x3bERwNWl1MGhIWU9yb0p4WWlBSmN5Y3JW?=
 =?utf-8?B?V0pyamxnZjdkMWJCdGxmRktBcWhmNkxTMk1IZkVqRStOcjZ6ZkRnb01xMzlh?=
 =?utf-8?B?b0ZZbzkwUUZEZm5iK0FpOTJIUGt2TTZjM29GUkRJV0kxMWt2ZngyV2VqR3Yx?=
 =?utf-8?B?SjdnWVNGSExTLzRFSnFwbDQ0Sit1NktBczBEbll3RUJKb0pzdmJ4Y0pkQnAr?=
 =?utf-8?B?L0ZFSy9CWWREWUxwa3lEVjAvVk5wN2tDMk8zVDBMUkRXR0xPYVdoU2l4LzFX?=
 =?utf-8?B?RXpWcUQrSTJsQS9sc3VkTlNvQU9iSVFOMUVNaWVtYzZyZTAwakkrVnBmMnJU?=
 =?utf-8?B?N3VBYWJrMlJSWnZaanZVUUVyU2FyNFFTREVLN0Q0VDlrY1pOMkpTMm1mdjBV?=
 =?utf-8?B?dytvT2hvUDR4NzkwRFdGWENYL3AvdDhMRUQ3OS96VVZSR25ocnV4Y2pKZHlU?=
 =?utf-8?B?dlVTbDZJSC9lM0FYbk1aVDUzS1pVT09yemxhREtySTZsRTU1K2tmcnVOdFNU?=
 =?utf-8?B?TjJSSXdCdHFVQmluakRaeC94ZkZucUgvTmdzc0lWOGorWUZQSHI3bjlNcFpo?=
 =?utf-8?B?V1hoSlRUbnFMd3VyakM0TStubkNxSXkrS1dGeU45SitGS2tSaTk2dERwR21N?=
 =?utf-8?B?VTZVbW5ET2dHd2ZGbXRJNTArWDNldVlWd2dSUE1zd3VORElSczgrTnZrbloz?=
 =?utf-8?Q?JKrxyTx8iXJgfX9g=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 77cf0bb4-c4c8-4384-5a60-08da32e6bd65
X-MS-Exchange-CrossTenant-AuthSource: BY5PR10MB4196.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 May 2022 00:39:42.9624
 (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: GubFXcN4Oiluwpd3DgncR4LQD5OL68IwQnRjznMg58Lwll0XQb/I2CTTM9ZXdiEj5GUtcJPt9PBQuREHFf7HDQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR10MB1872
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.486,18.0.858
 definitions=2022-05-10_07:2022-05-09,2022-05-10 signatures=0
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 phishscore=0
 adultscore=0 bulkscore=0 malwarescore=0 mlxscore=0 mlxlogscore=856
 spamscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2202240000 definitions=main-2205110000
X-Proofpoint-ORIG-GUID: M2BZ_9kCC44_yXE7FXcoS2kbv6p9LzEX
X-Proofpoint-GUID: M2BZ_9kCC44_yXE7FXcoS2kbv6p9LzEX
X-Spam-Status: No, score=-3.7 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_LOW,
        SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 5/10/22 14:30, Mike Kravetz wrote:
> On 5/8/22 23:27, Muchun Song wrote:
>> diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
>> index 029fb7e26504..917112661b5c 100644
>> --- a/include/linux/memory_hotplug.h
>> +++ b/include/linux/memory_hotplug.h
>> @@ -351,4 +351,13 @@ void arch_remove_linear_mapping(u64 start, u64 size);
>>  extern bool mhp_supports_memmap_on_memory(unsigned long size);
>>  #endif /* CONFIG_MEMORY_HOTPLUG */
>>  
>> +#ifdef CONFIG_MHP_MEMMAP_ON_MEMORY
>> +bool mhp_memmap_on_memory(void);
>> +#else
>> +static inline bool mhp_memmap_on_memory(void)
>> +{
>> +	return false;
>> +}
>> +#endif
>> +
>>  #endif /* __LINUX_MEMORY_HOTPLUG_H */
>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>> index 8605d7eb7f5c..86158eb9da70 100644
>> --- a/mm/hugetlb.c
>> +++ b/mm/hugetlb.c
>> @@ -1617,6 +1617,9 @@ static DECLARE_WORK(free_hpage_work, free_hpage_workfn);
>>  
>>  static inline void flush_free_hpage_work(struct hstate *h)
>>  {
>> +	if (!hugetlb_optimize_vmemmap_enabled())
>> +		return;
>> +
> 
> Hi Muchun,
> 
> In v9 I was suggesting that we may be able to eliminate the static_branch_inc/dec from the vmemmap free/alloc paths.  With this patch
> I believe hugetlb_optimize_vmemmap_enabled() is really checking
> 'has hugetlb vmemmap optimization been enabled' OR 'are there still vmemmap
> optimized hugetlb pages in the system'.  That may be confusing.
> 

Sorry, I forgot about the use of hugetlb_optimize_vmemmap_enabled in
page_fixed_fake_head.  We need to know if there are any vmemmap optimized
hugetlb pages in the system in this performance sensitive path.  So,
static_branch_inc/dec is indeed a good idea.

Please disregard my attempt below at removing static_branch_inc/dec.

I still find the name hugetlb_optimize_vmemmap_enabled a bit confusing as
it tests two conditions (enabled and pages in use).

You have already 'open coded' just the check for enabled in the routine
hugetlb_vmemmap_free with:

	READ_ONCE(vmemmap_optimize_mode) == VMEMMAP_OPTIMIZE_OFF

How about having hugetlb_optimize_vmemmap_enabled() just check
vmemmap_optimize_mode in a manner like above?  Then rename
hugetlb_optimize_vmemmap_enabled to something like:
hugetlb_optimized_vmemmap_possible().  Sorry, I can think if a great name.
-- 
Mike Kravetz