From: Oleg Nesterov <oleg@redhat.com>
To: Andrew Morton <akpm@linux-foundation.org>,
Michal Hocko <mhocko@suse.com>
Cc: Christian Brauner <brauner@kernel.org>,
"Eric W. Biederman" <ebiederm@xmission.com>,
Jens Axboe <axboe@kernel.dk>,
Jinliang Zheng <alexjlzheng@tencent.com>,
Mateusz Guzik <mjguzik@gmail.com>,
Matthew Wilcox <willy@infradead.org>,
Tycho Andersen <tandersen@netflix.com>,
linux-kernel@vger.kernel.org
Subject: [PATCH 1/2] memcg: mm_update_next_owner: kill the "retry" logic
Date: Wed, 26 Jun 2024 17:29:24 +0200 [thread overview]
Message-ID: <20240626152924.GA17933@redhat.com> (raw)
In-Reply-To: <20240626152835.GA17910@redhat.com>
Add the new helper, try_to_set_owner(), which tries to update mm->owner
once we see c->mm == mm. This way mm_update_next_owner() doesn't need to
restart the list_for_each_entry/for_each_process loops from the very
beginning if it races with exit/exec, it can just continue.
Unlike the current code, try_to_set_owner() re-checks tsk->mm == mm
before it drops tasklist_lock, so it doesn't need get/put_task_struct().
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
kernel/exit.c | 57 ++++++++++++++++++++++++---------------------------
1 file changed, 27 insertions(+), 30 deletions(-)
diff --git a/kernel/exit.c b/kernel/exit.c
index 45210443e68d..a1ef5f23d5be 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -439,6 +439,23 @@ static void coredump_task_exit(struct task_struct *tsk)
}
#ifdef CONFIG_MEMCG
+/* drops tasklist_lock if succeeds */
+static bool try_to_set_owner(struct task_struct *tsk, struct mm_struct *mm)
+{
+ bool ret = false;
+
+ task_lock(tsk);
+ if (likely(tsk->mm == mm)) {
+ /* tsk can't pass exit_mm/exec_mmap and exit */
+ read_unlock(&tasklist_lock);
+ WRITE_ONCE(mm->owner, tsk);
+ lru_gen_migrate_mm(mm);
+ ret = true;
+ }
+ task_unlock(tsk);
+ return ret;
+}
+
/*
* A task is exiting. If it owned this mm, find a new owner for the mm.
*/
@@ -446,7 +463,6 @@ void mm_update_next_owner(struct mm_struct *mm)
{
struct task_struct *c, *g, *p = current;
-retry:
/*
* If the exiting or execing task is not the owner, it's
* someone else's problem.
@@ -468,16 +484,16 @@ void mm_update_next_owner(struct mm_struct *mm)
* Search in the children
*/
list_for_each_entry(c, &p->children, sibling) {
- if (c->mm == mm)
- goto assign_new_owner;
+ if (c->mm == mm && try_to_set_owner(c, mm))
+ goto ret;
}
/*
* Search in the siblings
*/
list_for_each_entry(c, &p->real_parent->children, sibling) {
- if (c->mm == mm)
- goto assign_new_owner;
+ if (c->mm == mm && try_to_set_owner(c, mm))
+ goto ret;
}
/*
@@ -489,9 +505,11 @@ void mm_update_next_owner(struct mm_struct *mm)
if (g->flags & PF_KTHREAD)
continue;
for_each_thread(g, c) {
- if (c->mm == mm)
- goto assign_new_owner;
- if (c->mm)
+ struct mm_struct *c_mm = READ_ONCE(c->mm);
+ if (c_mm == mm) {
+ if (try_to_set_owner(c, mm))
+ goto ret;
+ } else if (c_mm)
break;
}
}
@@ -502,30 +520,9 @@ void mm_update_next_owner(struct mm_struct *mm)
* ptrace or page migration (get_task_mm()). Mark owner as NULL.
*/
WRITE_ONCE(mm->owner, NULL);
+ ret:
return;
-assign_new_owner:
- BUG_ON(c == p);
- get_task_struct(c);
- /*
- * The task_lock protects c->mm from changing.
- * We always want mm->owner->mm == mm
- */
- task_lock(c);
- /*
- * Delay read_unlock() till we have the task_lock()
- * to ensure that c does not slip away underneath us
- */
- read_unlock(&tasklist_lock);
- if (c->mm != mm) {
- task_unlock(c);
- put_task_struct(c);
- goto retry;
- }
- WRITE_ONCE(mm->owner, c);
- lru_gen_migrate_mm(mm);
- task_unlock(c);
- put_task_struct(c);
}
#endif /* CONFIG_MEMCG */
--
2.25.1.362.g51ebf55
next prev parent reply other threads:[~2024-06-26 15:31 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-06-26 15:28 [PATCH -mm 0/2] memcg: deuglify mm_update_next_owner() Oleg Nesterov
2024-06-26 15:29 ` Oleg Nesterov [this message]
2024-06-27 7:56 ` [PATCH 1/2] memcg: mm_update_next_owner: kill the "retry" logic Michal Hocko
2024-06-27 8:29 ` Oleg Nesterov
2024-06-27 9:02 ` Michal Hocko
2024-06-26 15:29 ` [PATCH 2/2] memcg: mm_update_next_owner: move for_each_thread() into try_to_set_owner() Oleg Nesterov
2024-06-27 9:02 ` Michal Hocko
2024-06-26 19:24 ` [PATCH -mm 0/2] memcg: deuglify mm_update_next_owner() Andrew Morton
2024-06-26 20:27 ` Oleg Nesterov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240626152924.GA17933@redhat.com \
--to=oleg@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=alexjlzheng@tencent.com \
--cc=axboe@kernel.dk \
--cc=brauner@kernel.org \
--cc=ebiederm@xmission.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mhocko@suse.com \
--cc=mjguzik@gmail.com \
--cc=tandersen@netflix.com \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®