On Fri, 13 Jul 2018, Michal Hocko wrote: > > diff --git a/mm/oom_kill.c b/mm/oom_kill.c > > index 0fe4087d5151..e6328cef090f 100644 > > --- a/mm/oom_kill.c > > +++ b/mm/oom_kill.c > > @@ -488,9 +488,11 @@ void __oom_reap_task_mm(struct mm_struct *mm) > > * Tell all users of get_user/copy_from_user etc... that the content > > * is no longer stable. No barriers really needed because unmapping > > * should imply barriers already and the reader would hit a page fault > > - * if it stumbled over a reaped memory. > > + * if it stumbled over a reaped memory. If MMF_UNSTABLE is already set, > > + * reaping as already occurred so nothing left to do. > > */ > > - set_bit(MMF_UNSTABLE, &mm->flags); > > + if (test_and_set_bit(MMF_UNSTABLE, &mm->flags)) > > + return; > > This could lead to pre mature oom victim selection > oom_reaper exiting victim > oom_reap_task exit_mmap > __oom_reap_task_mm __oom_reap_task_mm > test_and_set_bit(MMF_UNSTABLE) # wins the race > test_and_set_bit(MMF_UNSTABLE) > set_bit(MMF_OOM_SKIP) # new victim can be selected now. > This is not the current state of the code in the -mm tree: MMF_OOM_SKIP only gets set by the oom reaper when the timeout has expired when the victim has failed to free memory in the exit path. > Besides that, why should we back off in the first place. We can > race the two without any problems AFAICS. We already do have proper > synchronization between the two due to mmap_sem and MMF_OOM_SKIP. > test_and_set_bit() here is not strictly required, I thought it was better since any unmapping done in this context is going to be handled by whichever thread set MMF_UNSTABLE.
