Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33;
Date:   Fri, 13 Oct 2023 14:43:56 -0700
From:   Mike Kravetz <mike.kravetz@oracle.com>
To:     Naoya Horiguchi <naoya.horiguchi@linux.dev>,
        Andrew Morton <akpm@linux-foundation.org>
Cc:     linux-mm@kvack.org, linux-kernel@vger.kernel.org,
        Muchun Song <songmuchun@bytedance.com>,
        Joao Martins <joao.m.martins@oracle.com>,
        Oscar Salvador <osalvador@suse.de>,
        David Hildenbrand <david@redhat.com>,
        Miaohe Lin <linmiaohe@huawei.com>,
        David Rientjes <rientjes@google.com>,
        Anshuman Khandual <anshuman.khandual@arm.com>,
        Michal Hocko <mhocko@suse.com>,
        Matthew Wilcox <willy@infradead.org>,
        Xiongchun Duan <duanxiongchun@bytedance.com>
Subject: Re: [PATCH v2 01/11] hugetlb: set hugetlb page flag before
 optimizing vmemmap
Message-ID: <20231013214356.GA245341@monkey>
References: <20230905214412.89152-1-mike.kravetz@oracle.com>
 <20230905214412.89152-2-mike.kravetz@oracle.com>
 <20231013125856.GA636971@u2004>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20231013125856.GA636971@u2004>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?49wHEbTb1XOJ3PQmI3n/Vr4Y7oIZnK5iPIYZMcT/IP6kCqeDQJRk8OQwI/Ol?=
 =?us-ascii?Q?gIRBmNJNWT6+TeufFzSPx3no/RUoDTw+q5cuX9vOqlEiaaRHGqR7DwFw99U4?=
 =?us-ascii?Q?hDiOhzk1Fv/YpqzSY8E00sBSzimbYHt8ODwHTrZGrgkw/CM9+OcivK6fpCnF?=
 =?us-ascii?Q?VlTJENq2iqGgQzSLkwzutF54ioj9jo9SGWQBN+602yD93rVUfSTe+I65cFRd?=
 =?us-ascii?Q?PDcz6TcEHVQHx4AaQPylWke0T9TKOnBJVNTCpFeDG8UxvkYMzOub0Dczdoe0?=
 =?us-ascii?Q?0ShVyg2ZWXd8MkSE/vTEhXdOQD3Xckj0bBVznqgcBGsORym6kIZj0KXV1g4L?=
 =?us-ascii?Q?aiazEsoAEEf5vS12VRLtm+zFu8qNfjAw/A6EjMUWqgo6lV0W6cTsA62q/jtJ?=
 =?us-ascii?Q?4th4h72PLXkinKW4WzhbFwoTtrJPMzV/QaFc7kmOVi3nNZRTVMpWWDNqyrGh?=
 =?us-ascii?Q?7+K/AWWaxln6BbRyj2wmoHy9OHEXm9g0IS0WytR9Br58dfgbvkR6goUkh6yV?=
 =?us-ascii?Q?1x4rOgUOoh3iXkGNrsowjx0DUWkZQGZ5us2BeJE2g3rAbHlF7c9vV+3Hnxk2?=
 =?us-ascii?Q?9TVl0Z8/41vG+tW+wwWBcZ6s29OitQRnecAZpvg+6IRkxgr5wGOnI4WHU3TT?=
 =?us-ascii?Q?KxaSrRUqzNTUty8ycBQnGwRCGCXRZ+0N3tcLftEjzG2q3XL0GFG7Y26KaO8v?=
 =?us-ascii?Q?FbLTKO/nAnpA9vx9hzVgSJ8Js7Z7GKApKGiyhIZGVoZcHjAg3dmIVFxr9y5a?=
 =?us-ascii?Q?8CzQ3MryeNAd23pBzvLPG6haH5eCH2HAuqwdXM93HuEqIdmWLdoVhKqSPic1?=
 =?us-ascii?Q?5iEsTs/psN4EVRolIvX1CNAV9w+GtEc9gLsrS9rsbBrUpOTITPVardeO7ji1?=
 =?us-ascii?Q?L9tldLNMf/9GJgTPWBakUu72YlWHfMeRnd0hZVueO8G0KrtwF1UK4Juk8VVg?=
 =?us-ascii?Q?r8E2JvkezPnlsIJpkD+wrut2kxgoj3wTFgInjHsryHFSKRL7Tz+i4rHzJ/mg?=
 =?us-ascii?Q?7wtJp2bR80fDhcrzr0Gs5S3x518EkVIC5QOkzgOg79fhSYIIJ11PFy/YbpJR?=
 =?us-ascii?Q?gfrq1AADgM5LJAlSbTytTmzFSUqMIGhx5kb6SIk6SNibHValxUQ/7Qrm6Pwq?=
 =?us-ascii?Q?kEtEagOOP67OV5TFEJVqA5PbEwY5fH4Wle6wJt7e5XBG5xZsrqHsNQCw1dPe?=
 =?us-ascii?Q?wJErcB2+7d/+HquEhqcb/hOBxEwCT8AwWEyTkjqlvSvuMaCpBWBM0WsVhCbn?=
 =?us-ascii?Q?RvGnmkaezFONCVoHJfcylXx69UPSu4NoGoQ605hdyLcJnchsRszJ75vxdF7L?=
 =?us-ascii?Q?s+QB0W+OdRMfHvsCk5szJ4U6EwXljg+RPDRtfWElwpjXAtTL5bYzNsWJ2aYC?=
 =?us-ascii?Q?NWd09bcCtodCTXaqKlLLwe3sRzGLUYAcLHy9bHy73babDNVJ/KpTXSgb25kZ?=
 =?us-ascii?Q?XiUw6HH4Cp/o8l5WG5akKBo4K2yf8xIVmeGkC4GR5HgwLsqEvgYSJUyh8UnR?=
 =?us-ascii?Q?BynTbujLmcObsed2oI1z7T4vphM242xEubIvrdkrvaXg2zyFzegF3ydhUszp?=
 =?us-ascii?Q?fXdnAAgU+X+zo+i2L8G8Kif7jGJ4PNQVHcLoHAwS?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: =?us-ascii?Q?VJkmTWwwTbXhMd5UQTvCosCc/xIJSL01NY3V0XtMoNSODY9uFVLP34qf0ncW?=
 =?us-ascii?Q?PAIr4RBgKMSOkhAG/KPsUlBjCpMQE6vnzazFa4VTkfwEAuxaj96Anll4DoAA?=
 =?us-ascii?Q?37tfggKh2OAclwTJi4KzVOVHSJQLY6gUSdREtAYqgsRLJgiI47I437+QxQ62?=
 =?us-ascii?Q?LIs5MNCstICxEM0OkdlHjbLm4nx5T8gYg6J81MpWN8+ZTAOPGdS+bPF96K2g?=
 =?us-ascii?Q?WMgWxCgKRAdGROSCfkUU1doMYVZEs747LWg6gP7InWVxXG+L9HmSyfnuuBHV?=
 =?us-ascii?Q?qcqQOn82D7y59m4KVZmkHZsNBD0bJ+cX/WQVQHe4arcbW7Uamu4YHAhTYU2Z?=
 =?us-ascii?Q?QDk3FWSMErg2PjC8uAx3R9Grprpm6l6Ac6Kz3Y54yCg0SiUl+FTF4NtVoAFM?=
 =?us-ascii?Q?0Mle2VSA2k1rlIMVcF+QmeNWmtQHsntPn7k8R3PLcaExpRruua1xD2ilIBWT?=
 =?us-ascii?Q?EA0b/drZEWHQ2Z5NVOtAhuTPQ/yrlVYklUkl79mTPA1Q2vpnZlD+7W3tULan?=
 =?us-ascii?Q?99XQgwZlJwUjzJd5dk5gqLcbw0/qDX9XEvsOA+AU5h88sXEB26MtAU8/cZeQ?=
 =?us-ascii?Q?xGPPGUVdqc2Jfe9InWUzFwFefZzW6V7IB0V2p3KbRRDtXkrh6x5YpKzD31tG?=
 =?us-ascii?Q?bxaNdL9DvOhYv2i3CpkkEhLVFjhjeA+HwOYNgeU4MPJWwY18UTCnWPZP/Qso?=
 =?us-ascii?Q?n7gcDWKmnNZmBEcjNAu8uGHLFbvAxSxZ3TOwxHp581bFCADYamoDZqWUQisw?=
 =?us-ascii?Q?5H/gQ5Gm8NjIbpHw5yNtAxQ7vl+yAQRLKOqV9/Oak7e6U538GSKLugS/3vFm?=
 =?us-ascii?Q?uIRV0X3i8zhmaeRDOmz3Z8CyeAenaL/rICIg0//GczzYyeW+MaM/ECq6yxdh?=
 =?us-ascii?Q?HnjKIisZZcksfEmixn30hmECD0jYMJMoryufRj6D/+4RbqzrKzt6VX7YpeCb?=
 =?us-ascii?Q?73IiOm9brTRmd1FhzFGLQSFxmn/nPnGJoo+/zhu3Rx+WGlpyFC7YpvNZ3TfW?=
 =?us-ascii?Q?JJfyDWPObYphE64cM+PFm4Eq6qgFj8wAHE1j6oldYDIihoQ3IVoAj+HMVRYv?=
 =?us-ascii?Q?z7tCvUizfpkROJjIFRB8go+FY+gLxm+D6SJpj/xodzlWaTtLizU=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 40606df6-f73e-4b2a-0bff-08dbcc358243
X-MS-Exchange-CrossTenant-AuthSource: BY5PR10MB4196.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Oct 2023 21:43:59.7301
 (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: jTHHaY1xhphAqwhHClh5hOm2q8+jzJFLcXXYKTPiUKjnGqZ2lJgF9i0OCBxYW+liZ+n1yxly5aUg1MO0P6N7QA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR10MB5794
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.980,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2023-10-13_12,2023-10-12_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=999 suspectscore=0
 malwarescore=0 adultscore=0 mlxscore=0 bulkscore=0 phishscore=0
 spamscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2309180000 definitions=main-2310130189
X-Proofpoint-ORIG-GUID: dvxBJpHXLsHKe2VBRFUh64RwjNlA2lKm
X-Proofpoint-GUID: dvxBJpHXLsHKe2VBRFUh64RwjNlA2lKm
X-Spam-Status: No, score=-0.8 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,
	SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no
	version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lipwig.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (lipwig.vger.email [0.0.0.0]); Fri, 13 Oct 2023 14:45:09 -0700 (PDT)

On 10/13/23 21:58, Naoya Horiguchi wrote:
> On Tue, Sep 05, 2023 at 02:44:00PM -0700, Mike Kravetz wrote:
> > Currently, vmemmap optimization of hugetlb pages is performed before the
> > hugetlb flag (previously hugetlb destructor) is set identifying it as a
> > hugetlb folio.  This means there is a window of time where an ordinary
> > folio does not have all associated vmemmap present.  The core mm only
> > expects vmemmap to be potentially optimized for hugetlb  and device dax.
> > This can cause problems in code such as memory error handling that may
> > want to write to tail struct pages.
> > 
> > There is only one call to perform hugetlb vmemmap optimization today.
> > To fix this issue, simply set the hugetlb flag before that call.
> > 
> > There was a similar issue in the free hugetlb path that was previously
> > addressed.  The two routines that optimize or restore hugetlb vmemmap
> > should only be passed hugetlb folios/pages.  To catch any callers not
> > following this rule, add VM_WARN_ON calls to the routines.  In the
> > hugetlb free code paths, some calls could be made to restore vmemmap
> > after clearing the hugetlb flag.  This was 'safe' as in these cases
> > vmemmap was already present and the call was a NOOP.  However, for
> > consistency these calls where eliminated so that we can add the
> > VM_WARN_ON checks.
> > 
> > Fixes: f41f2ed43ca5 ("mm: hugetlb: free the vmemmap pages associated with each HugeTLB page")
> > Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
> 
> I saw that VM_WARN_ON_ONCE() in hugetlb_vmemmap_restore is triggered when
> memory_failure() is called on a free hugetlb page with vmemmap optimization
> disabled (the warning is not triggered if vmemmap optimization is enabled).
> I think that we need check folio_test_hugetlb() before dissolve_free_huge_page()
> calls hugetlb_vmemmap_restore_folio().
> 
> Could you consider adding some diff like below?

Thanks!  That case was indeed overlooked.

Andrew, this patch is currently in mm-stable.  How would you like to update?
- A new version of the patch
- An patch to the original patch
- Something else

-- 
Mike Kravetz

> 
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -2312,15 +2312,16 @@ int dissolve_free_huge_page(struct page *page)
>  		 * Attempt to allocate vmemmmap here so that we can take
>  		 * appropriate action on failure.
>  		 */
> -		rc = hugetlb_vmemmap_restore_folio(h, folio);
> -		if (!rc) {
> -			update_and_free_hugetlb_folio(h, folio, false);
> -		} else {
> -			spin_lock_irq(&hugetlb_lock);
> -			add_hugetlb_folio(h, folio, false);
> -			h->max_huge_pages++;
> -			spin_unlock_irq(&hugetlb_lock);
> +		if (folio_test_hugetlb(folio)) {
> +			rc = hugetlb_vmemmap_restore_folio(h, folio);
> +			if (rc) {
> +				spin_lock_irq(&hugetlb_lock);
> +				add_hugetlb_folio(h, folio, false);
> +				h->max_huge_pages++;
> +				goto out;
> +			}
>  		}
> +		update_and_free_hugetlb_folio(h, folio, false);
>  
>  		return rc;
>  	}
> 
> 
> Thanks,
> Naoya Horiguchi