Received-SPF: pass (google.com: domain of linux-kernel+bounces-146174-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Subject: Re: [PATCH v2] mm/memory-failure: fix deadlock when
 hugetlb_optimize_vmemmap is enabled
To: Andrew Morton <akpm@linux-foundation.org>
CC: Sidhartha Kumar <sidhartha.kumar@oracle.com>, <naoya.horiguchi@nec.com>,
	<osalvador@suse.de>, <linux-mm@kvack.org>, <linux-kernel@vger.kernel.org>
References: <20240412025754.1897615-1-linmiaohe@huawei.com>
 <48647e5b-d15b-457b-9879-fb1b6bbaee27@oracle.com>
 <8d186776-f3b1-5d9a-2f94-fa249dee7d5f@huawei.com>
 <20240412162111.10f67ad0f001734464b53ad8@linux-foundation.org>
From: Miaohe Lin <linmiaohe@huawei.com>
Message-ID: <e18fd131-2a31-48a6-d889-0a03ad1ffd38@huawei.com>
Date: Tue, 16 Apr 2024 11:23:01 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101
 Thunderbird/78.6.0
Precedence: bulk
MIME-Version: 1.0
In-Reply-To: <20240412162111.10f67ad0f001734464b53ad8@linux-foundation.org>
Content-Type: text/plain; charset="utf-8"
Content-Language: en-US
Content-Transfer-Encoding: 7bit

On 2024/4/13 7:21, Andrew Morton wrote:
> On Fri, 12 Apr 2024 16:11:52 +0800 Miaohe Lin <linmiaohe@huawei.com> wrote:
> 
>>> I recently sent a patch[1] to convert dissolve_free_huge_page() to folios which changes the function name and the name referenced in the comment so this will conflict with my patch. It's in mm-unstable now, would you be able to rebase to that in a new version?
>>>
> 
> This patch is a hotfixes, cc:stable one so the mm-unstable material will be
> based on top of this change.
> 
> I've queued this change up as a -fix against v1.  And I've retained
> this changelog addition:
> 
> : This issue won't occur until commit a6b40850c442 ("mm: hugetlb: replace
> : hugetlb_free_vmemmap_enabled with a static_key").  As it introduced
> : rlock(cpu_hotplug_lock) in dissolve_free_huge_page() code path while
> : lock(pcp_batch_high_lock) is already in the __page_handle_poison().
> 
> And I've queued another -fix to reflow that block comment to 80 columns.
> 
> --- a/mm/memory-failure.c~mm-memory-failure-fix-deadlock-when-hugetlb_optimize_vmemmap-is-enabled-v2-fix
> +++ a/mm/memory-failure.c
> @@ -155,14 +155,16 @@ static int __page_handle_poison(struct p
>  	int ret;
>  
>  	/*
> -	 * zone_pcp_disable() can't be used here. It will hold pcp_batch_high_lock and
> -	 * dissolve_free_huge_page() might hold cpu_hotplug_lock via static_key_slow_dec()
> -	 * when hugetlb vmemmap optimization is enabled. This will break current lock
> -	 * dependency chain and leads to deadlock.
> -	 * Disabling pcp before dissolving the page was a deterministic approach because
> -	 * we made sure that those pages cannot end up in any PCP list. Draining PCP lists
> -	 * expels those pages to the buddy system, but nothing guarantees that those pages
> -	 * do not get back to a PCP queue if we need to refill those.
> +	 * zone_pcp_disable() can't be used here. It will
> +	 * hold pcp_batch_high_lock and dissolve_free_huge_page() might hold
> +	 * cpu_hotplug_lock via static_key_slow_dec() when hugetlb vmemmap
> +	 * optimization is enabled. This will break current lock dependency
> +	 * chain and leads to deadlock.
> +	 * Disabling pcp before dissolving the page was a deterministic
> +	 * approach because we made sure that those pages cannot end up in any
> +	 * PCP list. Draining PCP lists expels those pages to the buddy system,
> +	 * but nothing guarantees that those pages do not get back to a PCP
> +	 * queue if we need to refill those.
>  	 */
>  	ret = dissolve_free_huge_page(page);
>  	if (!ret) {
> _
> 

Many thanks for doing this. :)
.

> .
>