diff mbox series

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

Message ID 20240708031517.856-1-justinjiang@vivo.com (mailing list archive)
State New
Headers show
Series [v4] mm: shrink skip folio mapped by an exiting task | expand

Commit Message

zhiguojiang July 8, 2024, 3:15 a.m. UTC
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 cpu load of the tasks exiting process.

Signed-off-by: Qianfeng Rong <rongqianfeng@vivo.corp-partner.google.com>
---

Change log:
v3->v4:
1.Modify that the unshared folios mapped only in exiting task are skip.
v2->v3:
Nothing.
v1->v2:
1.The VM_EXITING added in v1 patch is removed, because it will fail
to compile in 32-bit system.

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

Comments

Matthew Wilcox (Oracle) July 8, 2024, 3:34 a.m. UTC | #1
On Mon, Jul 08, 2024 at 11:15:17AM +0800, 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.

How is the file folio similar?  File folios are never written to swap,
and they'll be written back from the page cache whenever the filesystem
decides it's a good time to do so.

>  mm/rmap.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>  mode change 100644 => 100755 mm/rmap.c

Uh, what?  Why would you make this file executable?

> diff --git a/mm/rmap.c b/mm/rmap.c
> index 26806b49a86f..16b7ed04bcbe
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -843,6 +843,16 @@ static bool folio_referenced_one(struct folio *folio,
>  	int referenced = 0;
>  	unsigned long start = address, ptes = 0;
>  
> +	/* Skip the unshared folios mapped only by the single
> +	 * exiting process.
> +	 */

Comments start with a /* on a line by itself.

> +	if ((!atomic_read(&vma->vm_mm->mm_users) ||
> +		test_bit(MMF_OOM_SKIP, &vma->vm_mm->flags)) &&
> +		!test_bit(VM_SHARED, &vma->vm_flags)) {
> +		pra->referenced = -1;
> +		return false;

This indentation is unreadable.  Follow the style used in the rest of
the file.
Barry Song July 8, 2024, 3:34 a.m. UTC | #2
On Mon, Jul 8, 2024 at 3:15 PM Zhiguo Jiang <justinjiang@vivo.com> 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 cpu load of the tasks exiting process.
>
> Signed-off-by: Qianfeng Rong <rongqianfeng@vivo.corp-partner.google.com>
> ---
>
> Change log:
> v3->v4:
> 1.Modify that the unshared folios mapped only in exiting task are skip.
> v2->v3:
> Nothing.
> v1->v2:
> 1.The VM_EXITING added in v1 patch is removed, because it will fail
> to compile in 32-bit system.
>
> Signed-off-by: Zhiguo Jiang <justinjiang@vivo.com>
> ---
>  mm/rmap.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>  mode change 100644 => 100755 mm/rmap.c
>
> diff --git a/mm/rmap.c b/mm/rmap.c
> index 26806b49a86f..16b7ed04bcbe
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -843,6 +843,16 @@ static bool folio_referenced_one(struct folio *folio,
>         int referenced = 0;
>         unsigned long start = address, ptes = 0;
>
> +       /* Skip the unshared folios mapped only by the single
> +        * exiting process.
> +        */
> +       if ((!atomic_read(&vma->vm_mm->mm_users) ||
> +               test_bit(MMF_OOM_SKIP, &vma->vm_mm->flags)) &&
> +               !test_bit(VM_SHARED, &vma->vm_flags)) {

I don't think this is correct. folio_likely_mapped_shared() is almost "correct"
but not always.

> +               pra->referenced = -1;

Please explain why you set  pra->referenced =  -1. Please address all
comments before you send a new version.

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

Thanks
Barry
zhiguojiang July 8, 2024, 4:52 a.m. UTC | #3
在 2024/7/8 11:34, Matthew Wilcox 写道:
> On Mon, Jul 08, 2024 at 11:15:17AM +0800, 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.
> How is the file folio similar?  File folios are never written to swap,
> and they'll be written back from the page cache whenever the filesystem
> decides it's a good time to do so.
Hi Matthew Wilcox,

What do you mean is that the file folio will not have any relevant 
identifier left
in memory after it is reclamed in the shrink flow, and it will not be 
released
again during an exiting process? If that's the case, I think we only 
need the
anon folio is skipped here.

Thanks
Zhiguo
>
>>   mm/rmap.c | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>   mode change 100644 => 100755 mm/rmap.c
> Uh, what?  Why would you make this file executable?
>
>> diff --git a/mm/rmap.c b/mm/rmap.c
>> index 26806b49a86f..16b7ed04bcbe
>> --- a/mm/rmap.c
>> +++ b/mm/rmap.c
>> @@ -843,6 +843,16 @@ static bool folio_referenced_one(struct folio *folio,
>>   	int referenced = 0;
>>   	unsigned long start = address, ptes = 0;
>>   
>> +	/* Skip the unshared folios mapped only by the single
>> +	 * exiting process.
>> +	 */
> Comments start with a /* on a line by itself.
>
>> +	if ((!atomic_read(&vma->vm_mm->mm_users) ||
>> +		test_bit(MMF_OOM_SKIP, &vma->vm_mm->flags)) &&
>> +		!test_bit(VM_SHARED, &vma->vm_flags)) {
>> +		pra->referenced = -1;
>> +		return false;
> This indentation is unreadable.  Follow the style used in the rest of
> the file.
Ok, thanks.
>
zhiguojiang July 8, 2024, 9:25 a.m. UTC | #4
在 2024/7/8 11:34, Matthew Wilcox 写道:
> On Mon, Jul 08, 2024 at 11:15:17AM +0800, 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.
> How is the file folio similar?  File folios are never written to swap,
> and they'll be written back from the page cache whenever the filesystem
> decides it's a good time to do so.
>
>>   mm/rmap.c | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>   mode change 100644 => 100755 mm/rmap.c
> Uh, what?  Why would you make this file executable?
>
>> diff --git a/mm/rmap.c b/mm/rmap.c
>> index 26806b49a86f..16b7ed04bcbe
>> --- a/mm/rmap.c
>> +++ b/mm/rmap.c
>> @@ -843,6 +843,16 @@ static bool folio_referenced_one(struct folio *folio,
>>   	int referenced = 0;
>>   	unsigned long start = address, ptes = 0;
>>   
>> +	/* Skip the unshared folios mapped only by the single
>> +	 * exiting process.
>> +	 */
> Comments start with a /* on a line by itself.
>
>> +	if ((!atomic_read(&vma->vm_mm->mm_users) ||
>> +		test_bit(MMF_OOM_SKIP, &vma->vm_mm->flags)) &&
>> +		!test_bit(VM_SHARED, &vma->vm_flags)) {
>> +		pra->referenced = -1;
>> +		return false;
> This indentation is unreadable.  Follow the style used in the rest of
> the file.
Update in patch v5.
https://lore.kernel.org/linux-mm/20240708090413.888-1-justinjiang@vivo.com/
Thanks
>
diff mbox series

Patch

diff --git a/mm/rmap.c b/mm/rmap.c
index 26806b49a86f..16b7ed04bcbe
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -843,6 +843,16 @@  static bool folio_referenced_one(struct folio *folio,
 	int referenced = 0;
 	unsigned long start = address, ptes = 0;
 
+	/* Skip the unshared folios mapped only by the single
+	 * exiting process.
+	 */
+	if ((!atomic_read(&vma->vm_mm->mm_users) ||
+		test_bit(MMF_OOM_SKIP, &vma->vm_mm->flags)) &&
+		!test_bit(VM_SHARED, &vma->vm_flags)) {
+		pra->referenced = -1;
+		return false;
+	}
+
 	while (page_vma_mapped_walk(&pvmw)) {
 		address = pvmw.address;