Message-ID: <7964fa25-0ba6-43b5-b215-4e381a355a08@vivo.com>
Date: Mon, 8 Jul 2024 11:40:19 +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/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.
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