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.