[RFC 0/4] reduce pid_namespace footprint when not configured

[RFC 0/4] reduce pid_namespace footprint when not configured

[Rasmus Villemoes]

A few suggestions for reducing the footprint of pid namespaces when
CONFIG_PID_NS=n. The savings are admittedly not huge:

$ scripts/bloat-o-meter /tmp/vmlinux.master vmlinux
add/remove: 0/7 grow/shrink: 30/25 up/down: 368/-1032 (-664)

Rasmus Villemoes (4):
  pid_namespace: remove ->level and ->parent for !CONFIG_PID_NS
  pid_namespace: remove .kref field for !CONFIG_PID_NS
  pid_namespace: make pid_nr_ns and pid_vnr static inlines for
    !CONFIG_PID_NS
  pid_namespace: disable_pid_allocation is unused for !CONFIG_PID_NS

 include/linux/pid.h           | 11 +++++++++++
 include/linux/pid_namespace.h | 20 ++++++++++++++++++++
 kernel/acct.c                 |  4 ++--
 kernel/pid.c                  | 24 +++++++++++++++---------
 4 files changed, 48 insertions(+), 11 deletions(-)

-- 
2.1.3

[RFC 1/4] pid_namespace: remove ->level and ->parent for !CONFIG_PID_NS

[Rasmus Villemoes]

When !CONFIG_PID_NS, there is only ever one struct pid_namespace, and
for that we know that ->parent is NULL and ->level is 0. Telling the
compiler that allows it to generate slightly smaller code.

Removing the members from struct pid_namespace only saves a few bytes,
but it also helps ensure that everyone outside kernel/pid_namespace.c
use the new accessors pid_ns_{parent,level}.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 include/linux/pid_namespace.h | 18 ++++++++++++++++++
 kernel/acct.c                 |  4 ++--
 kernel/pid.c                  | 18 +++++++++---------
 3 files changed, 29 insertions(+), 11 deletions(-)

diff --git a/include/linux/pid_namespace.h b/include/linux/pid_namespace.h
index 918b117a7cd3..03a84a0cdc75 100644
--- a/include/linux/pid_namespace.h
+++ b/include/linux/pid_namespace.h
@@ -29,8 +29,10 @@ struct pid_namespace {
 	unsigned int nr_hashed;
 	struct task_struct *child_reaper;
 	struct kmem_cache *pid_cachep;
+#ifdef CONFIG_PID_NS
 	unsigned int level;
 	struct pid_namespace *parent;
+#endif
 #ifdef CONFIG_PROC_FS
 	struct vfsmount *proc_mnt;
 	struct dentry *proc_self;
@@ -58,6 +60,14 @@ static inline struct pid_namespace *get_pid_ns(struct pid_namespace *ns)
 		kref_get(&ns->kref);
 	return ns;
 }
+static inline struct pid_namespace *pid_ns_parent(struct pid_namespace *ns)
+{
+	return ns->parent;
+}
+static inline int pid_ns_level(struct pid_namespace *ns)
+{
+	return ns->level;
+}
 
 extern struct pid_namespace *copy_pid_ns(unsigned long flags,
 	struct user_namespace *user_ns, struct pid_namespace *ns);
@@ -72,6 +82,14 @@ static inline struct pid_namespace *get_pid_ns(struct pid_namespace *ns)
 {
 	return ns;
 }
+static inline struct pid_namespace *pid_ns_parent(struct pid_namespace *ns)
+{
+	return NULL;
+}
+static inline int pid_ns_level(struct pid_namespace *ns)
+{
+	return 0;
+}
 
 static inline struct pid_namespace *copy_pid_ns(unsigned long flags,
 	struct user_namespace *user_ns, struct pid_namespace *ns)
diff --git a/kernel/acct.c b/kernel/acct.c
index 74963d192c5d..43e570feea45 100644
--- a/kernel/acct.c
+++ b/kernel/acct.c
@@ -569,7 +569,7 @@ void acct_collect(long exitcode, int group_dead)
 
 static void slow_acct_process(struct pid_namespace *ns)
 {
-	for ( ; ns; ns = ns->parent) {
+	for ( ; ns; ns = pid_ns_parent(ns)) {
 		struct bsd_acct_struct *acct = acct_get(ns);
 		if (acct) {
 			do_acct_process(acct);
@@ -593,7 +593,7 @@ void acct_process(void)
 	 * alive and holds its namespace, which in turn holds
 	 * its parent.
 	 */
-	for (ns = task_active_pid_ns(current); ns != NULL; ns = ns->parent) {
+	for (ns = task_active_pid_ns(current); ns != NULL; ns = pid_ns_parent(ns)) {
 		if (ns->bacct)
 			break;
 	}
diff --git a/kernel/pid.c b/kernel/pid.c
index ca368793808e..909d3dcd8085 100644
--- a/kernel/pid.c
+++ b/kernel/pid.c
@@ -76,12 +76,12 @@ struct pid_namespace init_pid_ns = {
 	},
 	.last_pid = 0,
 	.nr_hashed = PIDNS_HASH_ADDING,
-	.level = 0,
 	.child_reaper = &init_task,
 	.user_ns = &init_user_ns,
 	.ns.inum = PROC_PID_INIT_INO,
 #ifdef CONFIG_PID_NS
 	.ns.ops = &pidns_operations,
+	.level = 0,
 #endif
 };
 EXPORT_SYMBOL_GPL(init_pid_ns);
@@ -308,8 +308,8 @@ struct pid *alloc_pid(struct pid_namespace *ns)
 		return ERR_PTR(retval);
 
 	tmp = ns;
-	pid->level = ns->level;
-	for (i = ns->level; i >= 0; i--) {
+	pid->level = pid_ns_level(ns);
+	for (i = pid_ns_level(ns); i >= 0; i--) {
 		nr = alloc_pidmap(tmp);
 		if (IS_ERR_VALUE(nr)) {
 			retval = nr;
@@ -318,7 +318,7 @@ struct pid *alloc_pid(struct pid_namespace *ns)
 
 		pid->numbers[i].nr = nr;
 		pid->numbers[i].ns = tmp;
-		tmp = tmp->parent;
+		tmp = pid_ns_parent(tmp);
 	}
 
 	if (unlikely(is_child_reaper(pid))) {
@@ -331,7 +331,7 @@ struct pid *alloc_pid(struct pid_namespace *ns)
 	for (type = 0; type < PIDTYPE_MAX; ++type)
 		INIT_HLIST_HEAD(&pid->tasks[type]);
 
-	upid = pid->numbers + ns->level;
+	upid = pid->numbers + pid_ns_level(ns);
 	spin_lock_irq(&pidmap_lock);
 	if (!(ns->nr_hashed & PIDNS_HASH_ADDING))
 		goto out_unlock;
@@ -349,7 +349,7 @@ out_unlock:
 	put_pid_ns(ns);
 
 out_free:
-	while (++i <= ns->level)
+	while (++i <= pid_ns_level(ns))
 		free_pidmap(pid->numbers + i);
 
 	kmem_cache_free(ns->pid_cachep, pid);
@@ -371,7 +371,7 @@ struct pid *find_pid_ns(int nr, struct pid_namespace *ns)
 			&pid_hash[pid_hashfn(nr, ns)], pid_chain)
 		if (pnr->nr == nr && pnr->ns == ns)
 			return container_of(pnr, struct pid,
-					numbers[ns->level]);
+					numbers[pid_ns_level(ns)]);
 
 	return NULL;
 }
@@ -502,8 +502,8 @@ pid_t pid_nr_ns(struct pid *pid, struct pid_namespace *ns)
 	struct upid *upid;
 	pid_t nr = 0;
 
-	if (pid && ns->level <= pid->level) {
-		upid = &pid->numbers[ns->level];
+	if (pid && pid_ns_level(ns) <= pid->level) {
+		upid = &pid->numbers[pid_ns_level(ns)];
 		if (upid->ns == ns)
 			nr = upid->nr;
 	}
-- 
2.1.3

[RFC 2/4] pid_namespace: remove .kref field for !CONFIG_PID_NS

[Rasmus Villemoes]

When there's only one global pid_namespace (init_pid_ns), the kref
field is unused (since both get_pid_ns and put_pid_ns are
no-ops). While we only save a few bytes by removing it, it could also
help catch users manipulating ->kref directly.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 include/linux/pid_namespace.h | 2 ++
 kernel/pid.c                  | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/include/linux/pid_namespace.h b/include/linux/pid_namespace.h
index 03a84a0cdc75..1cac17eb0775 100644
--- a/include/linux/pid_namespace.h
+++ b/include/linux/pid_namespace.h
@@ -22,7 +22,9 @@ struct pidmap {
 struct fs_pin;
 
 struct pid_namespace {
+#ifdef CONFIG_PID_NS
 	struct kref kref;
+#endif
 	struct pidmap pidmap[PIDMAP_ENTRIES];
 	struct rcu_head rcu;
 	int last_pid;
diff --git a/kernel/pid.c b/kernel/pid.c
index 909d3dcd8085..25332679a4d3 100644
--- a/kernel/pid.c
+++ b/kernel/pid.c
@@ -68,9 +68,11 @@ static inline int mk_pid(struct pid_namespace *pid_ns,
  * the scheme scales to up to 4 million PIDs, runtime.
  */
 struct pid_namespace init_pid_ns = {
+#ifdef CONFIG_PID_NS
 	.kref = {
 		.refcount       = ATOMIC_INIT(2),
 	},
+#endif
 	.pidmap = {
 		[ 0 ... PIDMAP_ENTRIES-1] = { ATOMIC_INIT(BITS_PER_PAGE), NULL }
 	},
-- 
2.1.3

[RFC 3/4] pid_namespace: make pid_nr_ns and pid_vnr static inlines for !CONFIG_PID_NS

[Rasmus Villemoes]

When there is only a single pid namespace, pid_nr_ns and pid_vnr
perform the same task as pid_nr, so we can save a little .text by
making them simple inlines.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 include/linux/pid.h | 11 +++++++++++
 kernel/pid.c        |  2 ++
 2 files changed, 13 insertions(+)

diff --git a/include/linux/pid.h b/include/linux/pid.h
index 23705a53abba..9aba74a90a64 100644
--- a/include/linux/pid.h
+++ b/include/linux/pid.h
@@ -169,8 +169,19 @@ static inline pid_t pid_nr(struct pid *pid)
 	return nr;
 }
 
+#ifdef CONFIG_PID_NS
 pid_t pid_nr_ns(struct pid *pid, struct pid_namespace *ns);
 pid_t pid_vnr(struct pid *pid);
+#else
+static inline pid_t pid_nr_ns(struct pid *pid, struct pid_namespace *ns)
+{
+	return pid_nr(pid);
+}
+static inline pid_t pid_vnr(struct pid *pid)
+{
+	return pid_nr(pid);
+}
+#endif
 
 #define do_each_pid_task(pid, type, task)				\
 	do {								\
diff --git a/kernel/pid.c b/kernel/pid.c
index 25332679a4d3..d422ddae0484 100644
--- a/kernel/pid.c
+++ b/kernel/pid.c
@@ -499,6 +499,7 @@ struct pid *find_get_pid(pid_t nr)
 }
 EXPORT_SYMBOL_GPL(find_get_pid);
 
+#ifdef CONFIG_PID_NS
 pid_t pid_nr_ns(struct pid *pid, struct pid_namespace *ns)
 {
 	struct upid *upid;
@@ -518,6 +519,7 @@ pid_t pid_vnr(struct pid *pid)
 	return pid_nr_ns(pid, task_active_pid_ns(current));
 }
 EXPORT_SYMBOL_GPL(pid_vnr);
+#endif
 
 pid_t __task_pid_nr_ns(struct task_struct *task, enum pid_type type,
 			struct pid_namespace *ns)
-- 
2.1.3

[RFC 4/4] pid_namespace: disable_pid_allocation is unused for !CONFIG_PID_NS

[Rasmus Villemoes]

disable_pid_allocation only has a single caller, in
kernel/pid_namespace.c, so when CONFIG_PID_NS=n we don't need it. I
don't think we need to #ifdef the declaration away, since we'll just
fail at link time rather than compile time, should
disable_pid_allocation ever grow a caller outside pid_namespace.c.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 kernel/pid.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/pid.c b/kernel/pid.c
index d422ddae0484..c9b595517c4f 100644
--- a/kernel/pid.c
+++ b/kernel/pid.c
@@ -358,12 +358,14 @@ out_free:
 	return ERR_PTR(retval);
 }
 
+#ifdef CONFIG_PID_NS
 void disable_pid_allocation(struct pid_namespace *ns)
 {
 	spin_lock_irq(&pidmap_lock);
 	ns->nr_hashed &= ~PIDNS_HASH_ADDING;
 	spin_unlock_irq(&pidmap_lock);
 }
+#endif
 
 struct pid *find_pid_ns(int nr, struct pid_namespace *ns)
 {
-- 
2.1.3

Re: [RFC 0/4] reduce pid_namespace footprint when not configured

[Eric W. Biederman]

Rasmus Villemoes <linux@rasmusvillemoes.dk> writes:

> A few suggestions for reducing the footprint of pid namespaces when
> CONFIG_PID_NS=n. The savings are admittedly not huge:
>
> $ scripts/bloat-o-meter /tmp/vmlinux.master vmlinux
> add/remove: 0/7 grow/shrink: 30/25 up/down: 368/-1032 (-664)

#ifdefs in C files.
More code to maintain.
Very minor savings.

I don't think it is worth the pain.  If anything I would prefer to
remove CONFIG_PID_NS.  The CONFIG_PID_NS ifdef really is about keeping
from exposing people to the feature tha don't use it more than it is
about size reduction.

Or do you have a case where this makes the difference between a usable
kernel and a kernel that is not?

Eric


> Rasmus Villemoes (4):
>   pid_namespace: remove ->level and ->parent for !CONFIG_PID_NS
>   pid_namespace: remove .kref field for !CONFIG_PID_NS
>   pid_namespace: make pid_nr_ns and pid_vnr static inlines for
>     !CONFIG_PID_NS
>   pid_namespace: disable_pid_allocation is unused for !CONFIG_PID_NS
>
>  include/linux/pid.h           | 11 +++++++++++
>  include/linux/pid_namespace.h | 20 ++++++++++++++++++++
>  kernel/acct.c                 |  4 ++--
>  kernel/pid.c                  | 24 +++++++++++++++---------
>  4 files changed, 48 insertions(+), 11 deletions(-)

Re: [RFC 0/4] reduce pid_namespace footprint when not configured

[Rasmus Villemoes]

On Wed, Sep 30 2015, ebiederm@xmission.com (Eric W. Biederman) wrote:

> Rasmus Villemoes <linux@rasmusvillemoes.dk> writes:
>
>> A few suggestions for reducing the footprint of pid namespaces when
>> CONFIG_PID_NS=n. The savings are admittedly not huge:
>>
>> $ scripts/bloat-o-meter /tmp/vmlinux.master vmlinux
>> add/remove: 0/7 grow/shrink: 30/25 up/down: 368/-1032 (-664)
>
> #ifdefs in C files.
> More code to maintain.
> Very minor savings.
>
> I don't think it is worth the pain.  If anything I would prefer to
> remove CONFIG_PID_NS.  The CONFIG_PID_NS ifdef really is about keeping
> from exposing people to the feature tha don't use it more than it is
> about size reduction.
>
> Or do you have a case where this makes the difference between a usable
> kernel and a kernel that is not?

No, it was mostly a small tinification effort, and we'd presumably also
save a few cycles on thread creation, but not anything measurable.

Rasmus