[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <5871495633F38949900D2BF2DC04883E63280E@G08CNEXMBPEKD02.g08.fujitsu.local>
Date: Thu, 13 Nov 2014 09:44:41 +0000
From: "Chen, Hanxiao" <chenhanxiao@...fujitsu.com>
To: Richard Weinberger <richard@....at>,
"Eric W. Biederman" <ebiederm@...ssion.com>,
Serge Hallyn <serge.hallyn@...ntu.com>,
"Oleg Nesterov" <oleg@...hat.com>
CC: "containers@...ts.linux-foundation.org"
<containers@...ts.linux-foundation.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
David Howells <dhowells@...hat.com>,
"Richard Weinberger" <richard.weinberger@...il.com>,
Pavel Emelyanov <xemul@...allels.com>,
Vasiliy Kulikov <segooon@...il.com>,
Mateusz Guzik <mguzik@...hat.com>
Subject: RE: [PATCH v7 1/2] procfs: show hierarchy of pid namespace
> -----Original Message-----
> From: Richard Weinberger [mailto:richard@....at]
> Sent: Wednesday, November 12, 2014 7:15 PM
> To: Chen, Hanxiao/陈 晗霄; Eric W. Biederman; Serge Hallyn; Oleg Nesterov
> Cc: containers@...ts.linux-foundation.org; linux-kernel@...r.kernel.org; David
> Howells; Richard Weinberger; Pavel Emelyanov; Vasiliy Kulikov; Mateusz Guzik
> Subject: Re: [PATCH v7 1/2] procfs: show hierarchy of pid namespace
>
> Am 12.11.2014 um 11:08 schrieb Chen Hanxiao:
> > We lack of pid hierarchy information, and this will lead to:
> > a) we don't know pids' relationship, who is whose child:
> > /proc/PID/ns/pid only tell us whether two pids live in different ns
> > b) bring trouble to nested lxc container check/restore/migration
> > c) bring trouble to pid translation between containers;
> >
> > This patch will show the hierarchy of pid namespace
> > by pidns_hierarchy like:
> >
> > <init_PID> <parent_of_init_PID> <relative PID level>
> >
> > Ex:
> > [root@...alhost ~]#cat /proc/pidns_hierarchy
> > 18060 1 1
> > 18102 18060 2
> > 1534 18102 3
> > 1600 18102 3
> > 1550 1 1
> > *Note: numbers represent the pid 1 in different ns
> >
[snip]
> > +
> > +#define NS_HIERARCHY "pidns_hierarchy"
> > +#define MAX(a, b) ((a) > (b) ? (a) : (b))
>
> Please use max() from kernel.h, there is no need to reinvent the wheel.
>
Thanks for your reminding.
> > +
> > +/* list for host pid collection */
> > +struct pidns_list {
> > + struct list_head list;
> > + struct pid *pid;
> > + int show_level;
>
> s/show_level/level, to keep it easy. :-)
Sure, also change the type to unsigned int
for making max() happy.
[snip]
> > +}
> > +
> > +/*
> > + * collect pids and stored in pidns_pid_list,
>
> s/stored/store
Oops...
>
> > + * then remove duplicated ones,
> > + * add the rest to pidns_pid_tree
> > + */
>
> This comment is a bit confusing.
>
> What about "proc_pidns_list_refresh - Finds all init pids, places them into
> pidns_pid_list
> and then stores the hirarchy into pidns_pid_tree."?
>
That's much more clear.
> Beside of my minor comments I like the patch. :-)
> Thanks a lot for doing this work!
>
Thanks for your kindly help. :)
- Chen
Powered by blists - more mailing lists