> > > @@ -745,6 +746,10 @@ static int exec_mmap(struct mm_struct *mm)
> > > tsk->mm = mm;
> > > tsk->active_mm = mm;
> > > activate_mm(active_mm, mm);
> > > + if (tsk->signal->oom_score_adj == OOM_SCORE_ADJ_MIN) {
> > > + atomic_dec(&active_mm->oom_disable_count);
> >
> > When kernel thread makes user-land process (e.g. usermode-helper),
> > active_mm might point to unrelated process. active_mm is only meaningful
> > for scheduler code. please don't touch it. probably you intend to
> > change old_mm.
>
> This is safe because kthreads never have non-zero
> p->signal->oom_score_adj.
Hm? my example is wrong? my point is, you shouldn't touch active_mm.
> > > @@ -1690,6 +1697,10 @@ SYSCALL_DEFINE1(unshare, unsigned long, unshare_flags)
> > > active_mm = current->active_mm;
> > > current->mm = new_mm;
> > > current->active_mm = new_mm;
> > > + if (current->signal->oom_score_adj == OOM_SCORE_ADJ_MIN) {
> > > + atomic_dec(&mm->oom_disable_count);
> > > + atomic_inc(&new_mm->oom_disable_count);
> > > + }
> > > activate_mm(active_mm, new_mm);
> > > new_mm = mm;
> > > }
> >
> > This place, we are grabbing task_lock(), but task_lock don't prevent
> > to change signal->oom_score_adj from another thread. This seems racy.
> >
>
> It does, task_lock(current) protects current->signal->oom_score_adj from
> changing in oom-add-per-mm-oom-disable-count.patch.
>
> I'll add the task_lock(p) in mm_init(), thanks for the review!
Wait, can you please elabolate more? task_lock() only lock one thread.
Why can it protect multi-thread race?
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxxx For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>