linux-next: proc tree build failure

linux-next: proc tree build failure

[Stephen Rothwell]

Hi Alexey,

Today's linux-next build (powerpc ppc64_defconfig) failed like this:

drivers/net/pppoe.c: In function 'pppoe_seq_open':
drivers/net/pppoe.c:1102: error: implicit declaration of function 'PDE_NET'
drivers/net/pppol2tp.c: In function 'pppol2tp_proc_open':
drivers/net/pppol2tp.c:2579: error: implicit declaration of function 'PDE_NET'

Caused by commits a6bcf1c1d38e0672db35e0d9f2504ac04ddf3ed5 ("net: pppoe -
introduce net-namespace functionality") and
4e9fb8016a351b5b9da7fea32bcfdbc9d836e421 ("net: pppol2tp - introduce
net-namespace functionality") from the net tree interacting with commit
0e6a2bfcbae4ee3cf770a6a5da203b4a336ff8ff ("proc 5/6: simplify network
namespace lookup") from the proc tree.

I added the following fix to the merge and can carry it as necessary.

I expect that there is a better fix for this, though.  These are the only
references to PDE_NET outside fs/proc.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

diff --git a/drivers/net/pppoe.c b/drivers/net/pppoe.c
index 798b8cf..3aabf92 100644
--- a/drivers/net/pppoe.c
+++ b/drivers/net/pppoe.c
@@ -1099,7 +1099,7 @@ static int pppoe_seq_open(struct inode *inode, struct file *file)
 		return err;
 
 	m = file->private_data;
-	net = maybe_get_net(PDE_NET(PDE(inode)));
+	net = maybe_get_net(inode->i_sb->s_fs_info);
 	BUG_ON(!net);
 	m->private = net;
 
diff --git a/drivers/net/pppol2tp.c b/drivers/net/pppol2tp.c
index 056e22a..d21470f 100644
--- a/drivers/net/pppol2tp.c
+++ b/drivers/net/pppol2tp.c
@@ -2576,7 +2576,7 @@ static int pppol2tp_proc_open(struct inode *inode, struct file *file)
 		goto out;
 
 	pd = m->private;
-	net = maybe_get_net(PDE_NET(PDE(inode)));
+	net = maybe_get_net(inode->i_sb->s_fs_info);
 	BUG_ON(!net);
 	pd->seq_net = net;
 	return 0;

Re: linux-next: proc tree build failure

[Cyrill Gorcunov]

[Stephen Rothwell - Fri, Jan 23, 2009 at 05:15:15PM +1100]
| Hi Alexey,
| 
| Today's linux-next build (powerpc ppc64_defconfig) failed like this:
| 
| drivers/net/pppoe.c: In function 'pppoe_seq_open':
| drivers/net/pppoe.c:1102: error: implicit declaration of function 'PDE_NET'
| drivers/net/pppol2tp.c: In function 'pppol2tp_proc_open':
| drivers/net/pppol2tp.c:2579: error: implicit declaration of function 'PDE_NET'
| 
| Caused by commits a6bcf1c1d38e0672db35e0d9f2504ac04ddf3ed5 ("net: pppoe -
| introduce net-namespace functionality") and
| 4e9fb8016a351b5b9da7fea32bcfdbc9d836e421 ("net: pppol2tp - introduce
| net-namespace functionality") from the net tree interacting with commit
| 0e6a2bfcbae4ee3cf770a6a5da203b4a336ff8ff ("proc 5/6: simplify network
| namespace lookup") from the proc tree.
| 
| I added the following fix to the merge and can carry it as necessary.
| 
| I expect that there is a better fix for this, though.  These are the only
| references to PDE_NET outside fs/proc.
| -- 
| Cheers,
| Stephen Rothwell                    sfr@canb.auug.org.au
| http://www.canb.auug.org.au/~sfr/
| 
| diff --git a/drivers/net/pppoe.c b/drivers/net/pppoe.c
| index 798b8cf..3aabf92 100644
| --- a/drivers/net/pppoe.c
| +++ b/drivers/net/pppoe.c
| @@ -1099,7 +1099,7 @@ static int pppoe_seq_open(struct inode *inode, struct file *file)
|  		return err;
|  
|  	m = file->private_data;
| -	net = maybe_get_net(PDE_NET(PDE(inode)));
| +	net = maybe_get_net(inode->i_sb->s_fs_info);
|  	BUG_ON(!net);
|  	m->private = net;
|  
| diff --git a/drivers/net/pppol2tp.c b/drivers/net/pppol2tp.c
| index 056e22a..d21470f 100644
| --- a/drivers/net/pppol2tp.c
| +++ b/drivers/net/pppol2tp.c
| @@ -2576,7 +2576,7 @@ static int pppol2tp_proc_open(struct inode *inode, struct file *file)
|  		goto out;
|  
|  	pd = m->private;
| -	net = maybe_get_net(PDE_NET(PDE(inode)));
| +	net = maybe_get_net(inode->i_sb->s_fs_info);
|  	BUG_ON(!net);
|  	pd->seq_net = net;
|  	return 0;
| 

Hi Stephen,

thanks for catching this. Until I miss something --
s_fs_info for /proc is struct pid_namespace* right?
So maybe_get_net will convert it to net*, which is
not right I guess. On the other hand -- and PDE, and
PDE_NET is defined in proc_fs.h regardless if CONIG_PROC_FS
is turned on/off. Confused...

(to be fair -- this maybe_get_net(PDE_NET(PDE(inode))) is
 taken from proc_net.c:get_proc_net, but to eliminate #ifdef
 I reimplemented it, which is not good too -- maybe I've been
 to put get_proc_net into some header, Eric?)

		- Cyrill -

Re: linux-next: proc tree build failure

[Alexey Dobriyan]

On Fri, Jan 23, 2009 at 10:09:55AM +0300, Cyrill Gorcunov wrote:
> [Stephen Rothwell - Fri, Jan 23, 2009 at 05:15:15PM +1100]
> | Hi Alexey,
> | 
> | Today's linux-next build (powerpc ppc64_defconfig) failed like this:
> | 
> | drivers/net/pppoe.c: In function 'pppoe_seq_open':
> | drivers/net/pppoe.c:1102: error: implicit declaration of function 'PDE_NET'
> | drivers/net/pppol2tp.c: In function 'pppol2tp_proc_open':
> | drivers/net/pppol2tp.c:2579: error: implicit declaration of function 'PDE_NET'
> | 
> | Caused by commits a6bcf1c1d38e0672db35e0d9f2504ac04ddf3ed5 ("net: pppoe -
> | introduce net-namespace functionality") and
> | 4e9fb8016a351b5b9da7fea32bcfdbc9d836e421 ("net: pppol2tp - introduce
> | net-namespace functionality") from the net tree interacting with commit
> | 0e6a2bfcbae4ee3cf770a6a5da203b4a336ff8ff ("proc 5/6: simplify network
> | namespace lookup") from the proc tree.
> | 
> | I added the following fix to the merge and can carry it as necessary.
> | 
> | I expect that there is a better fix for this, though.  These are the only
> | references to PDE_NET outside fs/proc.
> | -- 
> | Cheers,
> | Stephen Rothwell                    sfr@canb.auug.org.au
> | http://www.canb.auug.org.au/~sfr/
> | 
> | diff --git a/drivers/net/pppoe.c b/drivers/net/pppoe.c
> | index 798b8cf..3aabf92 100644
> | --- a/drivers/net/pppoe.c
> | +++ b/drivers/net/pppoe.c
> | @@ -1099,7 +1099,7 @@ static int pppoe_seq_open(struct inode *inode, struct file *file)
> |  		return err;
> |  
> |  	m = file->private_data;
> | -	net = maybe_get_net(PDE_NET(PDE(inode)));
> | +	net = maybe_get_net(inode->i_sb->s_fs_info);
> |  	BUG_ON(!net);
> |  	m->private = net;
> |  
> | diff --git a/drivers/net/pppol2tp.c b/drivers/net/pppol2tp.c
> | index 056e22a..d21470f 100644
> | --- a/drivers/net/pppol2tp.c
> | +++ b/drivers/net/pppol2tp.c
> | @@ -2576,7 +2576,7 @@ static int pppol2tp_proc_open(struct inode *inode, struct file *file)
> |  		goto out;
> |  
> |  	pd = m->private;
> | -	net = maybe_get_net(PDE_NET(PDE(inode)));
> | +	net = maybe_get_net(inode->i_sb->s_fs_info);
> |  	BUG_ON(!net);
> |  	pd->seq_net = net;
> |  	return 0;
> | 
> 
> Hi Stephen,
> 
> thanks for catching this. Until I miss something --
> s_fs_info for /proc is struct pid_namespace* right?
> So maybe_get_net will convert it to net*, which is
> not right I guess. On the other hand -- and PDE, and
> PDE_NET is defined in proc_fs.h regardless if CONIG_PROC_FS
> is turned on/off. Confused...
> 
> (to be fair -- this maybe_get_net(PDE_NET(PDE(inode))) is
>  taken from proc_net.c:get_proc_net, but to eliminate #ifdef
>  I reimplemented it, which is not good too -- maybe I've been
>  to put get_proc_net into some header, Eric?)

PDE_NET here is bogus. you should use seq_open_net/seq_release_net.
See how other code does this, plenty of examples.

Re: linux-next: proc tree build failure

[Cyrill Gorcunov]

[Alexey Dobriyan - Fri, Jan 23, 2009 at 09:48:37PM +0300]
...
| > Hi Stephen,
| > 
| > thanks for catching this. Until I miss something --
| > s_fs_info for /proc is struct pid_namespace* right?
| > So maybe_get_net will convert it to net*, which is
| > not right I guess. On the other hand -- and PDE, and
| > PDE_NET is defined in proc_fs.h regardless if CONIG_PROC_FS
| > is turned on/off. Confused...
| > 
| > (to be fair -- this maybe_get_net(PDE_NET(PDE(inode))) is
| >  taken from proc_net.c:get_proc_net, but to eliminate #ifdef
| >  I reimplemented it, which is not good too -- maybe I've been
| >  to put get_proc_net into some header, Eric?)
| 
| PDE_NET here is bogus. you should use seq_open_net/seq_release_net.
| See how other code does this, plenty of examples.
| 

The question is not in examples, I saw that code.
There is some problem with seq_open_net/seq_release_net --
I don't need additional private data for sequential files,
all I need is just a pointer (ie -- seq_file.private) which
is already allocated for seq_file structure itself. So
Alexey, why should I use seq_open_net then and ask for
additional memory which I don't need? (side note:
actually get_net/put_net is really needed here indeed).

		- Cyrill -

Re: linux-next: proc tree build failure

[Cyrill Gorcunov]

[Cyrill Gorcunov - Fri, Jan 23, 2009 at 10:26:07PM +0300]
| [Alexey Dobriyan - Fri, Jan 23, 2009 at 09:48:37PM +0300]
| ...
| | > Hi Stephen,
| | > 
| | > thanks for catching this. Until I miss something --
| | > s_fs_info for /proc is struct pid_namespace* right?
| | > So maybe_get_net will convert it to net*, which is
| | > not right I guess. On the other hand -- and PDE, and
| | > PDE_NET is defined in proc_fs.h regardless if CONIG_PROC_FS
| | > is turned on/off. Confused...
| | > 
| | > (to be fair -- this maybe_get_net(PDE_NET(PDE(inode))) is
| | >  taken from proc_net.c:get_proc_net, but to eliminate #ifdef
| | >  I reimplemented it, which is not good too -- maybe I've been
| | >  to put get_proc_net into some header, Eric?)
| | 
| | PDE_NET here is bogus. you should use seq_open_net/seq_release_net.
| | See how other code does this, plenty of examples.
| | 
| 
| The question is not in examples, I saw that code.
| There is some problem with seq_open_net/seq_release_net --
| I don't need additional private data for sequential files,
| all I need is just a pointer (ie -- seq_file.private) which
| is already allocated for seq_file structure itself. So
| Alexey, why should I use seq_open_net then and ask for
| additional memory which I don't need? (side note:
| actually get_net/put_net is really needed here indeed).
| 
| 		- Cyrill -

On the other hand - Alexey could we 'teach' seq_open_net/seq_release_net
to handle 0 private size? So then I could be able to set
seq_file.private to NULL
and call seq_release_net without problems?

		- Cyrill -

Re: linux-next: proc tree build failure

[Cyrill Gorcunov]

[Cyrill Gorcunov - Fri, Jan 23, 2009 at 10:41:09PM +0300]
| [Cyrill Gorcunov - Fri, Jan 23, 2009 at 10:26:07PM +0300]
| | [Alexey Dobriyan - Fri, Jan 23, 2009 at 09:48:37PM +0300]
| | ...
| | | > Hi Stephen,
| | | > 
| | | > thanks for catching this. Until I miss something --
| | | > s_fs_info for /proc is struct pid_namespace* right?
| | | > So maybe_get_net will convert it to net*, which is
| | | > not right I guess. On the other hand -- and PDE, and
| | | > PDE_NET is defined in proc_fs.h regardless if CONIG_PROC_FS
| | | > is turned on/off. Confused...
| | | > 
| | | > (to be fair -- this maybe_get_net(PDE_NET(PDE(inode))) is
| | | >  taken from proc_net.c:get_proc_net, but to eliminate #ifdef
| | | >  I reimplemented it, which is not good too -- maybe I've been
| | | >  to put get_proc_net into some header, Eric?)
| | | 
| | | PDE_NET here is bogus. you should use seq_open_net/seq_release_net.
| | | See how other code does this, plenty of examples.
| | | 
| | 
| | The question is not in examples, I saw that code.
| | There is some problem with seq_open_net/seq_release_net --
| | I don't need additional private data for sequential files,
| | all I need is just a pointer (ie -- seq_file.private) which
| | is already allocated for seq_file structure itself. So
| | Alexey, why should I use seq_open_net then and ask for
| | additional memory which I don't need? (side note:
| | actually get_net/put_net is really needed here indeed).
| | 
| | 		- Cyrill -
| 
| On the other hand - Alexey could we 'teach' seq_open_net/seq_release_net
| to handle 0 private size? So then I could be able to set
| seq_file.private to NULL
| and call seq_release_net without problems?
| 
| 		- Cyrill -

But if the only right way to play with this is to use seq_open/release_net
then I will change the PPPoX code. Alexey?

		- Cyrill -

[PATCH] pppol2tp: stop using proc internals

[Alexey Dobriyan]

On Fri, Jan 23, 2009 at 10:09:55AM +0300, Cyrill Gorcunov wrote:
> [Stephen Rothwell - Fri, Jan 23, 2009 at 05:15:15PM +1100]
> | Hi Alexey,
> | 
> | Today's linux-next build (powerpc ppc64_defconfig) failed like this:
> | 
> | drivers/net/pppoe.c: In function 'pppoe_seq_open':
> | drivers/net/pppoe.c:1102: error: implicit declaration of function 'PDE_NET'
> | drivers/net/pppol2tp.c: In function 'pppol2tp_proc_open':
> | drivers/net/pppol2tp.c:2579: error: implicit declaration of function 'PDE_NET'
> | 
> | Caused by commits a6bcf1c1d38e0672db35e0d9f2504ac04ddf3ed5 ("net: pppoe -
> | introduce net-namespace functionality") and
> | 4e9fb8016a351b5b9da7fea32bcfdbc9d836e421 ("net: pppol2tp - introduce
> | net-namespace functionality") from the net tree interacting with commit
> | 0e6a2bfcbae4ee3cf770a6a5da203b4a336ff8ff ("proc 5/6: simplify network
> | namespace lookup") from the proc tree.
> | 
> | I added the following fix to the merge and can carry it as necessary.
> | 
> | I expect that there is a better fix for this, though.  These are the only
> | references to PDE_NET outside fs/proc.
> | -- 
> | Cheers,
> | Stephen Rothwell                    sfr@canb.auug.org.au
> | http://www.canb.auug.org.au/~sfr/
> | 
> | diff --git a/drivers/net/pppoe.c b/drivers/net/pppoe.c
> | index 798b8cf..3aabf92 100644
> | --- a/drivers/net/pppoe.c
> | +++ b/drivers/net/pppoe.c
> | @@ -1099,7 +1099,7 @@ static int pppoe_seq_open(struct inode *inode, struct file *file)
> |  		return err;
> |  
> |  	m = file->private_data;
> | -	net = maybe_get_net(PDE_NET(PDE(inode)));
> | +	net = maybe_get_net(inode->i_sb->s_fs_info);
> |  	BUG_ON(!net);
> |  	m->private = net;
> |  
> | diff --git a/drivers/net/pppol2tp.c b/drivers/net/pppol2tp.c
> | index 056e22a..d21470f 100644
> | --- a/drivers/net/pppol2tp.c
> | +++ b/drivers/net/pppol2tp.c
> | @@ -2576,7 +2576,7 @@ static int pppol2tp_proc_open(struct inode *inode, struct file *file)
> |  		goto out;
> |  
> |  	pd = m->private;
> | -	net = maybe_get_net(PDE_NET(PDE(inode)));
> | +	net = maybe_get_net(inode->i_sb->s_fs_info);

Someone with ppp setup, please test this:



[PATCH] pppol2tp: stop using proc internals

PDE_NET usage in driver code is a sign and, indeed, switching
to seq_open_net/seq_release_net saves code and fixes bogus things, like
user triggerabble BUG_ON(!net) after maybe_get_net, and NULLifying ->private.

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---

 drivers/net/pppol2tp.c |   48 +++++-------------------------------------------
 1 file changed, 5 insertions(+), 43 deletions(-)

--- a/drivers/net/pppol2tp.c
+++ b/drivers/net/pppol2tp.c
@@ -2371,7 +2371,7 @@ end:
 #include <linux/seq_file.h>
 
 struct pppol2tp_seq_data {
-	struct net *seq_net;			/* net of inode */
+	struct seq_net_private p;
 	struct pppol2tp_tunnel *tunnel;		/* current tunnel */
 	struct pppol2tp_session *session;	/* NULL means get first session in tunnel */
 };
@@ -2436,7 +2436,7 @@ static void *pppol2tp_seq_start(struct seq_file *m, loff_t *offs)
 
 	BUG_ON(m->private == NULL);
 	pd = m->private;
-	pn = pppol2tp_pernet(pd->seq_net);
+	pn = pppol2tp_pernet(seq_file_net(m));
 
 	if (pd->tunnel == NULL) {
 		if (!list_empty(&pn->pppol2tp_tunnel_list))
@@ -2558,46 +2558,8 @@ static const struct seq_operations pppol2tp_seq_ops = {
  */
 static int pppol2tp_proc_open(struct inode *inode, struct file *file)
 {
-	struct seq_file *m;
-	struct pppol2tp_seq_data *pd;
-	struct net *net;
-	int ret = 0;
-
-	ret = seq_open(file, &pppol2tp_seq_ops);
-	if (ret < 0)
-		goto out;
-
-	m = file->private_data;
-
-	/* Allocate and fill our proc_data for access later */
-	ret = -ENOMEM;
-	m->private = kzalloc(sizeof(*pd), GFP_KERNEL);
-	if (m->private == NULL)
-		goto out;
-
-	pd = m->private;
-	net = maybe_get_net(PDE_NET(PDE(inode)));
-	BUG_ON(!net);
-	pd->seq_net = net;
-	return 0;
-
-out:
-	return ret;
-}
-
-/* Called when /proc file access completes.
- */
-static int pppol2tp_proc_release(struct inode *inode, struct file *file)
-{
-	struct seq_file *m = (struct seq_file *)file->private_data;
-	struct pppol2tp_seq_data *pd = m->private;
-
-	put_net(pd->seq_net);
-
-	kfree(m->private);
-	m->private = NULL;
-
-	return seq_release(inode, file);
+	return seq_open_net(inode, file, &pppol2tp_seq_ops,
+			    sizeof(struct pppol2tp_seq_data));
 }
 
 static const struct file_operations pppol2tp_proc_fops = {
@@ -2605,7 +2567,7 @@ static const struct file_operations pppol2tp_proc_fops = {
 	.open		= pppol2tp_proc_open,
 	.read		= seq_read,
 	.llseek		= seq_lseek,
-	.release	= pppol2tp_proc_release,
+	.release	= seq_release_net,
 };
 
 #endif /* CONFIG_PROC_FS */

Re: [PATCH] pppol2tp: stop using proc internals

[Cyrill Gorcunov]

[Alexey Dobriyan - Sun, Jan 25, 2009 at 01:00:48AM +0300]
| On Fri, Jan 23, 2009 at 10:09:55AM +0300, Cyrill Gorcunov wrote:
| > [Stephen Rothwell - Fri, Jan 23, 2009 at 05:15:15PM +1100]
| > | Hi Alexey,
| > | 
| > | Today's linux-next build (powerpc ppc64_defconfig) failed like this:
| > | 
| > | drivers/net/pppoe.c: In function 'pppoe_seq_open':
| > | drivers/net/pppoe.c:1102: error: implicit declaration of function 'PDE_NET'
| > | drivers/net/pppol2tp.c: In function 'pppol2tp_proc_open':
| > | drivers/net/pppol2tp.c:2579: error: implicit declaration of function 'PDE_NET'
| > | 
| > | Caused by commits a6bcf1c1d38e0672db35e0d9f2504ac04ddf3ed5 ("net: pppoe -
| > | introduce net-namespace functionality") and
| > | 4e9fb8016a351b5b9da7fea32bcfdbc9d836e421 ("net: pppol2tp - introduce
| > | net-namespace functionality") from the net tree interacting with commit
| > | 0e6a2bfcbae4ee3cf770a6a5da203b4a336ff8ff ("proc 5/6: simplify network
| > | namespace lookup") from the proc tree.
| > | 
| > | I added the following fix to the merge and can carry it as necessary.
| > | 
| > | I expect that there is a better fix for this, though.  These are the only
| > | references to PDE_NET outside fs/proc.
| > | -- 
| > | Cheers,
| > | Stephen Rothwell                    sfr@canb.auug.org.au
| > | http://www.canb.auug.org.au/~sfr/
| > | 
| > | diff --git a/drivers/net/pppoe.c b/drivers/net/pppoe.c
| > | index 798b8cf..3aabf92 100644
| > | --- a/drivers/net/pppoe.c
| > | +++ b/drivers/net/pppoe.c
| > | @@ -1099,7 +1099,7 @@ static int pppoe_seq_open(struct inode *inode, struct file *file)
| > |  		return err;
| > |  
| > |  	m = file->private_data;
| > | -	net = maybe_get_net(PDE_NET(PDE(inode)));
| > | +	net = maybe_get_net(inode->i_sb->s_fs_info);
| > |  	BUG_ON(!net);
| > |  	m->private = net;
| > |  
| > | diff --git a/drivers/net/pppol2tp.c b/drivers/net/pppol2tp.c
| > | index 056e22a..d21470f 100644
| > | --- a/drivers/net/pppol2tp.c
| > | +++ b/drivers/net/pppol2tp.c
| > | @@ -2576,7 +2576,7 @@ static int pppol2tp_proc_open(struct inode *inode, struct file *file)
| > |  		goto out;
| > |  
| > |  	pd = m->private;
| > | -	net = maybe_get_net(PDE_NET(PDE(inode)));
| > | +	net = maybe_get_net(inode->i_sb->s_fs_info);
| 
| Someone with ppp setup, please test this:
| 
| 
| 
| [PATCH] pppol2tp: stop using proc internals
| 
| PDE_NET usage in driver code is a sign and, indeed, switching
| to seq_open_net/seq_release_net saves code and fixes bogus things, like
| user triggerabble BUG_ON(!net) after maybe_get_net, and NULLifying ->private.
| 
| Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
| ---
| 
|  drivers/net/pppol2tp.c |   48 +++++-------------------------------------------
|  1 file changed, 5 insertions(+), 43 deletions(-)
| 
| --- a/drivers/net/pppol2tp.c
| +++ b/drivers/net/pppol2tp.c
| @@ -2371,7 +2371,7 @@ end:
|  #include <linux/seq_file.h>
|  
|  struct pppol2tp_seq_data {
| -	struct net *seq_net;			/* net of inode */
| +	struct seq_net_private p;
|  	struct pppol2tp_tunnel *tunnel;		/* current tunnel */
|  	struct pppol2tp_session *session;	/* NULL means get first session in tunnel */
|  };
| @@ -2436,7 +2436,7 @@ static void *pppol2tp_seq_start(struct seq_file *m, loff_t *offs)
|  
|  	BUG_ON(m->private == NULL);
|  	pd = m->private;
| -	pn = pppol2tp_pernet(pd->seq_net);
| +	pn = pppol2tp_pernet(seq_file_net(m));
|  
|  	if (pd->tunnel == NULL) {
|  		if (!list_empty(&pn->pppol2tp_tunnel_list))
| @@ -2558,46 +2558,8 @@ static const struct seq_operations pppol2tp_seq_ops = {
|   */
|  static int pppol2tp_proc_open(struct inode *inode, struct file *file)
|  {
| -	struct seq_file *m;
| -	struct pppol2tp_seq_data *pd;
| -	struct net *net;
| -	int ret = 0;
| -
| -	ret = seq_open(file, &pppol2tp_seq_ops);
| -	if (ret < 0)
| -		goto out;
| -
| -	m = file->private_data;
| -
| -	/* Allocate and fill our proc_data for access later */
| -	ret = -ENOMEM;
| -	m->private = kzalloc(sizeof(*pd), GFP_KERNEL);
| -	if (m->private == NULL)
| -		goto out;
| -
| -	pd = m->private;
| -	net = maybe_get_net(PDE_NET(PDE(inode)));
| -	BUG_ON(!net);
| -	pd->seq_net = net;
| -	return 0;
| -
| -out:
| -	return ret;
| -}
| -
| -/* Called when /proc file access completes.
| - */
| -static int pppol2tp_proc_release(struct inode *inode, struct file *file)
| -{
| -	struct seq_file *m = (struct seq_file *)file->private_data;
| -	struct pppol2tp_seq_data *pd = m->private;
| -
| -	put_net(pd->seq_net);
| -
| -	kfree(m->private);
| -	m->private = NULL;
| -
| -	return seq_release(inode, file);
| +	return seq_open_net(inode, file, &pppol2tp_seq_ops,
| +			    sizeof(struct pppol2tp_seq_data));
|  }
|  
|  static const struct file_operations pppol2tp_proc_fops = {
| @@ -2605,7 +2567,7 @@ static const struct file_operations pppol2tp_proc_fops = {
|  	.open		= pppol2tp_proc_open,
|  	.read		= seq_read,
|  	.llseek		= seq_lseek,
| -	.release	= pppol2tp_proc_release,
| +	.release	= seq_release_net,
|  };
|  
|  #endif /* CONFIG_PROC_FS */
| 

Thanks a lot Alexey! I'll fix PPPoE too.

		- Cyrill -

Re: [PATCH] pppol2tp: stop using proc internals

[David Miller]

From: Cyrill Gorcunov <gorcunov@gmail.com>
Date: Sun, 25 Jan 2009 11:47:18 +0300

> [Alexey Dobriyan - Sun, Jan 25, 2009 at 01:00:48AM +0300]
> | [PATCH] pppol2tp: stop using proc internals
> | 
> | PDE_NET usage in driver code is a sign and, indeed, switching
> | to seq_open_net/seq_release_net saves code and fixes bogus things, like
> | user triggerabble BUG_ON(!net) after maybe_get_net, and NULLifying ->private.
> | 
> | Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
 ...
> Thanks a lot Alexey! I'll fix PPPoE too.

Applied, thanks everyone.

linux-next: proc tree build failure

[Stephen Rothwell]

Hi Alexey,

Today's linux-next build (powerpc ppc64_defconfig) failed like this:

fs/proc/proc_net.c: In function 'proc_net_get_sb':
fs/proc/proc_net.c:296: error: void value not ignored as it ought to be

Caused by commit 39bc389a04412aa199d8abec6582be33d57d1344 ("proc 3/6:
make /proc/net it's own filesystem") interacting with commit
a3ec947c85ec339884b30ef6a08133e9311fdae1 ("vfs: simple_set_mnt() should
return void") from Linus' tree.

I applied the following patch and can carry it for a while.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Mon, 30 Mar 2009 17:41:54 +1100
Subject: [PATCH] proc: update proc_net.c for simple_set_mnt API change

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 fs/proc/proc_net.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/fs/proc/proc_net.c b/fs/proc/proc_net.c
index 32dd84f..1e31e5d 100644
--- a/fs/proc/proc_net.c
+++ b/fs/proc/proc_net.c
@@ -293,7 +293,8 @@ static int proc_net_get_sb(struct file_system_type *fs_type,
 		sb->s_flags |= MS_ACTIVE;
 	}
 
-	return simple_set_mnt(mnt, sb);
+	simple_set_mnt(mnt, sb);
+	return 0;
 }
 
 static struct file_system_type proc_net_fs_type = {
-- 
1.6.2.1

linux-next: proc tree build failure

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 465 bytes --]

Hi Alexey,

Today's linux-next build (sparc64 allmodconfig) failed like this:

fs/proc/base.c:362: undefined reference to `save_stack_trace_tsk'

It has been failing like this for some time.

Caused by commit 788ff43b53179f3af8677d59c87104aff1f75bde ("proc:
add /proc/*/stack") which has been fixed up for MIPS, but needs another
fix for Sparc.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: linux-next: proc tree build failure

[Alexey Dobriyan]

On Mon, Nov 24, 2008 at 06:15:59PM +1100, Stephen Rothwell wrote:
> Today's linux-next build (sparc64 allmodconfig) failed like this:
> 
> fs/proc/base.c:362: undefined reference to `save_stack_trace_tsk'
> 
> It has been failing like this for some time.
> 
> Caused by commit 788ff43b53179f3af8677d59c87104aff1f75bde ("proc:
> add /proc/*/stack") which has been fixed up for MIPS, but needs another
> fix for Sparc.

David, can you implement save_stack_trace_tsk() for sparc64? I have no idea
how to do it or is it doable at all.

Re: linux-next: proc tree build failure

[David Miller]

From: Alexey Dobriyan <adobriyan@gmail.com>
Date: Fri, 28 Nov 2008 11:05:55 +0300

> David, can you implement save_stack_trace_tsk() for sparc64? I have
> no idea how to do it or is it doable at all.

Sure thing Alexey.

Re: linux-next: proc tree build failure

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Fri, 28 Nov 2008 01:00:25 -0800 (PST)

> From: Alexey Dobriyan <adobriyan@gmail.com>
> Date: Fri, 28 Nov 2008 11:05:55 +0300
> 
> > David, can you implement save_stack_trace_tsk() for sparc64? I have
> > no idea how to do it or is it doable at all.
> 
> Sure thing Alexey.

I just pushed the following patch into sparc-next-2.6, so it
should show up in the next tree that Stephen builds:

sparc64: Add save_stack_trace_tsk().

And this allows us to indicate HAVE_LATENCYTOP_SUPPORT.

Signed-off-by: David S. Miller <davem@davemloft.net>
---
 arch/sparc/Kconfig               |    4 ++++
 arch/sparc64/kernel/stacktrace.c |   32 +++++++++++++++++++++++---------
 2 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index 10945c3..e162535 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -72,6 +72,10 @@ config LOCKDEP_SUPPORT
 	bool
 	default y if SPARC64
 
+config HAVE_LATENCYTOP_SUPPORT
+	bool
+	default y if SPARC64
+
 config AUDIT_ARCH
 	bool
 	default y if SPARC64
diff --git a/arch/sparc64/kernel/stacktrace.c b/arch/sparc64/kernel/stacktrace.c
index 4e21d4a..acb12f6 100644
--- a/arch/sparc64/kernel/stacktrace.c
+++ b/arch/sparc64/kernel/stacktrace.c
@@ -7,17 +7,18 @@
 
 #include "kstack.h"
 
-void save_stack_trace(struct stack_trace *trace)
+static void __save_stack_trace(struct thread_info *tp,
+			       struct stack_trace *trace,
+			       bool skip_sched)
 {
-	struct thread_info *tp = task_thread_info(current);
 	unsigned long ksp, fp;
 
-	stack_trace_flush();
-
-	__asm__ __volatile__(
-		"mov	%%fp, %0"
-		: "=r" (ksp)
-	);
+	if (tp == current_thread_info()) {
+		stack_trace_flush();
+		__asm__ __volatile__("mov %%fp, %0" : "=r" (ksp));
+	} else {
+		ksp = tp->ksp;
+	}
 
 	fp = ksp + STACK_BIAS;
 	do {
@@ -43,8 +44,21 @@ void save_stack_trace(struct stack_trace *trace)
 
 		if (trace->skip > 0)
 			trace->skip--;
-		else
+		else if (!skip_sched || !in_sched_functions(pc))
 			trace->entries[trace->nr_entries++] = pc;
 	} while (trace->nr_entries < trace->max_entries);
 }
+
+void save_stack_trace(struct stack_trace *trace)
+{
+	__save_stack_trace(current_thread_info(), trace, false);
+}
 EXPORT_SYMBOL_GPL(save_stack_trace);
+
+void save_stack_trace_tsk(struct task_struct *tsk, struct stack_trace *trace)
+{
+	struct thread_info *tp = task_thread_info(tsk);
+
+	__save_stack_trace(tp, trace, true);
+}
+EXPORT_SYMBOL_GPL(save_stack_trace_tsk);
-- 
1.5.6.5

linux-next: proc tree build failure

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 821 bytes --]

Hi Alexey,

Today's linux-next build (x86_64 allmodconfig) failed like this:

security/selinux/hooks.c: In function 'selinux_proc_get_sid':
security/selinux/hooks.c:1182: error: 'struct super_block' has no member named 'type'
security/selinux/hooks.c:1183: error: 'struct super_block' has no member named 'type'
security/selinux/hooks.c:1189: error: too few arguments to function 'memcpy'

Caused by commit e23b822350796e690a46873058d220878a18564c ("proc:
make /proc/net it's own filesystem"). This has clearly never been
compiled with CONFIG_PROC_FS set!  I have dropped the proc tree for today.
Much better testing is required - this should never have been submitted
to linux-next in this state.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: linux-next: proc tree build failure

[Eric W. Biederman]

Stephen Rothwell <sfr@canb.auug.org.au> writes:

> Hi Alexey,
>
> Today's linux-next build (x86_64 allmodconfig) failed like this:
>
> security/selinux/hooks.c: In function 'selinux_proc_get_sid':
> security/selinux/hooks.c:1182: error: 'struct super_block' has no member named
> 'type'
> security/selinux/hooks.c:1183: error: 'struct super_block' has no member named
> 'type'
> security/selinux/hooks.c:1189: error: too few arguments to function 'memcpy'
>
> Caused by commit e23b822350796e690a46873058d220878a18564c ("proc:
> make /proc/net it's own filesystem"). This has clearly never been
> compiled with CONFIG_PROC_FS set!

Nah.  Never compiled with CONFIG_SELINUX set.  I thought I had compile
tested that case but apparently not.

Sorry there was some stupid security drama going on when these patches
were written and that always seems to throws me off and reduces the
quality of my code.

Eric

linux-next: proc tree build failure

[Stephen Rothwell]

Hi Alexey,

Today's linux-next build (powerpc ppc64_defconfig) failed like this:

fs/proc/base.c:2547: error: 'pid_stack' undeclared here (not in a function)

Caused by an interaction of commits
078e9901c02d4899db5b24f708b8dfbca8935891 ("stacktrace:
add /proc/<pid>/stack to dump task's stack trace") and
35f0b5fd7fab907a1119eaa614d9b24e5e225755 ("stacktrace:
convert /proc/<pid>/stack to seqfiles") from the tip-core tree with commit
5d8880e3e0fb96459c652ac5e1504b90c949b3b9 ("proc: remove '##' usage") from
the proc tree.

The former added a new usage of INF (which the second converted to a
ONE).  I added the following patch to the merge of the proc tree and can
carry it.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Mon, 10 Nov 2008 16:25:29 +1100
Subject: [PATCH] proc: merge fix 1

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 fs/proc/base.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index 20e7752..f025953 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -2544,7 +2544,7 @@ static const struct pid_entry tgid_base_stuff[] = {
 	INF("wchan",      S_IRUGO, proc_pid_wchan),
 #endif
 #ifdef CONFIG_STACKTRACE
-	ONE("stack",      S_IRUSR, pid_stack),
+	ONE("stack",      S_IRUSR, proc_pid_stack),
 #endif
 #ifdef CONFIG_SCHEDSTATS
 	INF("schedstat",  S_IRUGO, proc_pid_schedstat),
-- 
1.5.6.5

Re: linux-next: proc tree build failure

[Alexey Dobriyan]

On Mon, Nov 10, 2008 at 04:29:41PM +1100, Stephen Rothwell wrote:
> Today's linux-next build (powerpc ppc64_defconfig) failed like this:
> 
> fs/proc/base.c:2547: error: 'pid_stack' undeclared here (not in a function)
> 
> Caused by an interaction of commits
> 078e9901c02d4899db5b24f708b8dfbca8935891 ("stacktrace:
> add /proc/<pid>/stack to dump task's stack trace") and
> 35f0b5fd7fab907a1119eaa614d9b24e5e225755 ("stacktrace:
> convert /proc/<pid>/stack to seqfiles") from the tip-core tree with commit
> 5d8880e3e0fb96459c652ac5e1504b90c949b3b9 ("proc: remove '##' usage") from
> the proc tree.
> 
> The former added a new usage of INF (which the second converted to a
> ONE).  I added the following patch to the merge of the proc tree and can
> carry it.

Time to steal this /proc/*/stack from Ingo. :-)

> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -2544,7 +2544,7 @@ static const struct pid_entry tgid_base_stuff[] = {
>  	INF("wchan",      S_IRUGO, proc_pid_wchan),
>  #endif
>  #ifdef CONFIG_STACKTRACE
> -	ONE("stack",      S_IRUSR, pid_stack),
> +	ONE("stack",      S_IRUSR, proc_pid_stack),

correct

Re: linux-next: proc tree build failure

[Ingo Molnar]

* Alexey Dobriyan <adobriyan@gmail.com> wrote:

> On Mon, Nov 10, 2008 at 04:29:41PM +1100, Stephen Rothwell wrote:
> > Today's linux-next build (powerpc ppc64_defconfig) failed like this:
> > 
> > fs/proc/base.c:2547: error: 'pid_stack' undeclared here (not in a function)
> > 
> > Caused by an interaction of commits
> > 078e9901c02d4899db5b24f708b8dfbca8935891 ("stacktrace:
> > add /proc/<pid>/stack to dump task's stack trace") and
> > 35f0b5fd7fab907a1119eaa614d9b24e5e225755 ("stacktrace:
> > convert /proc/<pid>/stack to seqfiles") from the tip-core tree with commit
> > 5d8880e3e0fb96459c652ac5e1504b90c949b3b9 ("proc: remove '##' usage") from
> > the proc tree.
> > 
> > The former added a new usage of INF (which the second converted to a
> > ONE).  I added the following patch to the merge of the proc tree and can
> > carry it.
> 
> Time to steal this /proc/*/stack from Ingo. :-)

Alexey, would you like to carry them for v2.6.29? They are in a 
separate tree here, the pull coordinates are below.

Please preserve the sha1's (do not rebase/cherry-pick, etc.) so that i 
can carry it too without causing confusion.

	Ingo

------------------->

The core/stacktrace git tree can be pulled from:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git core/stacktrace

 Thanks,

	Ingo

------------------>
Ken Chen (2):
      stacktrace: add /proc/<pid>/stack to dump task's stack trace
      stacktrace: convert /proc/<pid>/stack to seqfiles


 Documentation/filesystems/proc.txt |    1 +
 fs/proc/base.c                     |   50 ++++++++++++++++++++++++++++++++++-
 2 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/Documentation/filesystems/proc.txt b/Documentation/filesystems/proc.txt
index bcceb99..11f5b75 100644
--- a/Documentation/filesystems/proc.txt
+++ b/Documentation/filesystems/proc.txt
@@ -139,6 +139,7 @@ Table 1-1: Process specific entries in /proc
  statm		Process memory status information
  status		Process status in human readable form
  wchan		If CONFIG_KALLSYMS is set, a pre-decoded wchan
+ stack		Report full stack trace, enable via CONFIG_STACKTRACE
  smaps		Extension based on maps, the rss size for each mapped file
 ..............................................................................
 
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 486cf3f..6d294a4 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -65,6 +65,7 @@
 #include <linux/mm.h>
 #include <linux/rcupdate.h>
 #include <linux/kallsyms.h>
+#include <linux/stacktrace.h>
 #include <linux/resource.h>
 #include <linux/module.h>
 #include <linux/mount.h>
@@ -130,6 +131,12 @@ struct pid_entry {
 		{ .proc_show = &proc_##OTYPE } )
 
 /*
+ * buffer size used for proc read.  See proc_info_read().
+ * 4K page size but our output routines use some slack for overruns
+ */
+#define PROC_BLOCK_SIZE	(3*1024)
+
+/*
  * Count the number of hardlinks for the pid_entry table, excluding the .
  * and .. links.
  */
@@ -340,6 +347,44 @@ static int proc_pid_wchan(struct task_struct *task, char *buffer)
 }
 #endif /* CONFIG_KALLSYMS */
 
+#ifdef CONFIG_STACKTRACE
+
+#define MAX_STACK_TRACE_DEPTH	64
+
+static int proc_pid_stack(struct seq_file *m, struct pid_namespace *ns,
+			  struct pid *pid, struct task_struct *task)
+{
+	struct stack_trace trace;
+	unsigned long *entries;
+	int i;
+
+	entries = kmalloc(sizeof(*entries)*MAX_STACK_TRACE_DEPTH, GFP_KERNEL);
+	if (!entries)
+		return -ENOMEM;
+
+	trace.nr_entries	= 0;
+	trace.max_entries	= MAX_STACK_TRACE_DEPTH;
+	trace.entries		= entries;
+	trace.skip		= 0;
+
+	/*
+	 * Protect against the task exiting (and deallocating its
+	 * stack, etc.) while we save its backtrace:
+	 */
+	read_lock(&tasklist_lock);
+	save_stack_trace_tsk(task, &trace);
+	read_unlock(&tasklist_lock);
+
+	for (i = 0; i < trace.nr_entries; i++) {
+		seq_printf(m, "[<%p>] %pS\n",
+			   (void *)entries[i], (void *)entries[i]);
+	}
+	kfree(entries);
+
+	return 0;
+}
+#endif
+
 #ifdef CONFIG_SCHEDSTATS
 /*
  * Provides /proc/PID/schedstat
@@ -688,8 +733,6 @@ static const struct file_operations proc_mountstats_operations = {
 	.release	= mounts_release,
 };
 
-#define PROC_BLOCK_SIZE	(3*1024)		/* 4K page size but our output routines use some slack for overruns */
-
 static ssize_t proc_info_read(struct file * file, char __user * buf,
 			  size_t count, loff_t *ppos)
 {
@@ -2491,6 +2534,9 @@ static const struct pid_entry tgid_base_stuff[] = {
 #ifdef CONFIG_KALLSYMS
 	INF("wchan",      S_IRUGO, pid_wchan),
 #endif
+#ifdef CONFIG_STACKTRACE
+	ONE("stack",      S_IRUSR, pid_stack),
+#endif
 #ifdef CONFIG_SCHEDSTATS
 	INF("schedstat",  S_IRUGO, pid_schedstat),
 #endif

linux-next: proc tree build failure

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 761 bytes --]

Hi Alexey,

Today's linux-next build (powerpc ppc64_defconfig) failed like this:

fs/proc/meminfo.c: In function 'meminfo_proc_show':
fs/proc/meminfo.c:90: error: 'NR_ACTIVE' undeclared (first use in this function)
fs/proc/meminfo.c:91: error: 'NR_INACTIVE' undeclared (first use in this function)

Caused by commit d8ad172f12c10f974821247773a8b82ba7965ae4 ("proc:
switch /proc/meminfo to seq_file") from the proc tree interacting with
commit 4f98a2fee8acdb4ac84545df98cccecfd130f8db ("vmscan: split LRU lists
into anon & file sets") from  Linus' tree.

It was a bit hard to fix up this evening, so I have dropped the proc tree
for today.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]