[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240627082941.GA21813@redhat.com>
Date: Thu, 27 Jun 2024 10:29:42 +0200
From: Oleg Nesterov <oleg@...hat.com>
To: Michal Hocko <mhocko@...e.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
Christian Brauner <brauner@...nel.org>,
"Eric W. Biederman" <ebiederm@...ssion.com>,
Jens Axboe <axboe@...nel.dk>,
Jinliang Zheng <alexjlzheng@...cent.com>,
Mateusz Guzik <mjguzik@...il.com>,
Matthew Wilcox <willy@...radead.org>,
Tycho Andersen <tandersen@...flix.com>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] memcg: mm_update_next_owner: kill the "retry" logic
Michal, thanks for looking at this,
On 06/27, Michal Hocko wrote:
>
> On Wed 26-06-24 17:29:24, Oleg Nesterov wrote:
> > @@ -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;
>
> You need to unlock tasklist_lock, right? Same for other goto ret.
No. From the patch
+/* 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);
^^^^^^^^^^^^^^^^^^^^^^^^^^^
try_to_set_owner() drops tasklist right after it verifies that
tsk->mm == mm under task_lock().
> It should still die but it can do so in a better shape.
Agreed!
Oleg.
Powered by blists - more mailing lists