| Message ID | 1530376739-20459-1-git-send-email-ufo19890607@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sun 01-07-18 00:38:58, ufo19890607@gmail.com wrote: > From: yuzhoujian <yuzhoujian@didichuxing.com> > > The current system wide oom report prints information about the victim > and the allocation context and restrictions. It, however, doesn't > provide any information about memory cgroup the victim belongs to. This > information can be interesting for container users because they can find > the victim's container much more easily. > > I follow the advices of David Rientjes and Michal Hocko, and refactor > part of the oom report. After this patch, users can get the memcg's > path from the oom report and check the certain container more quickly. > > The oom print info after this patch: > oom-kill:constraint=<constraint>,nodemask=<nodemask>,oom_memcg=<memcg>,task_memcg=<memcg>,task=<comm>,pid=<pid>,uid=<uid> This changelog doesn't correspond to the patch. Also while we were discussing this off-list, I have suggested to pull the cpuset info into the single line output. What about the following? " OOM report contains several sections. The first one is the allocation context that has triggered the OOM. Then we have cpuset context followed by the stack trace of the OOM path. Followed by the oom eligible tasks and the information about the chosen oom victim. One thing that makes parsing more awkward than necessary is that we do not have a single and easily parsable line about the oom context. This patch is reorganizing the oom report to 1) who invoked oom and what was the allocation request [ 126.168182] panic invoked oom-killer: gfp_mask=0x6280ca(GFP_HIGHUSER_MOVABLE|__GFP_ZERO), order=0, oom_score_adj=0 2) OOM stack trace [ 126.169806] CPU: 23 PID: 8668 Comm: panic Not tainted 4.18.0-rc2+ #36 [ 126.170494] Hardware name: Inspur SA5212M4/YZMB-00370-107, BIOS 4.1.10 11/14/2016 [ 126.171197] Call Trace: [ 126.171901] dump_stack+0x5a/0x73 [ 126.172593] dump_header+0x58/0x2dc [ 126.173294] oom_kill_process+0x228/0x420 [ 126.173999] ? oom_badness+0x2a/0x130 [ 126.174705] out_of_memory+0x11a/0x4a0 [ 126.175415] __alloc_pages_slowpath+0x7cc/0xa1e [ 126.176128] ? __alloc_pages_slowpath+0x194/0xa1e [ 126.176853] ? page_counter_try_charge+0x54/0xc0 [ 126.177580] __alloc_pages_nodemask+0x277/0x290 [ 126.178319] alloc_pages_vma+0x73/0x180 [ 126.179058] do_anonymous_page+0xed/0x5a0 [ 126.179825] __handle_mm_fault+0xbb3/0xe70 [ 126.180566] handle_mm_fault+0xfa/0x210 [ 126.181313] __do_page_fault+0x233/0x4c0 [ 126.182063] do_page_fault+0x32/0x140 [ 126.182812] ? page_fault+0x8/0x30 [ 126.183560] page_fault+0x1e/0x30 3) oom context (contrains and the chosen victim) [ 126.190619] oom-kill:constraint=CONSTRAINT_NONE,nodemask=(null),cpuset=/,mems_allowed=0-1,task=panic,pid= 8673,uid= 0 An admin can easily get the full oom context at a single line which makes parsing much easier. "
On Sat, Jun 30, 2018 at 7:38 PM, <ufo19890607@gmail.com> wrote: > From: yuzhoujian <yuzhoujian@didichuxing.com> > > The current system wide oom report prints information about the victim > and the allocation context and restrictions. It, however, doesn't > provide any information about memory cgroup the victim belongs to. This > information can be interesting for container users because they can find > the victim's container much more easily. > > I follow the advices of David Rientjes and Michal Hocko, and refactor > part of the oom report. After this patch, users can get the memcg's > path from the oom report and check the certain container more quickly. > > The oom print info after this patch: > oom-kill:constraint=<constraint>,nodemask=<nodemask>,oom_memcg=<memcg>,task_memcg=<memcg>,task=<comm>,pid=<pid>,uid=<uid> > +static const char * const oom_constraint_text[] = { > + [CONSTRAINT_NONE] = "CONSTRAINT_NONE", > + [CONSTRAINT_CPUSET] = "CONSTRAINT_CPUSET", > + [CONSTRAINT_MEMORY_POLICY] = "CONSTRAINT_MEMORY_POLICY", > + [CONSTRAINT_MEMCG] = "CONSTRAINT_MEMCG", > +}; I'm not sure why we have this in the header. This produces a lot of noise when W=1. In file included from /home/andy/prj/linux-topic-mfld/include/linux/memcontrol.h:31:0, from /home/andy/prj/linux-topic-mfld/include/net/sock.h:58, from /home/andy/prj/linux-topic-mfld/include/linux/tcp.h:23, from /home/andy/prj/linux-topic-mfld/include/linux/ipv6.h:87, from /home/andy/prj/linux-topic-mfld/include/net/ipv6.h:16, from /home/andy/prj/linux-topic-mfld/net/ipv4/netfilter/nf_log_ipv4.c:17: /home/andy/prj/linux-topic-mfld/include/linux/oom.h:32:27: warning: ‘oom_constraint_text’ defined but not used [-W unused-const-variable=] static const char * const oom_constraint_text[] = { ^~~~~~~~~~~~~~~~~~~ CC [M] net/ipv4/netfilter/iptable_nat.o If you need (but looking at the code you actually don't if I didn't miss anything) it in several places, just export. Otherwise put it back to memcontrol.c.
Hi Andy The const char array need to be used by the new func mem_cgroup_print_oom_context and some funcs in oom_kill.c in the second patch. Thanks > > On Sat, Jun 30, 2018 at 7:38 PM, <ufo19890607@gmail.com> wrote: > > From: yuzhoujian <yuzhoujian@didichuxing.com> > > > > The current system wide oom report prints information about the victim > > and the allocation context and restrictions. It, however, doesn't > > provide any information about memory cgroup the victim belongs to. This > > information can be interesting for container users because they can find > > the victim's container much more easily. > > > > I follow the advices of David Rientjes and Michal Hocko, and refactor > > part of the oom report. After this patch, users can get the memcg's > > path from the oom report and check the certain container more quickly. > > > > The oom print info after this patch: > > oom-kill:constraint=<constraint>,nodemask=<nodemask>,oom_memcg=<memcg>,task_memcg=<memcg>,task=<comm>,pid=<pid>,uid=<uid> > > > > +static const char * const oom_constraint_text[] = { > > + [CONSTRAINT_NONE] = "CONSTRAINT_NONE", > > + [CONSTRAINT_CPUSET] = "CONSTRAINT_CPUSET", > > + [CONSTRAINT_MEMORY_POLICY] = "CONSTRAINT_MEMORY_POLICY", > > + [CONSTRAINT_MEMCG] = "CONSTRAINT_MEMCG", > > +}; > > I'm not sure why we have this in the header. > > This produces a lot of noise when W=1. > > In file included from > /home/andy/prj/linux-topic-mfld/include/linux/memcontrol.h:31:0, > from /home/andy/prj/linux-topic-mfld/include/net/sock.h:58, > from /home/andy/prj/linux-topic-mfld/include/linux/tcp.h:23, > from /home/andy/prj/linux-topic-mfld/include/linux/ipv6.h:87, > from /home/andy/prj/linux-topic-mfld/include/net/ipv6.h:16, > from > /home/andy/prj/linux-topic-mfld/net/ipv4/netfilter/nf_log_ipv4.c:17: > /home/andy/prj/linux-topic-mfld/include/linux/oom.h:32:27: warning: > ‘oom_constraint_text’ defined but not used [-W > unused-const-variable=] > static const char * const oom_constraint_text[] = { > ^~~~~~~~~~~~~~~~~~~ > CC [M] net/ipv4/netfilter/iptable_nat.o > > > If you need (but looking at the code you actually don't if I didn't > miss anything) it in several places, just export. > Otherwise put it back to memcontrol.c. > > -- > With Best Regards, > Andy Shevchenko
On Wed 04-07-18 10:25:30, 禹舟键 wrote: > Hi Andy > The const char array need to be used by the new func > mem_cgroup_print_oom_context and some funcs in oom_kill.c in the > second patch. Just declare it in oom.h and define in oom.c
On Wed, Jul 4, 2018 at 5:25 AM, 禹舟键 <ufo19890607@gmail.com> wrote: > Hi Andy > The const char array need to be used by the new func > mem_cgroup_print_oom_context and some funcs in oom_kill.c in the > second patch. Did I understand correctly that the array is added by you in this solely patch? Did I understand correctly that it's used only in one module (oom_kill.c in new version)? If both are true, just move it to the C file. If you need a synchronization, a) put a comment, b) create another enum item (like FOO_BAR_MAX) at the end and use it in the array as a fixed size, > > Thanks > >> >> On Sat, Jun 30, 2018 at 7:38 PM, <ufo19890607@gmail.com> wrote: >> > From: yuzhoujian <yuzhoujian@didichuxing.com> >> > >> > The current system wide oom report prints information about the victim >> > and the allocation context and restrictions. It, however, doesn't >> > provide any information about memory cgroup the victim belongs to. This >> > information can be interesting for container users because they can find >> > the victim's container much more easily. >> > >> > I follow the advices of David Rientjes and Michal Hocko, and refactor >> > part of the oom report. After this patch, users can get the memcg's >> > path from the oom report and check the certain container more quickly. >> > >> > The oom print info after this patch: >> > oom-kill:constraint=<constraint>,nodemask=<nodemask>,oom_memcg=<memcg>,task_memcg=<memcg>,task=<comm>,pid=<pid>,uid=<uid> >> >> >> > +static const char * const oom_constraint_text[] = { >> > + [CONSTRAINT_NONE] = "CONSTRAINT_NONE", >> > + [CONSTRAINT_CPUSET] = "CONSTRAINT_CPUSET", >> > + [CONSTRAINT_MEMORY_POLICY] = "CONSTRAINT_MEMORY_POLICY", >> > + [CONSTRAINT_MEMCG] = "CONSTRAINT_MEMCG", >> > +}; >> >> I'm not sure why we have this in the header. >> >> This produces a lot of noise when W=1. >> >> In file included from >> /home/andy/prj/linux-topic-mfld/include/linux/memcontrol.h:31:0, >> from /home/andy/prj/linux-topic-mfld/include/net/sock.h:58, >> from /home/andy/prj/linux-topic-mfld/include/linux/tcp.h:23, >> from /home/andy/prj/linux-topic-mfld/include/linux/ipv6.h:87, >> from /home/andy/prj/linux-topic-mfld/include/net/ipv6.h:16, >> from >> /home/andy/prj/linux-topic-mfld/net/ipv4/netfilter/nf_log_ipv4.c:17: >> /home/andy/prj/linux-topic-mfld/include/linux/oom.h:32:27: warning: >> ‘oom_constraint_text’ defined but not used [-W >> unused-const-variable=] >> static const char * const oom_constraint_text[] = { >> ^~~~~~~~~~~~~~~~~~~ >> CC [M] net/ipv4/netfilter/iptable_nat.o >> >> >> If you need (but looking at the code you actually don't if I didn't >> miss anything) it in several places, just export. >> Otherwise put it back to memcontrol.c. >> >> -- >> With Best Regards, >> Andy Shevchenko
Hi Michal and Andy The enum oom_constraint will be added in the struct oom_control. So I still think I should define it in oom.h. Michal Hocko <mhocko@kernel.org> 于2018年7月4日周三 下午4:17写道： > > On Wed 04-07-18 10:25:30, 禹舟键 wrote: > > Hi Andy > > The const char array need to be used by the new func > > mem_cgroup_print_oom_context and some funcs in oom_kill.c in the > > second patch. > > Just declare it in oom.h and define in oom.c > -- > Michal Hocko > SUSE Labs
On Thu, Jul 5, 2018 at 2:23 PM, 禹舟键 <ufo19890607@gmail.com> wrote: > Hi Michal and Andy > The enum oom_constraint will be added in the struct oom_control. So > I still think I should define it in oom.h. You missed the point. I'm talking about an array of string literals. Please, check what the warning I got from the compiler. > Michal Hocko <mhocko@kernel.org> 于2018年7月4日周三 下午4:17写道： >> >> On Wed 04-07-18 10:25:30, 禹舟键 wrote: >> > Hi Andy >> > The const char array need to be used by the new func >> > mem_cgroup_print_oom_context and some funcs in oom_kill.c in the >> > second patch. >> >> Just declare it in oom.h and define in oom.c >> -- >> Michal Hocko >> SUSE Labs
diff --git a/include/linux/oom.h b/include/linux/oom.h index 6adac113e96d..5bed78d4bfb8 100644 --- a/include/linux/oom.h +++ b/include/linux/oom.h @@ -15,6 +15,20 @@ struct notifier_block; struct mem_cgroup; struct task_struct; +enum oom_constraint { + CONSTRAINT_NONE, + CONSTRAINT_CPUSET, + CONSTRAINT_MEMORY_POLICY, + CONSTRAINT_MEMCG, +}; + +static const char * const oom_constraint_text[] = { + [CONSTRAINT_NONE] = "CONSTRAINT_NONE", + [CONSTRAINT_CPUSET] = "CONSTRAINT_CPUSET", + [CONSTRAINT_MEMORY_POLICY] = "CONSTRAINT_MEMORY_POLICY", + [CONSTRAINT_MEMCG] = "CONSTRAINT_MEMCG", +}; + /* * Details of the page allocation that triggered the oom killer that are used to * determine what should be killed. @@ -42,6 +56,9 @@ struct oom_control { unsigned long totalpages; struct task_struct *chosen; unsigned long chosen_points; + + /* Used to print the constraint info. */ + enum oom_constraint constraint; }; extern struct mutex oom_lock; diff --git a/mm/oom_kill.c b/mm/oom_kill.c index 84081e77bc51..f9b08e455fd1 100644 --- a/mm/oom_kill.c +++ b/mm/oom_kill.c @@ -237,13 +237,6 @@ unsigned long oom_badness(struct task_struct *p, struct mem_cgroup *memcg, return points > 0 ? points : 1; } -enum oom_constraint { - CONSTRAINT_NONE, - CONSTRAINT_CPUSET, - CONSTRAINT_MEMORY_POLICY, - CONSTRAINT_MEMCG, -}; - /* * Determine the type of allocation constraint. */ @@ -421,15 +414,20 @@ static void dump_tasks(struct mem_cgroup *memcg, const nodemask_t *nodemask) static void dump_header(struct oom_control *oc, struct task_struct *p) { - pr_warn("%s invoked oom-killer: gfp_mask=%#x(%pGg), nodemask=%*pbl, order=%d, oom_score_adj=%hd\n", - current->comm, oc->gfp_mask, &oc->gfp_mask, - nodemask_pr_args(oc->nodemask), oc->order, + pr_warn("%s invoked oom-killer: gfp_mask=%#x(%pGg), order=%d, oom_score_adj=%hd\n", + current->comm, oc->gfp_mask, &oc->gfp_mask, oc->order, current->signal->oom_score_adj); if (!IS_ENABLED(CONFIG_COMPACTION) && oc->order) pr_warn("COMPACTION is disabled!!!\n"); cpuset_print_current_mems_allowed(); dump_stack(); + + /* one line summary of the oom killer context. */ + pr_info("oom-kill:constraint=%s,nodemask=%*pbl,task=%s,pid=%5d,uid=%5d", + oom_constraint_text[oc->constraint], + nodemask_pr_args(oc->nodemask), + p->comm, p->pid, from_kuid(&init_user_ns, task_uid(p))); if (is_memcg_oom(oc)) mem_cgroup_print_oom_info(oc->memcg, p); else { @@ -973,8 +971,7 @@ static void oom_kill_process(struct oom_control *oc, const char *message) /* * Determines whether the kernel must panic because of the panic_on_oom sysctl. */ -static void check_panic_on_oom(struct oom_control *oc, - enum oom_constraint constraint) +static void check_panic_on_oom(struct oom_control *oc) { if (likely(!sysctl_panic_on_oom)) return; @@ -984,7 +981,7 @@ static void check_panic_on_oom(struct oom_control *oc, * does not panic for cpuset, mempolicy, or memcg allocation * failures. */ - if (constraint != CONSTRAINT_NONE) + if (oc->constraint != CONSTRAINT_NONE) return; } /* Do not panic for oom kills triggered by sysrq */ @@ -1021,8 +1018,8 @@ EXPORT_SYMBOL_GPL(unregister_oom_notifier); bool out_of_memory(struct oom_control *oc) { unsigned long freed = 0; - enum oom_constraint constraint = CONSTRAINT_NONE; + oc->constraint = CONSTRAINT_NONE; if (oom_killer_disabled) return false; @@ -1057,10 +1054,10 @@ bool out_of_memory(struct oom_control *oc) * Check if there were limitations on the allocation (only relevant for * NUMA and memcg) that may require different handling. */ - constraint = constrained_alloc(oc); - if (constraint != CONSTRAINT_MEMORY_POLICY) + oc->constraint = constrained_alloc(oc); + if (oc->constraint != CONSTRAINT_MEMORY_POLICY) oc->nodemask = NULL; - check_panic_on_oom(oc, constraint); + check_panic_on_oom(oc); if (!is_memcg_oom(oc) && sysctl_oom_kill_allocating_task && current->mm && !oom_unkillable_task(current, NULL, oc->nodemask) &&