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: <60998a62-eebc-7c95-2d48-943fe62d09b9@oracle.com>
Date:   Tue, 3 May 2022 14:49:19 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.8.0
Subject: Re: [PATCH v9 1/4] mm: hugetlb_vmemmap: disable
 hugetlb_optimize_vmemmap when struct page crosses page boundaries
Content-Language: en-US
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: <20220429121816.37541-1-songmuchun@bytedance.com>
 <20220429121816.37541-2-songmuchun@bytedance.com>
From:   Mike Kravetz <mike.kravetz@oracle.com>
In-Reply-To: <20220429121816.37541-2-songmuchun@bytedance.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?WWJoS0x2cWwwQlAvSjIzVjFSYXlBM0hWN3MrS2t6bVY2NnRHeU1xVXM0RkFr?=
 =?utf-8?B?VGphWVpHMkVwczNzTG5oZEZXUmI0eGg3YUVLWlhJb2g5YXBLZzFGQ0xzT2c1?=
 =?utf-8?B?OTg0K1hLQXhtZTZERk9rWGZkeGd5a0hoVElYZFliWnA2R2h1anh2TnkxUnNr?=
 =?utf-8?B?UXkyR2w3czF6SyswNXYxT0VRWFljY3R5R1FCdDREaWgrcVd3TG96b054dkVn?=
 =?utf-8?B?L0tKWU1jUTJoeVkxa0NYTjQ5YWk1MXBpZk9YbnJxeFpQNWpPUWhRaHNRaVhO?=
 =?utf-8?B?cGpOU2tlRE1kVkl5bjlNb1QxQ2FQTVpOR2ZjMUp1dCtYeWZKdTNLTDlzdXhp?=
 =?utf-8?B?QjJFMU9SZEFmMVhLQTVIdlN6V0xZTTBpcEpQMng3Q3d3SGNHKzNVaEdnYXJr?=
 =?utf-8?B?elFVTSs2Vko1c2o2WksxODdHa1ZNWEJLL0ZCazl0RWtLNkRyY3R0QXhJRCtS?=
 =?utf-8?B?anllZmt3N09yNXZ0VnRkZnZCN292OFo0bjA3NkFRc0dBWTRaQ0I2eUxCd1Fs?=
 =?utf-8?B?S3lhN2t1WVFXWFZkcldVY1d5a1lnMTVDdFpkOUtBZEVlMWpxdklBOFpoaHpi?=
 =?utf-8?B?WXlFL2RPa0NPOXFVckJjSjQzYTMwSG5tOVlrL0dVYU9vUUx5MVRVUThtUVRX?=
 =?utf-8?B?QWxuVjQ1SmNzNHpvK3JtRFYyRDBNbWlTVWJ3ZE92TXVob1MwSVBWaHk0aVFh?=
 =?utf-8?B?NG1KcmhpSmJmRmwzTW9IY1NmL010WVN0bjV0MVJKWHVxRzZYQkpQS1B5azhN?=
 =?utf-8?B?K0QxYnV0TnNDY2NOWEdKTXJQZnp0NE8zakdsaGNGUGFjazE4ZEJkNDdVWTBk?=
 =?utf-8?B?OGgzRlVoN3ZPUlk1ekJuc3dDVkRVWjhieGRhcDlGRGVNQjJOTlhieW5jbnkv?=
 =?utf-8?B?Mk10VjZEMFFlaHFSWXFCSzVzWWlJRFBPN1p4bFdXV0tIWlJySTZ1T1lEZEJO?=
 =?utf-8?B?RDBjWUFrcE5JcThNVDM0aEtrSVlRbTcrdkRjZVNkUUVTM2lXQkZtT2xXWENl?=
 =?utf-8?B?ZW9OR1RaLzQ3TzJWR0EzZWZmMGZMTjVQN3JSYVhoQWNrVU5KeVBUSGhsSGJL?=
 =?utf-8?B?Nk9EeWlZTCtYRVhyU1V5WllaNk00WFdzZ1RVSG9VdUpMYmJIbVZXLzRGNFl2?=
 =?utf-8?B?R09BcjUxVFRodFIvZTl5ZUFMYkQ3M281TC9XditaYzF5SG5EMC80V0lpelJM?=
 =?utf-8?B?SVd6a1ppQmZWQUh0T056NUdkUC9XZ0FtVmQreFZKdnZQMFZ1NDIxRjRaaXRt?=
 =?utf-8?B?VHd5TWx4eENDMWlEcy9zQjU1K253T21oOGhSTElzYjRsM0ZUcEc5cFdaZGtH?=
 =?utf-8?B?cEh6NGx2dVc1T3FoclNyakhpem1MNmNCTkJaWFNxUUh0dW9OeDdBbDh1MXp2?=
 =?utf-8?B?K2RTM29mZFVyWkl3dGF6bjQ5b09aYmtyZDAxNTdwdG8rVTlVOEp0TklEcGtT?=
 =?utf-8?B?RkVsN1RubC84STlRVnpVSDRMdkNIaWNSM2RsWHlhaHJ1blcyNnlyK2szNFI5?=
 =?utf-8?B?K2g1TWNETWtPbmJDUW13UGVlMG5LRGRmcWhlRXBXSEF4Wm1UQnNkdDQyT3Jz?=
 =?utf-8?B?alIvK0xnR0E4RTlEOCtpVTVDWGdPWDNuWndBeldoTjl0R1l4ajNOMFh0WHVl?=
 =?utf-8?B?VlNaK2xBTWNoUjd4YkVkdFBEWG14cHFNSjV3bkw3NldIYWt4a011S3NiRkdw?=
 =?utf-8?B?YkxrYVNjVEdhVUY4d205NWo2T3pQUjBEdC9laStlWDVONzNzZXlPSmZUVU5T?=
 =?utf-8?B?Ymo4TnVpVkUzSEJEb1RLVWcrOHV1MzBTY1VIQmcxQkFSaEdZb1FCOVp5ZHIw?=
 =?utf-8?B?MFN0TzJVQ05rbmo5NkZSRXNQTUQ3cXpjUUJ0NU93SytRK216MlVVTS83eHgx?=
 =?utf-8?B?TVQveCtEK2x0Vkh4bnVYSDhHUGNvOVlRc2RPY0VIVk1mV1hRejJ2bFFDUmxR?=
 =?utf-8?B?S3RFS1FHL0lXUXBvVElmeDFzU3BNaHpzM3MvVko4RUpiaTlyMUdPZnFwOURp?=
 =?utf-8?B?UWdhRzRSY083VkN2VFFlWitXV0hua0JWT3BNZEU3a3YxaVVnYitmQXBMbzRB?=
 =?utf-8?B?a0VBRFBVVE1tNFp4RUZkWnFIQ3Z2Y1lNb2FNR1JnbUFtR29zMFhKU21FSUx4?=
 =?utf-8?B?R1UxdUEzVW5EWitWeDFPMW9MRndZdzJHcFdYVG1MM29SSU9rbHh2Y0JETGdp?=
 =?utf-8?B?QTFPNGxWL3BZN3F0QkNYMjNZeVJTNk5pZWo4YUROaCtVVWIwWS9yb2tzZ2Yr?=
 =?utf-8?B?N2tCTWtLdWFtYmtIazNCWnloYm5BTVdScjlrYkZlcDVEMmZhdGlvNFdYQ3Bp?=
 =?utf-8?B?ZFR5c25qRXZDR05KN2Y2cEZ6TWg1SHJLTGlsTkRyemZzSTVKSFNCVjl0bWlZ?=
 =?utf-8?Q?zEWJO6stQqkvPELI=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 14d994e4-73ef-4bfd-4be7-08da2d4ec811
X-MS-Exchange-CrossTenant-AuthSource: BY5PR10MB4196.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 May 2022 21:49:21.5518
 (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: kteL85l+GsI6eU0zb3R9EC9U+nuR4hIFDE880ma4cyfX/zgkwypisgXYL87xjQ0cgfQtvLW8cuA8/Okkty2YjA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH2PR10MB3943
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.486,18.0.858
 definitions=2022-05-03_09:2022-05-02,2022-05-03 signatures=0
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 phishscore=0
 mlxlogscore=999 mlxscore=0 spamscore=0 suspectscore=0 bulkscore=0
 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2202240000 definitions=main-2205030132
X-Proofpoint-ORIG-GUID: E6C9IaDOW3n4DLoRsMTpGhgyN2UDhNKJ
X-Proofpoint-GUID: E6C9IaDOW3n4DLoRsMTpGhgyN2UDhNKJ
X-Spam-Status: No, score=-5.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 4/29/22 05:18, Muchun Song wrote:
> If the size of "struct page" is not the power of two but with the feature
> of minimizing overhead of struct page associated with each HugeTLB is
> enabled, then the vmemmap pages of HugeTLB will be corrupted after
> remapping (panic is about to happen in theory).  But this only exists when
> !CONFIG_MEMCG && !CONFIG_SLUB on x86_64.  However, it is not a conventional
> configuration nowadays.  So it is not a real word issue, just the result
> of a code review.
> 
> But we cannot prevent anyone from configuring that combined configure.
> This hugetlb_optimize_vmemmap should be disable in this case to fix this
> issue.
> 
> Signed-off-by: Muchun Song <songmuchun@bytedance.com>
> ---
>  mm/hugetlb_vmemmap.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)

Thanks,
I think the runtime power_of_two checks here and in later patches are
much simpler than trying to do config/build time checks.

Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>
-- 
Mike Kravetz

> 
> diff --git a/mm/hugetlb_vmemmap.c b/mm/hugetlb_vmemmap.c
> index 29554c6ef2ae..6254bb2d4ae5 100644
> --- a/mm/hugetlb_vmemmap.c
> +++ b/mm/hugetlb_vmemmap.c
> @@ -28,12 +28,6 @@ EXPORT_SYMBOL(hugetlb_optimize_vmemmap_key);
>  
>  static int __init hugetlb_vmemmap_early_param(char *buf)
>  {
> -	/* We cannot optimize if a "struct page" crosses page boundaries. */
> -	if (!is_power_of_2(sizeof(struct page))) {
> -		pr_warn("cannot free vmemmap pages because \"struct page\" crosses page boundaries\n");
> -		return 0;
> -	}
> -
>  	if (!buf)
>  		return -EINVAL;
>  
> @@ -119,6 +113,12 @@ void __init hugetlb_vmemmap_init(struct hstate *h)
>  	if (!hugetlb_optimize_vmemmap_enabled())
>  		return;
>  
> +	if (!is_power_of_2(sizeof(struct page))) {
> +		pr_warn_once("cannot optimize vmemmap pages because \"struct page\" crosses page boundaries\n");
> +		static_branch_disable(&hugetlb_optimize_vmemmap_key);
> +		return;
> +	}
> +
>  	vmemmap_pages = (nr_pages * sizeof(struct page)) >> PAGE_SHIFT;
>  	/*
>  	 * The head page is not to be freed to buddy allocator, the other tail