linux-kernel - Re: [PATCH 1/1] mm, oom_adj: don't loop through tasks in __set_oom

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]

Date:   Fri, 21 Aug 2020 09:17:30 +0200
From:   Michal Hocko <mhocko@...e.com>
To:     "Eric W. Biederman" <ebiederm@...ssion.com>
Cc:     Suren Baghdasaryan <surenb@...gle.com>,
        Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>,
        Christian Brauner <christian.brauner@...ntu.com>,
        Tim Murray <timmurray@...gle.com>, mingo@...nel.org,
        Peter Zijlstra <peterz@...radead.org>,
        Thomas Gleixner <tglx@...utronix.de>, esyr@...hat.com,
        christian@...lner.me, areber@...hat.com,
        Shakeel Butt <shakeelb@...gle.com>, cyphar@...har.com,
        Oleg Nesterov <oleg@...hat.com>, adobriyan@...il.com,
        Andrew Morton <akpm@...ux-foundation.org>,
        gladkov.alexey@...il.com, Michel Lespinasse <walken@...gle.com>,
        daniel.m.jordan@...cle.com, avagin@...il.com,
        bernd.edlinger@...mail.de,
        John Johansen <john.johansen@...onical.com>,
        laoar.shao@...il.com, Minchan Kim <minchan@...nel.org>,
        kernel-team <kernel-team@...roid.com>,
        LKML <linux-kernel@...r.kernel.org>,
        linux-fsdevel@...r.kernel.org, linux-mm <linux-mm@...ck.org>
Subject: Re: [PATCH 1/1] mm, oom_adj: don't loop through tasks in
 __set_oom_adj when not necessary

On Thu 20-08-20 23:39:25, Eric W. Biederman wrote:
> Michal Hocko <mhocko@...e.com> writes:
> 
> > On Thu 20-08-20 08:56:53, Suren Baghdasaryan wrote:
> > [...]
> >> Catching up on the discussion which was going on while I was asleep...
> >> So it sounds like there is a consensus that oom_adj should be moved to
> >> mm_struct rather than trying to synchronize it among tasks sharing mm.
> >> That sounds reasonable to me too. Michal answered all the earlier
> >> questions about this patch, so I won't be reiterating them, thanks
> >> Michal. If any questions are still lingering about the original patch
> >> I'll be glad to answer them.
> >
> > I think it still makes some sense to go with a simpler (aka less tricky)
> > solution which would be your original patch with an incremental fix for
> > vfork and the proper ordering (http://lkml.kernel.org/r/20200820124109.GI5033@dhcp22.suse.cz)
> > and then make a more complex shift to mm struct on top of that. The
> > former will be less tricky to backport to stable IMHO.
> 
> So I am confused.
> 
> I don't know how a subtle dependency on something in clone
> is better than something flat footed in exec.

Well, I think that setting a flag is an easier approach than handle all
the special cases for the mm thing. But this is likely because this is
not my domain so my judgment call might be misled. Anyway if there is a
general consensus that doing the middle step is not worth it I am not
going to object.

> That said if we are going for a small change why not:
> 
> 	/*
> 	 * Make sure we will check other processes sharing the mm if this is
> 	 * not vfrok which wants its own oom_score_adj.
> 	 * pin the mm so it doesn't go away and get reused after task_unlock
> 	 */
> 	if (!task->vfork_done) {
> 		struct task_struct *p = find_lock_task_mm(task);
> 
> 		if (p) {
> -			if (atomic_read(&p->mm->mm_users) > 1) {
> +			if (atomic_read(&p->mm->mm_users) > p->signal->nr_threads) {
> 				mm = p->mm;
> 				mmgrab(mm);
> 			}
> 			task_unlock(p);
> 		}
> 	}

I remember playing with something like that but it had problems too. I
do not remember details. Oleg would know better.

-- 
Michal Hocko
SUSE Labs