Message-ID: <00965028-b754-4959-b711-58ccb2e5e940@vivo.com>
Date: Mon, 8 Jul 2024 17:24:18 +0800
From: zhiguojiang <justinjiang@...o.com>
To: Barry Song <baohua@...nel.org>
Cc: Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org,
 linux-kernel@...r.kernel.org, opensource.kernel@...o.com
Subject: Re: [PATCH] mm: shrink skip folio mapped by an exiting task



在 2024/7/8 11:54, Barry Song 写道:
> [Some people who received this message don't often get email from baohua@...nel.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> On Mon, Jul 8, 2024 at 3:40 PM zhiguojiang <justinjiang@...o.com> wrote:
>>
>>
>> 在 2024/6/30 17:35, Barry Song 写道:
>>> [Some people who received this message don't often get email from baohua@...nel.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>>>
>>> On Thu, Feb 22, 2024 at 12:49 AM Zhiguo Jiang <justinjiang@...o.com> wrote:
>>> This is clearly version 3, as you previously sent version 2, correct?
>> Hi Barry,
>>
>> Yes, patchs are as follows:
>> v3:
>> https://lore.kernel.org/linux-mm/20240221114904.1849-1-justinjiang@vivo.com/
>> v2:
>> https://lore.kernel.org/linux-mm/20240202012251.1aa5afbfdf2f8b3a862bced3@linux-foundation.org/
>> v1:
>> https://lore.kernel.org/linux-mm/20240124124308.461-1-justinjiang@vivo.com/
>>>> If an anon folio reclaimed by shrink_inactive_list is mapped by an
>>>> exiting task, this anon folio will be firstly swaped-out into
>>>> swapspace in shrink flow and then this swap folio is freed in task
>>>> exit flow. But if this folio mapped by an exiting task can skip
>>>> shrink and be freed directly in task exiting flow, which will save
>>>> swap-out time and alleviate the load of the tasks exiting process.
>>>> The file folio is also similar.
>>>>
>>> Could you please describe the specific impact on users, including user
>>> experience and power consumption? How serious is this problem?
>> 1.At present, I do not have a suitable method to accurately measure the
>> optimization benefit datas of this modifications, but I believe it
>> theoretically
>> has some benefits.
>> 2.Launching large memory app (for example, starting the camera) in multiple
>> backend scenes may result in the high cpu load of exiting processes.
>>>> And when system is low memory, it more likely to occur, because more
>>>> backend applidatuions will be killed.
>>> Applications?
>> Yes.
>>>> This patch can alleviate the load of the tasks exiting process.
>>> I'm not completely convinced this patch is correct, but it appears to be
>>> heading in the right direction. Therefore, I expect to see new versions
>>> rather than it being dead.
>>>
>>>> Signed-off-by: Zhiguo Jiang <justinjiang@...o.com>
>>>> ---
>>>>    mm/rmap.c | 7 +++++++
>>>>    1 file changed, 7 insertions(+)
>>>>    mode change 100644 => 100755 mm/rmap.c
>>> You changed the file mode to 755, which is incorrect.
>>>
>>>> diff --git a/mm/rmap.c b/mm/rmap.c
>>>> index 3746a5531018..146e5f4ec069
>>>> --- a/mm/rmap.c
>>>> +++ b/mm/rmap.c
>>>> @@ -840,6 +840,13 @@ static bool folio_referenced_one(struct folio *folio,
>>>>           int referenced = 0;
>>>>           unsigned long start = address, ptes = 0;
>>>>
>>>> +       /* Skip this folio if it's mapped by an exiting task */
>>>> +       if (unlikely(!atomic_read(&vma->vm_mm->mm_users)) ||
>>>> +               unlikely(test_bit(MMF_OOM_SKIP, &vma->vm_mm->flags))) {
>>>> +               pra->referenced = -1;
>>> Why use -1? Is this meant to simulate lock contention to keep the folio without
>>> activating it?
>>>
>>>           /* rmap lock contention: rotate */
>>>           if (referenced_ptes == -1)
>>>                   return FOLIOREF_KEEP;
>>>
>>> Please do have some comments to explain why.
>>>
>>> I'm not convinced this change is appropriate for shared folios. It seems
>>> more suitable for exclusive folios used solely by the exiting process.
>> 1.The skiped folios are FOLIOREF_KEEP and added into inactive lru, beacase
>> the folios will be freed soon in the exiting process flow.
> This requires an explicit comment in the code. The expression
> referenced_ptes == -1
> implies "rmap lock contention: rotate", but it appears that this is
> not the case here. You
> should either update the original comment to reflect the current logic
> or use a different
> value.
Ok, this has been updated in patch v5.
https://lore.kernel.org/linux-mm/20240708090413.888-1-justinjiang@vivo.com/
Thanks
>
>> 2.Yes, the shared folios can not be simply skipped. I have made relevant
>> modifications in patch v4 and please help to further review.
>> https://lore.kernel.org/linux-mm/20240708031517.856-1-justinjiang@vivo.com/
>>
>> Thanks
>> Zhiguo
>>>
>>>> +               return false;
>>>> +       }
>>>> +
>>>>           while (page_vma_mapped_walk(&pvmw)) {
>>>>                   address = pvmw.address;
>>>>
>>>> --
>>>> 2.39.0
>>>>
>>> Thanks
>>> Barry

Powered by blists - more mailing lists