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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAGsJ_4ytMMbPo4EuNSQ-A75T+5h3O+nM127xcMg+QpKjf5D8Sg@mail.gmail.com>
Date: Sun, 30 Jun 2024 21:35:15 +1200
From: Barry Song <baohua@...nel.org>
To: Zhiguo Jiang <justinjiang@...o.com>
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

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?

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


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

Thanks
Barry