Re: [PATCH 03/14] sysctl: Add ctl_table_size to ctl_table_header

From: Joel Granados <j.granados@samsung.com>
To: Simon Horman <horms@kernel.org>
Cc: <mcgrof@kernel.org>, Kees Cook <keescook@chromium.org>,
	Iurii Zaikin <yzaikin@google.com>, <willy@infradead.org>,
	<josh@joshtriplett.org>, <linux-kernel@vger.kernel.org>,
	<linux-fsdevel@vger.kernel.org>
Subject: Re: [PATCH 03/14] sysctl: Add ctl_table_size to ctl_table_header
Date: Mon, 31 Jul 2023 14:10:15 +0200	[thread overview]
Message-ID: <20230731121015.i7vhfsyx7nzw7kpc@localhost> (raw)
In-Reply-To: <ZMOc/+Q0PT48ed0G@kernel.org>

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

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 {
~
-- 

Joel Granados

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
