Re: [PATCH v7 2/2] Refactor part of the oom report in dump_header

2018-06-11 Thread Michal Hocko
On Sun 10-06-18 08:12:16, Mike Rapoport wrote: > On Fri, Jun 08, 2018 at 05:53:14PM +0800, 禹舟键 wrote: > > Hi Mike > > > My question was why do you call to alloc_constrained in the dump_header() > > > function rather than pass the constraint that was detected a bit earlier > > > to > > > that funct

Re: [PATCH v7 2/2] Refactor part of the oom report in dump_header

2018-06-09 Thread Mike Rapoport
On Fri, Jun 08, 2018 at 05:53:14PM +0800, 禹舟键 wrote: > Hi Mike > > My question was why do you call to alloc_constrained in the dump_header() > > function rather than pass the constraint that was detected a bit earlier to > > that function? > > dump_header will be called by three functions: oom_kil

Re: [PATCH v7 2/2] Refactor part of the oom report in dump_header

2018-06-08 Thread 禹舟键
Hi Mike > My question was why do you call to alloc_constrained in the dump_header() > function rather than pass the constraint that was detected a bit earlier to > that function? dump_header will be called by three functions: oom_kill_process, check_panic_on_oom, out_of_memory. We can get the cons

Re: [PATCH v7 2/2] Refactor part of the oom report in dump_header

2018-06-04 Thread Michal Hocko
On Mon 04-06-18 20:13:44, 禹舟键 wrote: > Hi Michal > I will add the missing information in the cover-letter. I do not really think the cover letter needs much improvements. It is the patch description that should be as specific as possible. Cover letter should contain a highlevel description usually

Re: [PATCH v7 2/2] Refactor part of the oom report in dump_header

2018-06-04 Thread 禹舟键
Hi Michal I will add the missing information in the cover-letter. > That being said, I am ready to ack a patch which adds the memcg of the > oom victim. I will not ack (nor nack) the patch which turns it into a > single print because I am not sure the benefit is really worth it. Maybe > others wil

Re: [PATCH v7 2/2] Refactor part of the oom report in dump_header

2018-06-04 Thread Michal Hocko
On Mon 04-06-18 16:57:17, 禹舟键 wrote: > Hi Michal > > > I have earlier suggested that you split this into two parts. One to add > > the missing information and the later to convert it to a single printk > > output. > > I'm sorry I do not get your point. What do you mean the missing information?

Re: [PATCH v7 2/2] Refactor part of the oom report in dump_header

2018-06-04 Thread 禹舟键
Hi Michal > I have earlier suggested that you split this into two parts. One to add > the missing information and the later to convert it to a single printk > output. I'm sorry I do not get your point. What do you mean the missing information? > but it still really begs an example why we really

Re: [PATCH v7 2/2] Refactor part of the oom report in dump_header

2018-06-04 Thread 禹舟键
Hi Tetsuo I know what you mean. Actully I refer to the code for kernel version: 3.10.0-514. I think I can just use an array of type char, rather than static char. Is it right? Thanks

Re: [PATCH v7 2/2] Refactor part of the oom report in dump_header

2018-06-03 Thread Michal Hocko
On Sat 02-06-18 19:58:52, ufo19890...@gmail.com wrote: > From: yuzhoujian > > The dump_header does not print the memcg's name when the system > oom happened, so users cannot locate the certain container which > contains the task that has been killed by the oom killer. > > I follow the advices of

Re: [PATCH v7 2/2] Refactor part of the oom report in dump_header

2018-06-03 Thread 禹舟键
Hi Mike > My question was why do you call to alloc_constrained in the dump_header() >function rather than pass the constraint that was detected a bit earlier to >that function? Ok, I will add a new parameter in the dump_header. Thank you.

Re: [PATCH v7 2/2] Refactor part of the oom report in dump_header

2018-06-03 Thread Mike Rapoport
On Mon, Jun 04, 2018 at 10:41:10AM +0800, 禹舟键 wrote: > Hi Tetsuo > > Since origin_memcg_name is printed for both memcg OOM and !memcg OOM, it is > > strange that origin_memcg_name is updated only when memcg != NULL. Have you > > really tested !memcg OOM case? > > if memcg == NULL , origin_memcg_

Re: [PATCH v7 2/2] Refactor part of the oom report in dump_header

2018-06-03 Thread 禹舟键
Hi Tetsuo > Since origin_memcg_name is printed for both memcg OOM and !memcg OOM, it is > strange that origin_memcg_name is updated only when memcg != NULL. Have you > really tested !memcg OOM case? if memcg == NULL , origin_memcg_name will also be NULL, so the length of it is 0. origin_memcg_na

Re: [PATCH v7 2/2] Refactor part of the oom report in dump_header

2018-06-03 Thread 禹舟键
Hi Mike > Please keep the brief description of the function actually brief and move the > detailed explanation after the parameters description. Thanks for your advice. > The allocation constraint is detected by the dump_header() callers, why not > just use it here? David suggest that constraint

Re: [PATCH v7 2/2] Refactor part of the oom report in dump_header

2018-06-03 Thread Tetsuo Handa
On 2018/06/02 20:58, yuzhoujian wrote: > -void mem_cgroup_print_oom_info(struct mem_cgroup *memcg, struct task_struct > *p) > +void mem_cgroup_print_oom_context(struct mem_cgroup *memcg, struct > task_struct *p, > + enum oom_constraint constraint, nodemask_t *nodemask) > { >

Re: [PATCH v7 2/2] Refactor part of the oom report in dump_header

2018-06-03 Thread Mike Rapoport
On Sat, Jun 02, 2018 at 07:58:52PM +0800, ufo19890...@gmail.com wrote: > From: yuzhoujian > > The dump_header does not print the memcg's name when the system > oom happened, so users cannot locate the certain container which > contains the task that has been killed by the oom killer. > > I follo

[PATCH v7 2/2] Refactor part of the oom report in dump_header

2018-06-02 Thread ufo19890607
From: yuzhoujian The dump_header does not print the memcg's name when the system oom happened, so users cannot locate the certain container which contains the task that has been killed by the oom killer. I follow the advices of David Rientjes and Michal Hocko, and refactor part of the oom report

[PATCH v7 2/2] Refactor part of the oom report in dump_header

2018-06-02 Thread ufo19890607
From: yuzhoujian The dump_header does not print the memcg's name when the system oom happened, so users cannot locate the certain container which contains the task that has been killed by the oom killer. I follow the advices of David Rientjes and Michal Hocko, and refactor part of the oom report