[RFC] backports: add remove_proc_subtree() backport

[RFC] backports: add remove_proc_subtree() backport

[Luis R. Rodriguez]

From: "Luis R. Rodriguez" <mcgrof@do-not-panic.com>

Intorduced in next-20130429 and Linus has merged it
onto v3.10-rc1. Backport this for older kernels.

The new in-kernel implementation relies on internal
mechanisms for parent directory parsing and then traversal
but these rely on internal procfs locking primatives which
we cannot use externally. This limits our implementation
with the assumption your procfs implementation is correct.

This implementation uses recursion then if subdirs are found
otherwise it treats it as a regular remove_proc_entry()

commit 8ce584c7416d8a85a6f3edc17d1cddefe331e87e
Author: Al Viro <viro@zeniv.linux.org.uk>
Date:   Sat Mar 30 20:13:46 2013 -0400

    procfs: add proc_remove_subtree()

    just what it sounds like; do that only to procfs subtrees you've
    created - doing that to something shared with another driver is
    not only antisocial, but might cause interesting races with
    proc_create() and its ilk.

    Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

Cc: viro@zeniv.linux.org.uk
Signed-off-by: Luis R. Rodriguez <mcgrof@do-not-panic.com>
---

Al, I tried to backport remove_proc_subtree() as best as
I could but I am not confident on this solution and would
appreciate a review on your part if possible.

 backport/backport-include/linux/proc_fs.h |    7 +++++
 backport/compat/backport-3.10.c           |   48 +++++++++++++++++++++++++++--
 2 files changed, 52 insertions(+), 3 deletions(-)

diff --git a/backport/backport-include/linux/proc_fs.h b/backport/backport-include/linux/proc_fs.h
index 5a1bec1..a85b4fb 100644
--- a/backport/backport-include/linux/proc_fs.h
+++ b/backport/backport-include/linux/proc_fs.h
@@ -12,6 +12,13 @@ static inline void *PDE_DATA(const struct inode *inode)
 {
 	return PROC_I(inode)->pde->data;
 }
+
+#ifdef CONFIG_PROC_FS
+extern int remove_proc_subtree(const char *name, struct proc_dir_entry *parent);
+#else
+#define remove_proc_subtree(name, parent) do {} while (0)
+#endif /* CONFIG_PROC_FS */
+
 #endif
 
 #endif /* __BACKPORT_PROC_FS_H */
diff --git a/backport/compat/backport-3.10.c b/backport/compat/backport-3.10.c
index 69c3788..0ddffbf 100644
--- a/backport/compat/backport-3.10.c
+++ b/backport/compat/backport-3.10.c
@@ -8,14 +8,17 @@
  * published by the Free Software Foundation.
  */
 
-#if (LINUX_VERSION_CODE >= KERNEL_VERSION(3,6,0))
 #include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/err.h>
+#include <linux/proc_fs.h>
+
+#if (LINUX_VERSION_CODE >= KERNEL_VERSION(3,6,0))
 #include <linux/init.h>
 #include <linux/debugfs.h>
 #include <linux/device.h>
 #include <linux/slab.h>
 #include <linux/async.h>
-#include <linux/err.h>
 #include <linux/mutex.h>
 #include <linux/suspend.h>
 #include <linux/delay.h>
@@ -26,8 +29,47 @@
 #include <linux/regulator/consumer.h>
 #include <linux/regulator/driver.h>
 #include <linux/regulator/machine.h>
-#include <linux/module.h>
+#endif /* (LINUX_VERSION_CODE >= KERNEL_VERSION(3,6,0)) */
+
+#if LINUX_VERSION_CODE < KERNEL_VERSION(3,10,0)
+#ifdef CONFIG_PROC_FS
 
+static void backport_proc_subdir_remove(struct proc_dir_entry *dir)
+{
+	struct proc_dir_entry *pe, *tmp;
+	pe = dir->subdir;
+	while (pe) {
+		tmp = pe->next;
+		backport_proc_subdir_remove(pe);
+		remove_proc_entry(pe->name, dir);
+		pe = tmp;
+	}
+};
+
+int remove_proc_subtree(const char *name, struct proc_dir_entry *parent)
+{
+	struct proc_dir_entry *pe, *tmp;
+
+	if (!parent)
+		goto out;
+
+	pe = parent->subdir;
+	while (pe) {
+		tmp = pe->next;
+		backport_proc_subdir_remove(pe);
+		remove_proc_entry(pe->name, parent);
+		pe = tmp;
+	}
+
+out:
+	remove_proc_entry(name, parent);
+
+	return 0;
+}
+#endif /* CONFIG_PROC_FS */
+#endif /* LINUX_VERSION_CODE < KERNEL_VERSION(3,10,0) */
+
+#if (LINUX_VERSION_CODE >= KERNEL_VERSION(3,6,0))
 /**
  * regulator_map_voltage_ascend - map_voltage() for ascendant voltage list
  *
-- 
1.7.10.4

Re: [RFC] backports: add remove_proc_subtree() backport

[Al Viro]

On Sun, May 12, 2013 at 09:31:38PM -0700, Luis R. Rodriguez wrote:

> This implementation uses recursion then if subdirs are found
> otherwise it treats it as a regular remove_proc_entry()

Ugh...  What for?  It's not as if traversing the damn thing had
been complicated:
	de = root = root of subtree to be killed
	unlink de
	while true
		/* de is already unlinked */
		if de has children
			child = first child of de
			unlink child
			de = child
		else
			/* de can be killed now */
			parent = parent of de
			kill de
			if de == root
				return
			de = parent
and that's it.  Why bother with recursion, chew stack space, etc.?
We do depth-first walk through the tree, unlinking the nodes from the
lists of children on the way in and freeing them on the way out.

The difference from your variant is that you use the stack to hold
pointers to ancestors of the current victim.  You get to the parent
of said victim by discarding a stack frame.  No need, since the
victim contained an explicit pointer to its parent...

Re: [RFC] backports: add remove_proc_subtree() backport

[Luis R. Rodriguez]

On Sun, May 12, 2013 at 10:03 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Sun, May 12, 2013 at 09:31:38PM -0700, Luis R. Rodriguez wrote:
>
>> This implementation uses recursion then if subdirs are found
>> otherwise it treats it as a regular remove_proc_entry()
>
> Ugh...  What for?

I could not figure out a way to traverse the tree safely without being
able to use the proc_subdir_lock spinlock. Given that the way we
backport is through an external module I had to look at what ways this
could be done safely.

> It's not as if traversing the damn thing had
> been complicated:
>         de = root = root of subtree to be killed
>         unlink de
>         while true
>                 /* de is already unlinked */
>                 if de has children
>                         child = first child of de
>                         unlink child
>                         de = child
>                 else
>                         /* de can be killed now */
>                         parent = parent of de
>                         kill de
>                         if de == root
>                                 return
>                         de = parent
> and that's it.  Why bother with recursion, chew stack space, etc.?

I would pick up the same implementation if we could but not having
access to the locks and also in the case of NULL being passed as the
parent, which some drivers do pass I couldn't see how to easily get
access to &proc_root. So this is what I came up with and so I wanted
to check if perhaps I missed something.

> We do depth-first walk through the tree, unlinking the nodes from the
> lists of children on the way in and freeing them on the way out.

OK thanks for the clarification.

> The difference from your variant is that you use the stack to hold
> pointers to ancestors of the current victim.  You get to the parent
> of said victim by discarding a stack frame.  No need, since the
> victim contained an explicit pointer to its parent...

Except I thought NULL was passed?

  Luis

Re: [RFC] backports: add remove_proc_subtree() backport

[Al Viro]

On Sun, May 12, 2013 at 10:21:02PM -0700, Luis R. Rodriguez wrote:

> > The difference from your variant is that you use the stack to hold
> > pointers to ancestors of the current victim.  You get to the parent
> > of said victim by discarding a stack frame.  No need, since the
> > victim contained an explicit pointer to its parent...
> 
> Except I thought NULL was passed?

AFAICS, your variant removes _everything_ in the parent.  IOW, instead
of
	rm -rf $ROOT/$RELATIVE_PATH
you do
	test -n $ROOT && rm -rf $ROOT/*
	rm $ROOT/$RELATIVE_PATH
which is not the same thing...

Re: [RFC] backports: add remove_proc_subtree() backport

[Luis R. Rodriguez]

On Sun, May 12, 2013 at 10:40 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Sun, May 12, 2013 at 10:21:02PM -0700, Luis R. Rodriguez wrote:
>
>> > The difference from your variant is that you use the stack to hold
>> > pointers to ancestors of the current victim.  You get to the parent
>> > of said victim by discarding a stack frame.  No need, since the
>> > victim contained an explicit pointer to its parent...
>>
>> Except I thought NULL was passed?
>
> AFAICS, your variant removes _everything_ in the parent.  IOW, instead
> of
>         rm -rf $ROOT/$RELATIVE_PATH
> you do
>         test -n $ROOT && rm -rf $ROOT/*
>         rm $ROOT/$RELATIVE_PATH
> which is not the same thing...

Crap, yes, I see now thanks. I can't see how we can safely traverse
the tree to find the respective struct proc_dir_entry for the passed
name as a modular solution. That is, in order to rm -rf
$ROOT/$RELATIVE_PATH I first need the $ROOT/$RELATIVE_PATH struct
proc_dir_entry.

Any recommendations?

  Luis