diff mbox series

[03/14] sysctl: Add ctl_table_size to ctl_table_header

Message ID 20230726140635.2059334-4-j.granados@samsung.com
State New
Headers
Series [01/14] sysctl: Prefer ctl_table_header in proc_sysctl |

Commit Message

Joel Granados July 26, 2023, 2:06 p.m. UTC 
  The new ctl_table_size element will hold the size of the ctl_table
contained in the header. This value is passed by the callers to the
sysctl register infrastructure.

This is a preparation commit that allows us to systematically add
ctl_table_size and start using it only when it is in all the places
where there is a sysctl registration.

Signed-off-by: Joel Granados <j.granados@samsung.com>
---
 include/linux/sysctl.h | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

Comments

Simon Horman July 28, 2023, 10:48 a.m. UTC | #1 
On Wed, Jul 26, 2023 at 04:06:23PM +0200, Joel Granados wrote:
> The new ctl_table_size element will hold the size of the ctl_table
> contained in the header. This value is passed by the callers to the
> sysctl register infrastructure.
> 
> This is a preparation commit that allows us to systematically add
> ctl_table_size and start using it only when it is in all the places
> where there is a sysctl registration.
> 
> Signed-off-by: Joel Granados <j.granados@samsung.com>
> ---
>  include/linux/sysctl.h | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
> index 59d451f455bf..33252ad58ebe 100644
> --- a/include/linux/sysctl.h
> +++ b/include/linux/sysctl.h
> @@ -159,12 +159,22 @@ struct ctl_node {
>  	struct ctl_table_header *header;
>  };
>  
> -/* struct ctl_table_header is used to maintain dynamic lists of
> -   struct ctl_table trees. */
> +/**
> + * struct ctl_table_header - maintains dynamic lists of struct ctl_table trees
> + * @ctl_table: pointer to the first element in ctl_table array
> + * @ctl_table_size: number of elements pointed by @ctl_table
> + * @used: The entry will never be touched when equal to 0.
> + * @count: Upped every time something is added to @inodes and downed every time
> + *         something is removed from inodes
> + * @nreg: When nreg drops to 0 the ctl_table_header will be unregistered.
> + * @rcu: Delays the freeing of the inode. Introduced with "unfuck proc_sysctl ->d_compare()"
> + *
> + */

Please consider documenting all fields of struct ctl_table_header.
./scripts/kernel-doc complains that the following are missing:

  unregistering
  ctl_table_arg
  root
  set
  parent
  node
  inodes
Joel Granados July 31, 2023, 12:10 p.m. UTC | #2 
On Fri, Jul 28, 2023 at 12:48:31PM +0200, Simon Horman wrote:
> On Wed, Jul 26, 2023 at 04:06:23PM +0200, Joel Granados wrote:
> > The new ctl_table_size element will hold the size of the ctl_table
> > contained in the header. This value is passed by the callers to the
> > sysctl register infrastructure.
> > 
> > This is a preparation commit that allows us to systematically add
> > ctl_table_size and start using it only when it is in all the places
> > where there is a sysctl registration.
> > 
> > Signed-off-by: Joel Granados <j.granados@samsung.com>
> > ---
> >  include/linux/sysctl.h | 14 ++++++++++++--
> >  1 file changed, 12 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
> > index 59d451f455bf..33252ad58ebe 100644
> > --- a/include/linux/sysctl.h
> > +++ b/include/linux/sysctl.h
> > @@ -159,12 +159,22 @@ struct ctl_node {
> >  	struct ctl_table_header *header;
> >  };
> >  
> > -/* struct ctl_table_header is used to maintain dynamic lists of
> > -   struct ctl_table trees. */
> > +/**
> > + * struct ctl_table_header - maintains dynamic lists of struct ctl_table trees
> > + * @ctl_table: pointer to the first element in ctl_table array
> > + * @ctl_table_size: number of elements pointed by @ctl_table
> > + * @used: The entry will never be touched when equal to 0.
> > + * @count: Upped every time something is added to @inodes and downed every time
> > + *         something is removed from inodes
> > + * @nreg: When nreg drops to 0 the ctl_table_header will be unregistered.
> > + * @rcu: Delays the freeing of the inode. Introduced with "unfuck proc_sysctl ->d_compare()"
> > + *
> > + */
> 
> Please consider documenting all fields of struct ctl_table_header.
> ./scripts/kernel-doc complains that the following are missing:
> 
>   unregistering
>   ctl_table_arg
>   root
>   set
>   parent
>   node
>   inodes

This one I'm unsure about as things go in and then get changed without updating the docs
I have tried to follow the changes from the point of introduction, but as I said, I'm
unsure if I missed something.

diff --git i/include/linux/sysctl.h w/include/linux/sysctl.h
index 09d7429d67c0..fc0461f2a0c8 100644
--- i/include/linux/sysctl.h
+++ w/include/linux/sysctl.h
@@ -168,7 +168,13 @@ struct ctl_node {
  *         something is removed from inodes
  * @nreg: When nreg drops to 0 the ctl_table_header will be unregistered.
  * @rcu: Delays the freeing of the inode. Introduced with "unfuck proc_sysctl ->d_compare()"
- *
+ * @unregistering: Holds the completion when dropping (un-registering) a ctl_table
+ * @ctl_table_arg: The ctl_table array that was passed to register_sysctl_paths
+ * @root: The root of a sysctl namespace
+ * @set: Set of sysctls
+ * @parent: Pointer to the ctl_dir of the parent directory
+ * @node: Pointer to the rbtree node for this header
+ * @inodes: head for proc_inode->sysctl_inodes
  */
 struct ctl_table_header {
        union {
@@ -187,7 +193,7 @@ struct ctl_table_header {
        struct ctl_table_set *set;
        struct ctl_dir *parent;
        struct ctl_node *node;
-       struct hlist_head inodes; /* head for proc_inode->sysctl_inodes */
+       struct hlist_head inodes;
 };

 struct ctl_dir {
~
diff mbox series

Patch

diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index 59d451f455bf..33252ad58ebe 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -159,12 +159,22 @@  struct ctl_node {
 	struct ctl_table_header *header;
 };
 
-/* struct ctl_table_header is used to maintain dynamic lists of
-   struct ctl_table trees. */
+/**
+ * struct ctl_table_header - maintains dynamic lists of struct ctl_table trees
+ * @ctl_table: pointer to the first element in ctl_table array
+ * @ctl_table_size: number of elements pointed by @ctl_table
+ * @used: The entry will never be touched when equal to 0.
+ * @count: Upped every time something is added to @inodes and downed every time
+ *         something is removed from inodes
+ * @nreg: When nreg drops to 0 the ctl_table_header will be unregistered.
+ * @rcu: Delays the freeing of the inode. Introduced with "unfuck proc_sysctl ->d_compare()"
+ *
+ */
 struct ctl_table_header {
 	union {
 		struct {
 			struct ctl_table *ctl_table;
+			int ctl_table_size;
 			int used;
 			int count;
 			int nreg;