Re: [PATCH v3] mm: release private data before split THP

[Yin Fengwei <fengwei.yin@intel.com>]

On 2022/8/11 01:09, Yang Shi wrote:
> On Tue, Aug 9, 2022 at 11:50 PM Yin Fengwei <fengwei.yin@intel.com> wrote:
>>
>> If there is private data attached to THP, the refcount of
>> THP will be increased and block the THP split. Release
>> private data attached to THP before split it to increase
>> the chance of splitting THP successfully.
>>
>> There was a memory failure issue hit during HW error
>> injection testing with 5.18 kernel + xfs as rootfs. Test
>> got killed and system reboot was required to re-run the
>> test.
>>
>> The issue was tracked down to THP split failure caused the
>> memory failure not being handled. The page dump showed:
>>
>> [ 1785.433075] page:0000000025f9530b refcount:18 mapcount:0 mapping:000000008162eea7 index:0xa10 pfn:0x2f0200
>> [ 1785.443954] head:0000000025f9530b order:4 compound_mapcount:0 compound_pincount:0
>> [ 1785.452408] memcg:ff4247f2d28e9000
>> [ 1785.456304] aops:xfs_address_space_operations ino:8555182 dentry name:"baseos-filenames.solvx"
>> [ 1785.466612] flags: 0x1000000000012036(referenced|uptodate|lru|active|private|head|node=0|zone=2)
>> [ 1785.476514] raw: 1000000000012036 ffb9460f8bc07c08 ffb9460f8bc08408 ff4247f22e6299f8
>> [ 1785.485268] raw: 0000000000000a10 ff4247f194ade900 00000012ffffffff ff4247f2d28e9000
>>
>> It was like the error was injected to a large folio for xfs
>> with private data attached.
>>
>> With private data released before split THP, the test case
>> could be run successfully many times without reboot system.
>>
>> Co-developed-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com>
>> Signed-off-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com>
>> Signed-off-by: Yin Fengwei <fengwei.yin@intel.com>
>> Suggested-by: Matthew Wilcox <willy@infradead.org>
>> ---
>> Changelog from v2:
>>  - Use safe gfp flags for different callsite of split_huge_page_to_list
>>    per Yang's comment.
>>  - Remove reviewed-by tag from Aaron which was only valid for RFC patch
>>    but keep it by mistake.
> 
> Reviewed-by: Yang Shi <shy828301@gmail.com>
Thanks for your reviewing.

Regards
Yin, Fengwei

> 
>>
>> Changelog from v1:
>>  - Move private release to split_huge_page_to_list
>>    to cover wider path per Yang's comment.
>>  - Update to commit message.
>>
>> Changelog from RFC:
>>  - Use new folio API per Mathhew Wilcox's suggestion.
>>  - Add one line comment before re-get folio of page per
>>    Miaohe's comment.
>>  - Remove RFC tag
>>  - Add Co-developed-by of Qiuxu who did a lot of debugging
>>    work to locate where the real issue is.
>>  mm/huge_memory.c | 14 ++++++++++++--
>>  1 file changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>> index 8a7c1b344abe..ae8c4e209e58 100644
>> --- a/mm/huge_memory.c
>> +++ b/mm/huge_memory.c
>> @@ -2627,6 +2627,8 @@ int split_huge_page_to_list(struct page *page, struct list_head *list)
>>                 mapping = NULL;
>>                 anon_vma_lock_write(anon_vma);
>>         } else {
>> +               gfp_t gfp;
>> +
>>                 mapping = head->mapping;
>>
>>                 /* Truncated ? */
>> @@ -2635,8 +2637,16 @@ int split_huge_page_to_list(struct page *page, struct list_head *list)
>>                         goto out;
>>                 }
>>
>> -               xas_split_alloc(&xas, head, compound_order(head),
>> -                               mapping_gfp_mask(mapping) & GFP_RECLAIM_MASK);
>> +               gfp = current_gfp_context(mapping_gfp_mask(mapping) &
>> +                                                       GFP_RECLAIM_MASK);
>> +
>> +               if (folio_test_private(folio) &&
>> +                               !filemap_release_folio(folio, gfp)) {
>> +                       ret = -EBUSY;
>> +                       goto out;
>> +               }
>> +
>> +               xas_split_alloc(&xas, head, compound_order(head), gfp);
>>                 if (xas_error(&xas)) {
>>                         ret = xas_error(&xas);
>>                         goto out;
>>
>> base-commit: d4252071b97d2027d246f6a82cbee4d52f618b47
>> --
>> 2.25.1
>>
>