Re: [PATCH v3 07/11] mm: do not split a folio if it has minimum folio order requirement

[Zi Yan]

On 26 Mar 2024, at 12:33, Pankaj Raghav wrote:

> On 26/03/2024 17:23, Zi Yan wrote:
>> On 26 Mar 2024, at 12:10, Pankaj Raghav (Samsung) wrote:
>>
>>> On Mon, Mar 25, 2024 at 07:06:04PM +0000, Matthew Wilcox wrote:
>>>> On Wed, Mar 13, 2024 at 06:02:49PM +0100, Pankaj Raghav (Samsung) wrote:
>>>>> From: Pankaj Raghav <p.raghav@xxxxxxxxxxx>
>>>>>
>>>>> As we don't have a way to split a folio to a any given lower folio
>>>>> order yet, avoid splitting the folio in split_huge_page_to_list() if it
>>>>> has a minimum folio order requirement.
>>>>
>>>> FYI, Zi Yan's patch to do that is now in Andrew's tree.
>>>> c010d47f107f609b9f4d6a103b6dfc53889049e9 in current linux-next (dated
>>>> Feb 26)
>>>
>>> Yes, I started playing with the patches but I am getting a race condition
>>> resulting in a null-ptr-deref for which I don't have a good answer for
>>> yet.
>>>
>>> @zi yan Did you encounter any issue like this when you were testing?
>>>
>>> I did the following change (just a prototype) instead of this patch:
>>>
>>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>>> index 9859aa4f7553..63ee7b6ed03d 100644
>>> --- a/mm/huge_memory.c
>>> +++ b/mm/huge_memory.c
>>> @@ -3041,6 +3041,10 @@ int split_huge_page_to_list_to_order(struct page *page, struct list_head *list,
>>>  {
>>>         struct folio *folio = page_folio(page);
>>>         struct deferred_split *ds_queue = get_deferred_split_queue(folio);
>>> +       unsigned int mapping_min_order = mapping_min_folio_order(folio->mapping);
>>
>> I am not sure if this is right. Since folio can be anonymous and folio->mapping
>> does not point to struct address_space.
>>
>>> +
>>> +       if (!folio_test_anon(folio))
>
> Hmm, but I update the new_order only if it is not anonymous. Do you think it is still
> wrong?

For anonymous folio, folio->mapping has last bit set and point to a possible struct anon_vma. I do not know what ->flag will be or if it is accessible in that case.
See: https://elixir.bootlin.com/linux/latest/source/include/linux/page-flags.h#L608

--
Best Regards,
Yan, Zi

Attachment: signature.asc
Description: OpenPGP digital signature