Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) client-ip=23.128.96.38;
Date:   Mon, 16 Oct 2023 20:21:40 -0700
From:   Mike Kravetz <mike.kravetz@oracle.com>
To:     Naoya Horiguchi <naoya.horiguchi@linux.dev>
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>,
        Barry Song <song.bao.hua@hisilicon.com>,
        Michal Hocko <mhocko@suse.com>,
        Matthew Wilcox <willy@infradead.org>,
        Xiongchun Duan <duanxiongchun@bytedance.com>,
        Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH v2 01/11] hugetlb: set hugetlb page flag before
 optimizing vmemmap
Message-ID: <20231017032140.GA3680@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?kkvufVbdz0k56c20T/bMAmDjWvfrcbtRZuk91HER+8+ttJ4HqSPY4B363ii3?=
 =?us-ascii?Q?ZbyGCqJ/ov0eU3rW2+mCU5dOWjBWuNenaEPlyGXQpWhuCZcRC9KZ5j3yorzf?=
 =?us-ascii?Q?nFCP72pn9jDYKUx526ALpLb3F2pyBdd/WyVZvyIe99nIXAmo9RGsZzKIz+F8?=
 =?us-ascii?Q?4ikgFQO9oRF8LzTp8ZPLqqJ77lXjVqK+1reCeqEIw3XEt8+R6wYaj+m4dPA8?=
 =?us-ascii?Q?YpQZcz9hUFWtwNGl9msgxKCFJ6+SHFRFsyBHDxvNOOM80Y+RNMCZ5N5D5HDK?=
 =?us-ascii?Q?gOmfOUam2z8BumeMJN/7ouTTfmpuiEc0uTxrfXjLYtsdsw4Fw4ArWZDCsMAM?=
 =?us-ascii?Q?BFxxK2fF3UBeCHp48aLPzXlrVMP2oGewxXLFD6xiyWDscKkuektQjF+dB3SH?=
 =?us-ascii?Q?6hEW2v1vxkF+RrR/jjsziRZufZ4LzE9/J3UAe0JLSvgOTSCHu4szuVchqZq3?=
 =?us-ascii?Q?BFqLR9DCmHU3l6LGCi1YiuMK7ThVXY34KnQi9pvLmVLZrQGN1fp8DqrFnncx?=
 =?us-ascii?Q?3blGWVeIfOb2dn7RN6G5kx2kBGnrnJGrx2hbHwnplGMcM2j+ehOqSu46y74h?=
 =?us-ascii?Q?/njpmLrvmKeQZIuCXiTJLW+13qBy2Cb6KJ7CGhYQeuL7bhsYB5ZLhsE/bjaN?=
 =?us-ascii?Q?E5SASQ8UuNkHB7fs+0nQ/HiMklY+75Sw2f/1/Gvs5K1NOsmEH6OVitb9k2tc?=
 =?us-ascii?Q?i+9wmn8zFjiUYzpcbxSpXjaf+eXv0almXtvm8XcNZQRn+UbsPZLo3QPQoaNm?=
 =?us-ascii?Q?le0kcu5Fmqtkw24GOVYn5+nXFleOMPfknjfTLdbVYUq6auXlILD6NzNvmqna?=
 =?us-ascii?Q?is1l0lWXrRHitMUm/cya5HRUAjYAq3LAeHtaszHX7RwOzIshldmzvQVAogt+?=
 =?us-ascii?Q?zmrFJUBXk9CsKftAaPVr90qejik2SKuKBuKyBQNUyiB5SVIlpao/Xk+J/PO9?=
 =?us-ascii?Q?jTprl6YMF3iomMuxGq7fec8z2PNh9LiGEYkkhwUfVzyev9Qs6ZozqJTTGjBX?=
 =?us-ascii?Q?2ca3OA8v8iNmvKFVOprUs1S/OeEPF4XNMhkgAnTw6SoAHEfUvObmcebpmN20?=
 =?us-ascii?Q?8YOi2ftkNU27aQpNXA1CW4Pk+FnQRSHJntWTYXEOnVGpkWu97YQ7gy0j6ZY4?=
 =?us-ascii?Q?+Yc7kxKHIO3w+8728Eez2ibggFxWCbWzAZXeVsjkaOdh8uIMEcpz4fJBgTW6?=
 =?us-ascii?Q?CUHKDlmUp6mHYzcpKioOk/C+o/sp9joYTgv9ObUiVsl/DtMYmLCzvma7B1hV?=
 =?us-ascii?Q?+mmLVVLJecUmYw7LVwC9uiQpZQHdOpBpohcrKsMdE0csn89LwVB9hcUmd8i0?=
 =?us-ascii?Q?YhOXof2G+ywn6HSPuxDiX3BtooGAMWY5lxLlOD8Gg1KLvyKbX2jy9sZEEjmL?=
 =?us-ascii?Q?NhwlFpGMAWP488VbqnjhakOR/FgsnDDRvLvirj5GkOgilPtUS9+zUwEPrghU?=
 =?us-ascii?Q?PpSHCviXi3z2KxwUZ6Yqo+dOn9qvHoEXk3hGIhGEoADlW+QABEPpCpRUbKlP?=
 =?us-ascii?Q?NbO5SwB95/jlwbLXBzG7xPq1Re68+G3A4Pjlnt1miXIlFxdfmPXJ5EmHSwg5?=
 =?us-ascii?Q?Ys1dxGRFC8n6+PNcnldOl0jQtM7uNltgpNLWQ4/r?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: =?us-ascii?Q?nkfipApsnww5thI+FzIlfCAI/Q1JJ6JLzj8c8PNgWbxXkwUgRxfLGVuKE2XH?=
 =?us-ascii?Q?Exl2OfAFU0CVvoOPGSN0xMQPcFSwMmqUi/pEIaFefRyrtMj4rH3oWk3uP0oH?=
 =?us-ascii?Q?u7SSunfV2DCabv8TI4qYyRY5PBvhJUIqrXUAj1rn7YSv+b2B4niI0ni3rWsY?=
 =?us-ascii?Q?wEogBiVMursNWJ8aoGX5b3kxS3aCexBbh3T/2uNIII+lC5IgENI5JVlbwAN+?=
 =?us-ascii?Q?A9DhIwXJpWfT+ZJOQj9P7JMJM0bL7/69Db9d8J9QRrELd9SmbuWMqCJ1n+re?=
 =?us-ascii?Q?L+zduCICc4seNvI3O6M0GNFTj5mlUIf5vBQaqK/gufOzjMkb8vxTP2odx1yl?=
 =?us-ascii?Q?yCMYic9vQgLHfSzp8BCqLhVMpD+ppvi45JuyW2JWDOEKHGGoEeeYZhAUK3ps?=
 =?us-ascii?Q?LaBD+uiYMFbrsUbkoE39ngtli3S1DpT07e1PXAdVN1g6s2PkD0HINJMjBf4+?=
 =?us-ascii?Q?/4OpYzAXQJgpM3B+DO4CNRZFV2+pzvL0yvWE7uAeOLQVeV1S3uqZraIy0di6?=
 =?us-ascii?Q?TLB+I/GII8LWEpPow8xBA9HMYOg0Wrcz4febi5wCtkQ7GXxrCsGTmHWgecS+?=
 =?us-ascii?Q?SfFiU9wr4Iq6csirjmaLVV3jhBQK05eH7vc13gguAOLVtFw4T0FPErW9tqwn?=
 =?us-ascii?Q?mACXyHcBpqbHGM/CNIjwUQtaaf2bxHFd6AKnX2882TEQAETGI9ZsS0rlqWAY?=
 =?us-ascii?Q?RRNorOA8g6LRsy41DV7EE0V5LaT5QpkeDjyLviJT1ys/7/+Mzd9urRmPLI69?=
 =?us-ascii?Q?olsEkw0dGLigKom5JyciD46PgKZUeatyD9lReXuiSEHmqCQEzsEvqS3QcRBX?=
 =?us-ascii?Q?4DvchRmkXIAJNTUfBXNfdir4UrTi8wJi5xaDFkc58tqoet4pV6PZsWU/0Vxz?=
 =?us-ascii?Q?9/qm/vt/GFnAi1UrS495CJqyPVZ4nsnlPz6CJGLfsLk2CGrshiGJVuZQqYmt?=
 =?us-ascii?Q?QIC510EbnlIyMkFA1pL/+B4aOBxUpD3NTyauMmYjnMvpwUrY3VppbZhK+HSe?=
 =?us-ascii?Q?lpIAQWsSDFIZDiwNWs81Q25Db60LDMQrFyx33xYzVYU52z2CYJBi8ESlNTLv?=
 =?us-ascii?Q?z+mEn8qDCg6hulOgpO5e3rJkX4al0wcfE5fQg+j1ay3MpCgTDlOpFWxugc+9?=
 =?us-ascii?Q?FFE8HhDv9yAu?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: a2d5f71c-9e04-4b9e-6f25-08dbcec02f55
X-MS-Exchange-CrossTenant-AuthSource: BY5PR10MB4196.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Oct 2023 03:21:42.8693
 (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: U0OvJoapTexPSIxpIY8S5GcwIfB9/IRmL1G8ZoszCGux4q2dyWdHA5hHRP5HvKzfRE4PD8wWOdBvBsgoN+1hUA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA2PR10MB4537
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-16_13,2023-10-12_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 bulkscore=0 adultscore=0 phishscore=0
 mlxlogscore=999 mlxscore=0 suspectscore=0 malwarescore=0 spamscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2309180000
 definitions=main-2310170027
X-Proofpoint-GUID: 9Rkug1g6hHrObwK8goVyjKBg3ztqDZr_
X-Proofpoint-ORIG-GUID: 9Rkug1g6hHrObwK8goVyjKBg3ztqDZr_
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 fry.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 (fry.vger.email [0.0.0.0]); Mon, 16 Oct 2023 20:24:07 -0700 (PDT)

On 10/13/23 21:58, Naoya Horiguchi wrote:
> On Tue, Sep 05, 2023 at 02:44:00PM -0700, Mike Kravetz wrote:
> > 
> > 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?
> 
> 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;
>  	}
> 

Hi Naoya,

I believe we need to set 'rc = 0' in the !folio_test_hugetlb().  I put
together the following patch based on mm-stable.  Please take a look.

From f19fbfab324d7d17de4a1e814f95ee655950c58e Mon Sep 17 00:00:00 2001
From: Mike Kravetz <mike.kravetz@oracle.com>
Date: Mon, 16 Oct 2023 19:55:49 -0700
Subject: [PATCH] hugetlb: check for hugetlb folio before vmemmap_restore

In commit d8f5f7e445f0 ("hugetlb: set hugetlb page flag before
optimizing vmemmap") checks were added to print a warning if
hugetlb_vmemmap_restore was called on a non-hugetlb page.  This
was mostly due to ordering issues in the hugetlb page set up and
 tear down sequencees.  One place missed was the routine
dissolve_free_huge_page.  Naoya Horiguchi noted: "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()."

Perform the check as suggested by Naoya.

Fixes: d8f5f7e445f0 ("hugetlb: set hugetlb page flag before optimizing vmemmap")
Suggested-by: Naoya Horiguchi <naoya.horiguchi@linux.dev>
Signed-off-by: Mike Kravetz <mike.kravetz@oracle.com>
---
 mm/hugetlb.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 36b40bc9ac25..13736cbb2c19 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -2290,17 +2290,23 @@ int dissolve_free_huge_page(struct page *page)
 		 * need to adjust max_huge_pages if the page is not freed.
 		 * Attempt to allocate vmemmmap here so that we can take
 		 * appropriate action on failure.
+		 *
+		 * The folio_test_hugetlb check here is because
+		 * remove_hugetlb_folio will clear hugetlb folio flag for
+		 * non-vmemmap optimized hugetlb folios.
 		 */
-		rc = hugetlb_vmemmap_restore(h, &folio->page);
-		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(h, &folio->page);
+			if (rc) {
+				spin_lock_irq(&hugetlb_lock);
+				add_hugetlb_folio(h, folio, false);
+				h->max_huge_pages++;
+				goto out;
+			}
+		} else
+			rc = 0;
 
+		update_and_free_hugetlb_folio(h, folio, false);
 		return rc;
 	}
 out:
-- 
2.41.0