On 8/3/2022 9:01 PM, Matthew Wilcox wrote:
> On Wed, Aug 03, 2022 at 10:52:43AM +0800, Yin Fengwei wrote:
>> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
>> index da39ec8afca8..08e21973b120 100644
>> --- a/mm/memory-failure.c
>> +++ b/mm/memory-failure.c
>> @@ -1484,7 +1484,16 @@ static int identify_page_state(unsigned long pfn, struct page *p,
>>
>> static int try_to_split_thp_page(struct page *page, const char *msg)
>> {
>> + struct page *head = compound_head(page);
> > +
>> lock_page(page);
>> + /*
>> + * If thp page has private data attached, thp split will fail.
>> + * Release private data before split thp.
>> + */
>> + if (page_has_private(head))
>> + try_to_release_page(head, GFP_KERNEL);
>> +
>> if (unlikely(split_huge_page(page))) {
>> unsigned long pfn = page_to_pfn(page);
>
> It seems a shame to use the old page approach instead of the
> shiny new folio approach. We're quite close to being able to remove
> try_to_release_page() in 6.1 or 6.2 so adding a new caller is a bad idea.
> How about this:
I am not aware try_to_release_page() was on remove plan. Yes. New API
is good.
>
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index da39ec8afca8..765b383288b1 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -1484,16 +1484,21 @@ static int identify_page_state(unsigned long pfn, struct page *p,
>
> static int try_to_split_thp_page(struct page *page, const char *msg)
> {
> - lock_page(page);
> + struct folio *folio = page_folio(page);
> +
> + folio_lock(folio);
> + if (folio_test_private(folio))
> + filemap_release_folio(folio, GFP_KERNEL);> if (unlikely(split_huge_page(page))) {
> unsigned long pfn = page_to_pfn(page);
>
> - unlock_page(page);
> + folio_unlock(folio);
> pr_info("%s: %#lx: thp split failed\n", msg, pfn);
> - put_page(page);
> + folio_put(folio);
> return -EBUSY;
> }
> - unlock_page(page);
> + folio = page_folio(page);
Already got page folio. I suppose don't need above line.
I will re-run the test with the new folio API based patch.
Regards
Yin, Fengwei
> + folio_unlock(folio);
>
> return 0;
> }
