| 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
| ||
|
Message-ID: <cf6c6d1f-3719-4744-b943-5c572d0d5fb4@vivo.com>
Date: Mon, 8 Jul 2024 21:11:31 +0800
From: zhiguojiang <justinjiang@...o.com>
To: Barry Song <baohua@...nel.org>
Cc: Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org,
LKML <linux-kernel@...r.kernel.org>, opensource.kernel@...o.com
Subject: Re: [PATCH v5] mm: shrink skip folio mapped by an exiting process
在 2024/7/8 20:41, Barry Song 写道:
>
>
> zhiguojiang <justinjiang@...o.com> 于 2024年7月9日周二 00:25写道：
>
>
>
> 在 2024/7/8 20:17, zhiguojiang 写道:
> >
> >
> > 在 2024/7/8 19:02, Barry Song 写道:
> >> On Mon, Jul 8, 2024 at 9:04 PM Zhiguo Jiang <justinjiang@...o.com>
> >> wrote:
> >>> The releasing process of the non-shared anonymous folio mapped
> >>> solely by
> >>> an exiting process may go through two flows: 1) the anonymous
> folio is
> >>> firstly is swaped-out into swapspace and transformed into a
> swp_entry
> >>> in shrink_folio_list; 2) then the swp_entry is released in the
> process
> >>> exiting flow. This will increase the cpu load of releasing a
> non-shared
> >>> anonymous folio mapped solely by an exiting process, because
> the folio
> >>> go through swap-out and the releasing the swapspace and swp_entry.
> >>>
> >>> When system is low memory, it is more likely to occur, because
> more
> >>> backend applidatuions will be killed.
> >>>
> >>> The modification is that shrink skips the non-shared anonymous
> folio
> >>> solely mapped by an exting process and the folio is only released
> >>> directly in the process exiting flow, which will save swap-out
> time
> >>> and alleviate the load of the process exiting.
> >>>
> >>> Signed-off-by: Zhiguo Jiang <justinjiang@...o.com>
> >>> ---
> >>>
> >>> Change log:
> >>> v4->v5:
> >>> 1.Modify to skip non-shared anonymous folio only.
> >>> 2.Update comments for pra->referenced = -1.
> >>> 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.
> >>>
> >>> mm/rmap.c | 13 +++++++++++++
> >>> mm/vmscan.c | 7 ++++++-
> >>> 2 files changed, 19 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/mm/rmap.c b/mm/rmap.c
> >>> index 26806b49a86f..5b5281d71dbb
> >>> --- a/mm/rmap.c
> >>> +++ b/mm/rmap.c
> >>> @@ -843,6 +843,19 @@ static bool folio_referenced_one(struct
> folio
> >>> *folio,
> >>> int referenced = 0;
> >>> unsigned long start = address, ptes = 0;
> >>>
> >>> + /*
> >>> + * Skip the non-shared anonymous folio mapped solely by
> >>> + * the single exiting process, and release it directly
> >>> + * in the process exiting.
> >>> + */
> >>> + if ((!atomic_read(&vma->vm_mm->mm_users) ||
> >>> + test_bit(MMF_OOM_SKIP, &vma->vm_mm->flags)) &&
> >>> + folio_test_anon(folio) &&
> >>> folio_test_swapbacked(folio) &&
> >>> + !folio_likely_mapped_shared(folio)) {
> >>> + pra->referenced = -1;
> >>> + return false;
> >>> + }
> >>> +
> >>> while (page_vma_mapped_walk(&pvmw)) {
> >>> address = pvmw.address;
> > Sure, I agree with your modification suggestions. This way,
> using PTL
> > indeed sure
> > that the folio is mapped by this process.
> > Thanks
> >> As David suggested, what about the below?
> >>
> >> @@ -883,6 +870,21 @@ static bool folio_referenced_one(struct folio
> >> *folio,
> >> continue;
> >> }
> >>
> >> + /*
> >> + * Skip the non-shared anonymous folio mapped
> solely by
> >> + * the single exiting process, and release it
> directly
> >> + * in the process exiting.
> >> + */
> >> + if ((!atomic_read(&vma->vm_mm->mm_users) ||
> >> + test_bit(MMF_OOM_SKIP,
> >> &vma->vm_mm->flags)) &&
> >> + folio_test_anon(folio) &&
> >> folio_test_swapbacked(folio) &&
> >> + !folio_likely_mapped_shared(folio)) {
> >> + pra->referenced = -1;
> >> + page_vma_mapped_walk_done(&pvmw);
> >> + return false;
> >> + }
> >> +
> >> if (pvmw.pte) {
> >> if (lru_gen_enabled() &&
> >> pte_young(ptep_get(pvmw.pte))) {
> >>
> >>
> >> By the way, I am not convinced that using test_bit(MMF_OOM_SKIP,
> >> &vma->vm_mm->flags) is
> >> correct (I think it is wrong). For example, global_init can
> >> directly have it:
> >> if (is_global_init(p)) {
> >> can_oom_reap = false;
> >> set_bit(MMF_OOM_SKIP, &mm->flags);
> >> pr_info("oom killer %d (%s) has mm
> pinned by
> >> %d (%s)\n",
> >> task_pid_nr(victim),
> >> victim->comm,
> >> task_pid_nr(p), p->comm);
> >> continue;
> >> }
> >>
> >> And exit_mmap() automatically has MMF_OOM_SKIP.
> >>
> >> What is the purpose of this check? Is there a better way to
> determine
> >> if a process is an
> >> OOM target? What about check_stable_address_space() ?
> > 1.Sorry, I overlook the situation with if (is_global_init(p)),
> > MMF_OOM_SKIP is indeed not suitable.
> >
> > 2.check_stable_address_space() can indicate oom_reaper, but it
> seems
> > unable to identify the situation where the process exits normally.
> > What about task_is_dying()? static inline bool
> task_is_dying(void) {
> > return tsk_is_oom_victim(current) ||
> fatal_signal_pending(current) ||
> > (current->flags & PF_EXITING); } Thanks
> We can migrate task_is_dying() from mm/memcontrol.c to
> include/linux/oom.h
> > static inline bool task_is_dying(void)
> > {
> > return tsk_is_oom_victim(current) ||
> fatal_signal_pending(current) ||
> > (current->flags & PF_EXITING);
> > }
>
>
> no. current is kswapd.
Hi Barry,
It seems feasible for check_stable_address_space() replacing MMF_OOM_SKIP.
check_stable_address_space() can indicate oom kill, and
!atomic_read(&vma->vm_mm->mm_users)
can indicate the normal process exiting.
/*
* Skip the non-shared anonymous folio mapped solely by
* the single exiting process, and release it directly
* in the process exiting.
*/
if ((!atomic_read(&vma->vm_mm->mm_users) ||
check_stable_address_space(vma->vm_mm)) &&
folio_test_anon(folio) && folio_test_swapbacked(folio) &&
!folio_likely_mapped_shared(folio)) {
pra->referenced = -1;
page_vma_mapped_walk_done(&pvmw);
return false;
}
Thanks
Zhiguo
>
>
> >>
> >>
> >>> diff --git a/mm/vmscan.c b/mm/vmscan.c
> >>> index 0761f91b407f..bae7a8bf6b3d
> >>> --- a/mm/vmscan.c
> >>> +++ b/mm/vmscan.c
> >>> @@ -863,7 +863,12 @@ static enum folio_references
> >>> folio_check_references(struct folio *folio,
> >>> if (vm_flags & VM_LOCKED)
> >>> return FOLIOREF_ACTIVATE;
> >>>
> >>> - /* rmap lock contention: rotate */
> >>> + /*
> >>> + * There are two cases to consider.
> >>> + * 1) Rmap lock contention: rotate.
> >>> + * 2) Skip the non-shared anonymous folio mapped solely by
> >>> + * the single exiting process.
> >>> + */
> >>> if (referenced_ptes == -1)
> >>> return FOLIOREF_KEEP;
> >>>
> >>> --
> >>> 2.39.0
> >>>
> >> Thanks
> >> Barry
> >
>
Powered by blists - more mailing lists