[PATCH] mm: shrink skip folio mapped by an exiting task

[PATCH] mm: shrink skip folio mapped by an exiting task

[Zhiguo Jiang]

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.

And when system is low memory, it more likely to occur, because more
backend applidatuions will be killed.

This patch can alleviate the load of the tasks exiting process.

Signed-off-by: Zhiguo Jiang <justinjiang@vivo.com>
---
 mm/rmap.c | 7 +++++++
 1 file changed, 7 insertions(+)
 mode change 100644 => 100755 mm/rmap.c

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;
+		return false;
+	}
+
 	while (page_vma_mapped_walk(&pvmw)) {
 		address = pvmw.address;
 
-- 
2.39.0

Re: [PATCH] mm: shrink skip folio mapped by an exiting task

[Barry Song]

On Thu, Feb 22, 2024 at 12:49 AM Zhiguo Jiang <justinjiang@vivo.com> wrote:
>

This is clearly version 3, as you previously sent version 2, correct?

> 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?

> And when system is low memory, it more likely to occur, because more
> backend applidatuions will be killed.

Applications?

>
> 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@vivo.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.


> +               return false;
> +       }
> +
>         while (page_vma_mapped_walk(&pvmw)) {
>                 address = pvmw.address;
>
> --
> 2.39.0
>

Thanks
Barry

Re: [PATCH] mm: shrink skip folio mapped by an exiting task

[zhiguojiang]

在 2024/6/30 17:35, Barry Song 写道:
> [Some people who received this message don't often get email from baohua@kernel.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> On Thu, Feb 22, 2024 at 12:49 AM Zhiguo Jiang <justinjiang@vivo.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@vivo.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

Re: [PATCH] mm: shrink skip folio mapped by an exiting task

[Barry Song]

On Mon, Jul 8, 2024 at 3:40 PM zhiguojiang <justinjiang@vivo.com> wrote:
>
>
>
> 在 2024/6/30 17:35, Barry Song 写道:
> > [Some people who received this message don't often get email from baohua@kernel.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> >
> > On Thu, Feb 22, 2024 at 12:49 AM Zhiguo Jiang <justinjiang@vivo.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@vivo.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.

> 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
>

Re: [PATCH] mm: shrink skip folio mapped by an exiting task

[zhiguojiang]

在 2024/7/8 11:54, Barry Song 写道:
> [Some people who received this message don't often get email from baohua@kernel.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> On Mon, Jul 8, 2024 at 3:40 PM zhiguojiang <justinjiang@vivo.com> wrote:
>>
>>
>> 在 2024/6/30 17:35, Barry Song 写道:
>>> [Some people who received this message don't often get email from baohua@kernel.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>>>
>>> On Thu, Feb 22, 2024 at 12:49 AM Zhiguo Jiang <justinjiang@vivo.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@vivo.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

Re: [PATCH] mm: shrink skip folio mapped by an exiting task

[David Hildenbrand]

On 21.02.24 12:49, Zhiguo Jiang wrote:
> 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.
> 
> And when system is low memory, it more likely to occur, because more
> backend applidatuions will be killed.
> 
> This patch can alleviate the load of the tasks exiting process.
> 
> Signed-off-by: Zhiguo Jiang <justinjiang@vivo.com>
> ---
>   mm/rmap.c | 7 +++++++
>   1 file changed, 7 insertions(+)
>   mode change 100644 => 100755 mm/rmap.c
> 
> 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;
> +		return false;
> +	}
> +
>   	while (page_vma_mapped_walk(&pvmw)) {
>   		address = pvmw.address;
>   

... but what if it is shared among multiple processes and only one of 
them is exiting?

-- 
Cheers,

David / dhildenb