On 2024/4/13 7:21, Andrew Morton wrote:
> On Fri, 12 Apr 2024 16:11:52 +0800 Miaohe Lin <linmiaohe@xxxxxxxxxx> 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. :)
.
> .
>
