Tetsuo Handa wrote: > Michal Hocko wrote: > > The victim selection code can be reduced because it is basically > > shared between the two, only the iterator differs. But I guess that > > can be eliminated by a simple helper. > > Thank you for CC: me. I like this clean up. > > > --- > > include/linux/oom.h | 5 +++++ > > mm/memcontrol.c | 47 ++++++----------------------------------- > > mm/oom_kill.c | 60 ++++++++++++++++++++++++++++------------------------- > > 3 files changed, 43 insertions(+), 69 deletions(-) > > I think we can apply your version with below changes folded into your version. > (I think totalpages argument can be passed via oom_control as well. Also, according to > http://lkml.kernel.org/r/201602192336.EJF90671.HMFLFSVOFJOtOQ@xxxxxxxxxxxxxxxxxxx , > we can safely replace oc->memcg in oom_badness() in oom_evaluate_task() with NULL. ) > > include/linux/oom.h | 10 ---------- > mm/memcontrol.c | 7 +++++-- > mm/oom_kill.c | 14 ++++++++++++-- > 3 files changed, 17 insertions(+), 14 deletions(-) > > diff --git a/include/linux/oom.h b/include/linux/oom.h > index 7b3eb25..77e98a0 100644 > --- a/include/linux/oom.h > +++ b/include/linux/oom.h > @@ -49,13 +49,6 @@ enum oom_constraint { > CONSTRAINT_MEMCG, > }; > > -enum oom_scan_t { > - OOM_SCAN_OK, /* scan thread and find its badness */ > - OOM_SCAN_CONTINUE, /* do not consider thread for oom kill */ > - OOM_SCAN_ABORT, /* abort the iteration and return */ > - OOM_SCAN_SELECT, /* always select this thread first */ > -}; > - > extern struct mutex oom_lock; > > static inline void set_current_oom_origin(void) > @@ -96,9 +89,6 @@ extern void oom_kill_process(struct oom_control *oc, struct task_struct *p, > extern void check_panic_on_oom(struct oom_control *oc, > enum oom_constraint constraint); > > -extern enum oom_scan_t oom_scan_process_thread(struct oom_control *oc, > - struct task_struct *task); > - > extern bool out_of_memory(struct oom_control *oc); > > extern void exit_oom_victim(struct task_struct *tsk); > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 9c51b4d..f3482a2 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -1288,12 +1288,15 @@ static bool mem_cgroup_out_of_memory(struct mem_cgroup *memcg, gfp_t gfp_mask, > > css_task_iter_start(&iter->css, &it); > while ((task = css_task_iter_next(&it))) > - if (!oom_evaluate_task(&oc, task, totalpages)) > + if (!oom_evaluate_task(&oc, task, totalpages)) { > + css_task_iter_end(&it); Oops. Duplicated css_task_iter_end() calls. If it is safe to reverse ordering of css_task_iter_end(&it) and mem_cgroup_iter_break(memcg, iter), removing this css_task_iter_end(&it) line is the simplest fix. > + mem_cgroup_iter_break(memcg, iter); > break; > + } > css_task_iter_end(&it); > } > > - if (oc.chosen) { > + if (oc.chosen && oc.chosen != (void *) -1UL) { > points = oc.chosen_points * 1000 / totalpages; > oom_kill_process(&oc, oc.chosen, points, totalpages, > "Memory cgroup out of memory"); > diff --git a/mm/oom_kill.c b/mm/oom_kill.c > index bce3ea2..f634bca 100644 > --- a/mm/oom_kill.c > +++ b/mm/oom_kill.c > @@ -273,8 +273,15 @@ static enum oom_constraint constrained_alloc(struct oom_control *oc, > } > #endif > > -enum oom_scan_t oom_scan_process_thread(struct oom_control *oc, > - struct task_struct *task) > +enum oom_scan_t { > + OOM_SCAN_OK, /* scan thread and find its badness */ > + OOM_SCAN_CONTINUE, /* do not consider thread for oom kill */ > + OOM_SCAN_ABORT, /* abort the iteration and return */ > + OOM_SCAN_SELECT, /* always select this thread first */ > +}; > + > +static enum oom_scan_t oom_scan_process_thread(struct oom_control *oc, > + struct task_struct *task) > { > if (oom_unkillable_task(task, NULL, oc->nodemask)) > return OOM_SCAN_CONTINUE; > @@ -307,6 +314,9 @@ int oom_evaluate_task(struct oom_control *oc, struct task_struct *p, unsigned lo > case OOM_SCAN_CONTINUE: > return 1; > case OOM_SCAN_ABORT: > + if (oc->chosen) > + put_task_struct(oc->chosen); > + oc->chosen = (void *) -1UL; > return 0; > case OOM_SCAN_OK: > break; > -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>