Re: [PATCH 20/39] afs: simplify procfs code

[David Howells]

Christoph Hellwig  wrote:

> I don't think you should need any of these.  seq_file_net or
> seq_file_single_net will return you the net_ns based on a struct
> seq_file.  And even from your write routines you can reach the
> seq_file in file->private pretty easily.

You've taken away things like single_open/release_net() which means I can't
supply my own fops and use the proc_net stuff.  I wonder if I should add a
write op to struct proc_dir_entry.

David

Re: [PATCH 20/39] afs: simplify procfs code

[David Howells]

Christoph Hellwig  wrote:

> I don't think you should need any of these.  seq_file_net or
> seq_file_single_net will return you the net_ns based on a struct
> seq_file.  And even from your write routines you can reach the
> seq_file in file->private pretty easily.

You've taken away things like single_open/release_net() which means I can't
supply my own fops and use the proc_net stuff.  I wonder if I should add a
write op to struct proc_dir_entry.

David

Re: [PATCH 20/39] afs: simplify procfs code

[Christoph Hellwig]

On Fri, Apr 20, 2018 at 01:29:34PM +0100, David Howells wrote:
> David Howells  wrote:
> 
> > > Use remove_proc_subtree to remove the whole subtree on cleanup, and
> > > unwind the registration loop into individual calls.  Switch to use
> > > proc_create_seq where applicable.
> > 
> > Note that this is likely going to clash with my patch to net-namespace all 
> > of
> > the afs proc files:
> > 
> > 
> > https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/commit/?h=mount-context=f60c26c827c073583107ebf19d87bc5c0e71c3d2
> > 
> > If it helps, I should be able to disentangle this from the mount-api changes
> > since the subsequent patch connects the dots to propagate the network
> > namespace over automount using the new fs_context to do it.
> 
> Okay, I'll follow up this mail with a pair of patches to just use network
> namespacing in AFS.  The first exports a function from core code only; the
> second is the actual modifications to AFS.

I don't think you should need any of these.  seq_file_net or
seq_file_single_net will return you the net_ns based on a struct
seq_file.  And even from your write routines you can reach the
seq_file in file->private pretty easily.

Re: [PATCH 20/39] afs: simplify procfs code

[Christoph Hellwig]

On Fri, Apr 20, 2018 at 01:29:34PM +0100, David Howells wrote:
> David Howells  wrote:
> 
> > > Use remove_proc_subtree to remove the whole subtree on cleanup, and
> > > unwind the registration loop into individual calls.  Switch to use
> > > proc_create_seq where applicable.
> > 
> > Note that this is likely going to clash with my patch to net-namespace all 
> > of
> > the afs proc files:
> > 
> > 
> > https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/commit/?h=mount-context=f60c26c827c073583107ebf19d87bc5c0e71c3d2
> > 
> > If it helps, I should be able to disentangle this from the mount-api changes
> > since the subsequent patch connects the dots to propagate the network
> > namespace over automount using the new fs_context to do it.
> 
> Okay, I'll follow up this mail with a pair of patches to just use network
> namespacing in AFS.  The first exports a function from core code only; the
> second is the actual modifications to AFS.

I don't think you should need any of these.  seq_file_net or
seq_file_single_net will return you the net_ns based on a struct
seq_file.  And even from your write routines you can reach the
seq_file in file->private pretty easily.

Re: [PATCH 20/39] afs: simplify procfs code

[David Howells]

David Howells  wrote:

> > Use remove_proc_subtree to remove the whole subtree on cleanup, and
> > unwind the registration loop into individual calls.  Switch to use
> > proc_create_seq where applicable.
> 
> Note that this is likely going to clash with my patch to net-namespace all of
> the afs proc files:
> 
>   
> https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/commit/?h=mount-context=f60c26c827c073583107ebf19d87bc5c0e71c3d2
> 
> If it helps, I should be able to disentangle this from the mount-api changes
> since the subsequent patch connects the dots to propagate the network
> namespace over automount using the new fs_context to do it.

Okay, I'll follow up this mail with a pair of patches to just use network
namespacing in AFS.  The first exports a function from core code only; the
second is the actual modifications to AFS.

Note that this doesn't actually yet permit AFS to use any other namespace.
The propagation-over-automount issue is handled by a separate patch.

David

Re: [PATCH 20/39] afs: simplify procfs code

[David Howells]

David Howells  wrote:

> > Use remove_proc_subtree to remove the whole subtree on cleanup, and
> > unwind the registration loop into individual calls.  Switch to use
> > proc_create_seq where applicable.
> 
> Note that this is likely going to clash with my patch to net-namespace all of
> the afs proc files:
> 
>   
> https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/commit/?h=mount-context=f60c26c827c073583107ebf19d87bc5c0e71c3d2
> 
> If it helps, I should be able to disentangle this from the mount-api changes
> since the subsequent patch connects the dots to propagate the network
> namespace over automount using the new fs_context to do it.

Okay, I'll follow up this mail with a pair of patches to just use network
namespacing in AFS.  The first exports a function from core code only; the
second is the actual modifications to AFS.

Note that this doesn't actually yet permit AFS to use any other namespace.
The propagation-over-automount issue is handled by a separate patch.

David

Re: [PATCH 20/39] afs: simplify procfs code

[David Howells]

Christoph Hellwig  wrote:

> Use remove_proc_subtree to remove the whole subtree on cleanup, and
> unwind the registration loop into individual calls.  Switch to use
> proc_create_seq where applicable.

Note that this is likely going to clash with my patch to net-namespace all of
the afs proc files:


https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/commit/?h=mount-context=f60c26c827c073583107ebf19d87bc5c0e71c3d2

If it helps, I should be able to disentangle this from the mount-api changes
since the subsequent patch connects the dots to propagate the network
namespace over automount using the new fs_context to do it.

David

Re: [PATCH 20/39] afs: simplify procfs code

[David Howells]

Christoph Hellwig  wrote:

> Use remove_proc_subtree to remove the whole subtree on cleanup, and
> unwind the registration loop into individual calls.  Switch to use
> proc_create_seq where applicable.

Note that this is likely going to clash with my patch to net-namespace all of
the afs proc files:


https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/commit/?h=mount-context=f60c26c827c073583107ebf19d87bc5c0e71c3d2

If it helps, I should be able to disentangle this from the mount-api changes
since the subsequent patch connects the dots to propagate the network
namespace over automount using the new fs_context to do it.

David

[PATCH 20/39] afs: simplify procfs code

[Christoph Hellwig]

Use remove_proc_subtree to remove the whole subtree on cleanup, and
unwind the registration loop into individual calls.  Switch to use
proc_create_seq where applicable.

Signed-off-by: Christoph Hellwig 
---
 fs/afs/proc.c | 134 ++
 1 file changed, 15 insertions(+), 119 deletions(-)

diff --git a/fs/afs/proc.c b/fs/afs/proc.c
index 839a22280606..3aad32762989 100644
--- a/fs/afs/proc.c
+++ b/fs/afs/proc.c
@@ -62,7 +62,6 @@ static const struct file_operations afs_proc_rootcell_fops = {
.llseek = no_llseek,
 };
 
-static int afs_proc_cell_volumes_open(struct inode *inode, struct file *file);
 static void *afs_proc_cell_volumes_start(struct seq_file *p, loff_t *pos);
 static void *afs_proc_cell_volumes_next(struct seq_file *p, void *v,
loff_t *pos);
@@ -76,15 +75,6 @@ static const struct seq_operations afs_proc_cell_volumes_ops 
= {
.show   = afs_proc_cell_volumes_show,
 };
 
-static const struct file_operations afs_proc_cell_volumes_fops = {
-   .open   = afs_proc_cell_volumes_open,
-   .read   = seq_read,
-   .llseek = seq_lseek,
-   .release= seq_release,
-};
-
-static int afs_proc_cell_vlservers_open(struct inode *inode,
-   struct file *file);
 static void *afs_proc_cell_vlservers_start(struct seq_file *p, loff_t *pos);
 static void *afs_proc_cell_vlservers_next(struct seq_file *p, void *v,
  loff_t *pos);
@@ -98,14 +88,6 @@ static const struct seq_operations 
afs_proc_cell_vlservers_ops = {
.show   = afs_proc_cell_vlservers_show,
 };
 
-static const struct file_operations afs_proc_cell_vlservers_fops = {
-   .open   = afs_proc_cell_vlservers_open,
-   .read   = seq_read,
-   .llseek = seq_lseek,
-   .release= seq_release,
-};
-
-static int afs_proc_servers_open(struct inode *inode, struct file *file);
 static void *afs_proc_servers_start(struct seq_file *p, loff_t *pos);
 static void *afs_proc_servers_next(struct seq_file *p, void *v,
loff_t *pos);
@@ -119,13 +101,6 @@ static const struct seq_operations afs_proc_servers_ops = {
.show   = afs_proc_servers_show,
 };
 
-static const struct file_operations afs_proc_servers_fops = {
-   .open   = afs_proc_servers_open,
-   .read   = seq_read,
-   .llseek = seq_lseek,
-   .release= seq_release,
-};
-
 static int afs_proc_sysname_open(struct inode *inode, struct file *file);
 static int afs_proc_sysname_release(struct inode *inode, struct file *file);
 static void *afs_proc_sysname_start(struct seq_file *p, loff_t *pos);
@@ -152,7 +127,7 @@ static const struct file_operations afs_proc_sysname_fops = 
{
.write  = afs_proc_sysname_write,
 };
 
-static const struct file_operations afs_proc_stats_fops;
+static int afs_proc_stats_show(struct seq_file *m, void *v);
 
 /*
  * initialise the /proc/fs/afs/ directory
@@ -167,8 +142,8 @@ int afs_proc_init(struct afs_net *net)
 
if (!proc_create("cells", 0644, net->proc_afs, _proc_cells_fops) ||
!proc_create("rootcell", 0644, net->proc_afs, 
_proc_rootcell_fops) ||
-   !proc_create("servers", 0644, net->proc_afs, 
_proc_servers_fops) ||
-   !proc_create("stats", 0644, net->proc_afs, _proc_stats_fops) ||
+   !proc_create_seq("servers", 0644, net->proc_afs, 
_proc_servers_ops) ||
+   !proc_create_single("stats", 0644, net->proc_afs, 
afs_proc_stats_show) ||
!proc_create("sysname", 0644, net->proc_afs, 
_proc_sysname_fops))
goto error_tree;
 
@@ -196,16 +171,7 @@ void afs_proc_cleanup(struct afs_net *net)
  */
 static int afs_proc_cells_open(struct inode *inode, struct file *file)
 {
-   struct seq_file *m;
-   int ret;
-
-   ret = seq_open(file, _proc_cells_ops);
-   if (ret < 0)
-   return ret;
-
-   m = file->private_data;
-   m->private = PDE_DATA(inode);
-   return 0;
+   return seq_open(file, _proc_cells_ops);
 }
 
 /*
@@ -430,10 +396,11 @@ int afs_proc_cell_setup(struct afs_net *net, struct 
afs_cell *cell)
if (!dir)
goto error_dir;
 
-   if (!proc_create_data("vlservers", 0, dir,
- _proc_cell_vlservers_fops, cell) ||
-   !proc_create_data("volumes", 0, dir,
- _proc_cell_volumes_fops, cell))
+   if (!proc_create_seq_data("vlservers", 0, dir,
+   _proc_cell_vlservers_ops, cell))
+   goto error_tree;
+   if (!proc_create_seq_data("volumes", 0, dir, _proc_cell_volumes_ops,
+   cell))
goto error_tree;
 
_leave(" = 0");
@@ -458,29 +425,6 @@ void afs_proc_cell_remove(struct afs_net *net, struct 
afs_cell *cell)

[PATCH 20/39] afs: simplify procfs code

[Christoph Hellwig]

Use remove_proc_subtree to remove the whole subtree on cleanup, and
unwind the registration loop into individual calls.  Switch to use
proc_create_seq where applicable.

Signed-off-by: Christoph Hellwig 
---
 fs/afs/proc.c | 134 ++
 1 file changed, 15 insertions(+), 119 deletions(-)

diff --git a/fs/afs/proc.c b/fs/afs/proc.c
index 839a22280606..3aad32762989 100644
--- a/fs/afs/proc.c
+++ b/fs/afs/proc.c
@@ -62,7 +62,6 @@ static const struct file_operations afs_proc_rootcell_fops = {
.llseek = no_llseek,
 };
 
-static int afs_proc_cell_volumes_open(struct inode *inode, struct file *file);
 static void *afs_proc_cell_volumes_start(struct seq_file *p, loff_t *pos);
 static void *afs_proc_cell_volumes_next(struct seq_file *p, void *v,
loff_t *pos);
@@ -76,15 +75,6 @@ static const struct seq_operations afs_proc_cell_volumes_ops 
= {
.show   = afs_proc_cell_volumes_show,
 };
 
-static const struct file_operations afs_proc_cell_volumes_fops = {
-   .open   = afs_proc_cell_volumes_open,
-   .read   = seq_read,
-   .llseek = seq_lseek,
-   .release= seq_release,
-};
-
-static int afs_proc_cell_vlservers_open(struct inode *inode,
-   struct file *file);
 static void *afs_proc_cell_vlservers_start(struct seq_file *p, loff_t *pos);
 static void *afs_proc_cell_vlservers_next(struct seq_file *p, void *v,
  loff_t *pos);
@@ -98,14 +88,6 @@ static const struct seq_operations 
afs_proc_cell_vlservers_ops = {
.show   = afs_proc_cell_vlservers_show,
 };
 
-static const struct file_operations afs_proc_cell_vlservers_fops = {
-   .open   = afs_proc_cell_vlservers_open,
-   .read   = seq_read,
-   .llseek = seq_lseek,
-   .release= seq_release,
-};
-
-static int afs_proc_servers_open(struct inode *inode, struct file *file);
 static void *afs_proc_servers_start(struct seq_file *p, loff_t *pos);
 static void *afs_proc_servers_next(struct seq_file *p, void *v,
loff_t *pos);
@@ -119,13 +101,6 @@ static const struct seq_operations afs_proc_servers_ops = {
.show   = afs_proc_servers_show,
 };
 
-static const struct file_operations afs_proc_servers_fops = {
-   .open   = afs_proc_servers_open,
-   .read   = seq_read,
-   .llseek = seq_lseek,
-   .release= seq_release,
-};
-
 static int afs_proc_sysname_open(struct inode *inode, struct file *file);
 static int afs_proc_sysname_release(struct inode *inode, struct file *file);
 static void *afs_proc_sysname_start(struct seq_file *p, loff_t *pos);
@@ -152,7 +127,7 @@ static const struct file_operations afs_proc_sysname_fops = 
{
.write  = afs_proc_sysname_write,
 };
 
-static const struct file_operations afs_proc_stats_fops;
+static int afs_proc_stats_show(struct seq_file *m, void *v);
 
 /*
  * initialise the /proc/fs/afs/ directory
@@ -167,8 +142,8 @@ int afs_proc_init(struct afs_net *net)
 
if (!proc_create("cells", 0644, net->proc_afs, _proc_cells_fops) ||
!proc_create("rootcell", 0644, net->proc_afs, 
_proc_rootcell_fops) ||
-   !proc_create("servers", 0644, net->proc_afs, 
_proc_servers_fops) ||
-   !proc_create("stats", 0644, net->proc_afs, _proc_stats_fops) ||
+   !proc_create_seq("servers", 0644, net->proc_afs, 
_proc_servers_ops) ||
+   !proc_create_single("stats", 0644, net->proc_afs, 
afs_proc_stats_show) ||
!proc_create("sysname", 0644, net->proc_afs, 
_proc_sysname_fops))
goto error_tree;
 
@@ -196,16 +171,7 @@ void afs_proc_cleanup(struct afs_net *net)
  */
 static int afs_proc_cells_open(struct inode *inode, struct file *file)
 {
-   struct seq_file *m;
-   int ret;
-
-   ret = seq_open(file, _proc_cells_ops);
-   if (ret < 0)
-   return ret;
-
-   m = file->private_data;
-   m->private = PDE_DATA(inode);
-   return 0;
+   return seq_open(file, _proc_cells_ops);
 }
 
 /*
@@ -430,10 +396,11 @@ int afs_proc_cell_setup(struct afs_net *net, struct 
afs_cell *cell)
if (!dir)
goto error_dir;
 
-   if (!proc_create_data("vlservers", 0, dir,
- _proc_cell_vlservers_fops, cell) ||
-   !proc_create_data("volumes", 0, dir,
- _proc_cell_volumes_fops, cell))
+   if (!proc_create_seq_data("vlservers", 0, dir,
+   _proc_cell_vlservers_ops, cell))
+   goto error_tree;
+   if (!proc_create_seq_data("volumes", 0, dir, _proc_cell_volumes_ops,
+   cell))
goto error_tree;
 
_leave(" = 0");
@@ -458,29 +425,6 @@ void afs_proc_cell_remove(struct afs_net *net, struct 
afs_cell *cell)
_leave("");
 }