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

[Andy Shevchenko <andy.shevchenko@gmail.com>]

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



-- 
With Best Regards,
Andy Shevchenko