| Message ID | 20210611171940.960887-1-atomlin@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC] mm/oom_kill: show oom eligibility when displaying the current memory state of all tasks | expand |
On 6/11/21 1:19 PM, Aaron Tomlin wrote: > At the present time, when showing potential OOM victims, we do not > exclude tasks which already have MMF_OOM_SKIP set; it is possible that > the last OOM killable victim was already OOM killed, yet the OOM > reaper failed to reclaim memory and set MMF_OOM_SKIP. > This can be confusing/or perhaps even misleading, to the reader of the > OOM report. Now, we already unconditionally display a task's > oom_score_adj_min value that can be set to OOM_SCORE_ADJ_MIN which is > indicative of an "unkillable" task i.e. is not eligible. > > This patch provides a clear indication with regard to the OOM > eligibility of each displayed task. > > Signed-off-by: Aaron Tomlin <atomlin@redhat.com> > --- > mm/oom_kill.c | 31 +++++++++++++++++++++++++++---- > 1 file changed, 27 insertions(+), 4 deletions(-) > > diff --git a/mm/oom_kill.c b/mm/oom_kill.c > index eefd3f5fde46..70781d681a6e 100644 > --- a/mm/oom_kill.c > +++ b/mm/oom_kill.c > @@ -160,6 +160,27 @@ static inline bool is_sysrq_oom(struct oom_control *oc) > return oc->order == -1; > } > > +/** > + * is_task_eligible_oom - determine if and why a task cannot be OOM killed > + * @tsk: task to check > + * > + * Needs to be called with task_lock(). > + */ > +static const char * is_task_oom_eligible(struct task_struct *p) > +{ > + long adj; > + > + adj = (long)p->signal->oom_score_adj; > + if (adj == OOM_SCORE_ADJ_MIN) > + return "no: oom score"; > + else if (test_bit(MMF_OOM_SKIP, &p->mm->flags) > + return "no: oom reaped"; > + else if (in_vfork(p)) > + return "no: in vfork"; > + else > + return "yes"; > +} > + > /* return true if the task is not adequate as candidate victim task. */ > static bool oom_unkillable_task(struct task_struct *p) > { > @@ -401,12 +422,13 @@ static int dump_task(struct task_struct *p, void *arg) > return 0; > } > > - pr_info("[%7d] %5d %5d %8lu %8lu %8ld %8lu %5hd %s\n", > + pr_info("[%7d] %5d %5d %8lu %8lu %8ld %8lu %5hd %-15s %s\n", > task->pid, from_kuid(&init_user_ns, task_uid(task)), > task->tgid, task->mm->total_vm, get_mm_rss(task->mm), > mm_pgtables_bytes(task->mm), > get_mm_counter(task->mm, MM_SWAPENTS), > - task->signal->oom_score_adj, task->comm); > + task->signal->oom_score_adj, is_task_oom_eligible(task), > + task->comm); > task_unlock(task); > > return 0; > @@ -420,12 +442,13 @@ static int dump_task(struct task_struct *p, void *arg) > * memcg, not in the same cpuset, or bound to a disjoint set of mempolicy nodes > * are not shown. > * State information includes task's pid, uid, tgid, vm size, rss, > - * pgtables_bytes, swapents, oom_score_adj value, and name. > + * pgtables_bytes, swapents, oom_score_adj value, oom eligible status > + * and name. > */ > static void dump_tasks(struct oom_control *oc) > { > pr_info("Tasks state (memory values in pages):\n"); > - pr_info("[ pid ] uid tgid total_vm rss pgtables_bytes swapents oom_score_adj name\n"); > + pr_info("[ pid ] uid tgid total_vm rss pgtables_bytes swapents oom_score_adj oom eligible? name\n"); A minor nit: "oom eligible?" has 13 characters. The field width is 15. Maybe you should pad 2 more spaces to make the proper alignment. Cheers, Longman
On 2021/06/12 2:42, Waiman Long wrote: > A minor nit: > > "oom eligible?" has 13 characters. The field width is 15. Maybe you should pad 2 more spaces to make the proper alignment. Maybe single character (e.g. 'S' for OOM_SCORE_ADJ_MIN, 'R' for MMF_OOM_SKIP, 'V' for in_vfork(), none for eligible) is better because dump_task() might print many hundreds of lines?
On 2021/6/12 01:19, Aaron Tomlin wrote: > At the present time, when showing potential OOM victims, we do not > exclude tasks which already have MMF_OOM_SKIP set; it is possible that > the last OOM killable victim was already OOM killed, yet the OOM > reaper failed to reclaim memory and set MMF_OOM_SKIP. > This can be confusing/or perhaps even misleading, to the reader of the > OOM report. Now, we already unconditionally display a task's > oom_score_adj_min value that can be set to OOM_SCORE_ADJ_MIN which is > indicative of an "unkillable" task i.e. is not eligible. > > This patch provides a clear indication with regard to the OOM > eligibility of each displayed task. > > Signed-off-by: Aaron Tomlin <atomlin@redhat.com> > --- > mm/oom_kill.c | 31 +++++++++++++++++++++++++++---- > 1 file changed, 27 insertions(+), 4 deletions(-) > > diff --git a/mm/oom_kill.c b/mm/oom_kill.c > index eefd3f5fde46..70781d681a6e 100644 > --- a/mm/oom_kill.c > +++ b/mm/oom_kill.c > @@ -160,6 +160,27 @@ static inline bool is_sysrq_oom(struct oom_control *oc) > return oc->order == -1; > } > > +/** > + * is_task_eligible_oom - determine if and why a task cannot be OOM killed > + * @tsk: task to check > + * > + * Needs to be called with task_lock(). > + */ > +static const char * is_task_oom_eligible(struct task_struct *p) > +{ > + long adj; > + > + adj = (long)p->signal->oom_score_adj; accoring to the origin type, adj this place maybe use *short* instead. > + if (adj == OOM_SCORE_ADJ_MIN) > + return "no: oom score"; > + else if (test_bit(MMF_OOM_SKIP, &p->mm->flags) > + return "no: oom reaped"; > + else if (in_vfork(p)) > + return "no: in vfork"; > + else > + return "yes"; > +} > + > /* return true if the task is not adequate as candidate victim task. */ > static bool oom_unkillable_task(struct task_struct *p) > { > @@ -401,12 +422,13 @@ static int dump_task(struct task_struct *p, void *arg) > return 0; > } > > - pr_info("[%7d] %5d %5d %8lu %8lu %8ld %8lu %5hd %s\n", > + pr_info("[%7d] %5d %5d %8lu %8lu %8ld %8lu %5hd %-15s %s\n", > task->pid, from_kuid(&init_user_ns, task_uid(task)), > task->tgid, task->mm->total_vm, get_mm_rss(task->mm), > mm_pgtables_bytes(task->mm), > get_mm_counter(task->mm, MM_SWAPENTS), > - task->signal->oom_score_adj, task->comm); > + task->signal->oom_score_adj, is_task_oom_eligible(task), > + task->comm); > task_unlock(task); > > return 0; > @@ -420,12 +442,13 @@ static int dump_task(struct task_struct *p, void *arg) > * memcg, not in the same cpuset, or bound to a disjoint set of mempolicy nodes > * are not shown. > * State information includes task's pid, uid, tgid, vm size, rss, > - * pgtables_bytes, swapents, oom_score_adj value, and name. > + * pgtables_bytes, swapents, oom_score_adj value, oom eligible status > + * and name. > */ > static void dump_tasks(struct oom_control *oc) > { > pr_info("Tasks state (memory values in pages):\n"); > - pr_info("[ pid ] uid tgid total_vm rss pgtables_bytes swapents oom_score_adj name\n"); > + pr_info("[ pid ] uid tgid total_vm rss pgtables_bytes swapents oom_score_adj oom eligible? name\n"); > > if (is_memcg_oom(oc)) > mem_cgroup_scan_tasks(oc->memcg, dump_task, oc);
diff --git a/mm/oom_kill.c b/mm/oom_kill.c index eefd3f5fde46..70781d681a6e 100644 --- a/mm/oom_kill.c +++ b/mm/oom_kill.c @@ -160,6 +160,27 @@ static inline bool is_sysrq_oom(struct oom_control *oc) return oc->order == -1; } +/** + * is_task_eligible_oom - determine if and why a task cannot be OOM killed + * @tsk: task to check + * + * Needs to be called with task_lock(). + */ +static const char * is_task_oom_eligible(struct task_struct *p) +{ + long adj; + + adj = (long)p->signal->oom_score_adj; + if (adj == OOM_SCORE_ADJ_MIN) + return "no: oom score"; + else if (test_bit(MMF_OOM_SKIP, &p->mm->flags) + return "no: oom reaped"; + else if (in_vfork(p)) + return "no: in vfork"; + else + return "yes"; +} + /* return true if the task is not adequate as candidate victim task. */ static bool oom_unkillable_task(struct task_struct *p) { @@ -401,12 +422,13 @@ static int dump_task(struct task_struct *p, void *arg) return 0; } - pr_info("[%7d] %5d %5d %8lu %8lu %8ld %8lu %5hd %s\n", + pr_info("[%7d] %5d %5d %8lu %8lu %8ld %8lu %5hd %-15s %s\n", task->pid, from_kuid(&init_user_ns, task_uid(task)), task->tgid, task->mm->total_vm, get_mm_rss(task->mm), mm_pgtables_bytes(task->mm), get_mm_counter(task->mm, MM_SWAPENTS), - task->signal->oom_score_adj, task->comm); + task->signal->oom_score_adj, is_task_oom_eligible(task), + task->comm); task_unlock(task); return 0; @@ -420,12 +442,13 @@ static int dump_task(struct task_struct *p, void *arg) * memcg, not in the same cpuset, or bound to a disjoint set of mempolicy nodes * are not shown. * State information includes task's pid, uid, tgid, vm size, rss, - * pgtables_bytes, swapents, oom_score_adj value, and name. + * pgtables_bytes, swapents, oom_score_adj value, oom eligible status + * and name. */ static void dump_tasks(struct oom_control *oc) { pr_info("Tasks state (memory values in pages):\n"); - pr_info("[ pid ] uid tgid total_vm rss pgtables_bytes swapents oom_score_adj name\n"); + pr_info("[ pid ] uid tgid total_vm rss pgtables_bytes swapents oom_score_adj oom eligible? name\n"); if (is_memcg_oom(oc)) mem_cgroup_scan_tasks(oc->memcg, dump_task, oc);
At the present time, when showing potential OOM victims, we do not exclude tasks which already have MMF_OOM_SKIP set; it is possible that the last OOM killable victim was already OOM killed, yet the OOM reaper failed to reclaim memory and set MMF_OOM_SKIP. This can be confusing/or perhaps even misleading, to the reader of the OOM report. Now, we already unconditionally display a task's oom_score_adj_min value that can be set to OOM_SCORE_ADJ_MIN which is indicative of an "unkillable" task i.e. is not eligible. This patch provides a clear indication with regard to the OOM eligibility of each displayed task. Signed-off-by: Aaron Tomlin <atomlin@redhat.com> --- mm/oom_kill.c | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-)