| Message ID | 20230731071728.3493794-4-j.granados@samsung.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,01/14] sysctl: Prefer ctl_table_header in proc_sysctl | expand |
On Mon, Jul 31, 2023 at 09:17:17AM +0200, Joel Granados wrote: > The new ctl_table_size element will hold the size of the ctl_table > arrays contained in the ctl_table_header. This value should eventually > be passed by the callers to the sysctl register infrastructure. And > while this commit introduces the variable, it does not set nor use it > because that requires case by case considerations for each caller. > > It provides two important things: (1) A place to put the > result of the ctl_table array calculation when it gets introduced for > each caller. And (2) the size that will be used as the additional > stopping criteria in the list_for_each_table_entry macro (to be added > when all the callers are migrated) > > 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()" > + * > + */ Hi Joel, Please consider also adding kernel doc entries for the other fields of struct ctl_table_header. According to ./scripts/kernel-doc -none they are: unregistering ctl_table_arg root set parent node inodes > struct ctl_table_header { > union { > struct { > struct ctl_table *ctl_table; > + int ctl_table_size; > int used; > int count; > int nreg; > -- > 2.30.2 >
On Mon, Jul 31, 2023 at 08:30:34PM +0200, Simon Horman wrote: > On Mon, Jul 31, 2023 at 09:17:17AM +0200, Joel Granados wrote: > > The new ctl_table_size element will hold the size of the ctl_table > > arrays contained in the ctl_table_header. This value should eventually > > be passed by the callers to the sysctl register infrastructure. And > > while this commit introduces the variable, it does not set nor use it > > because that requires case by case considerations for each caller. > > > > It provides two important things: (1) A place to put the > > result of the ctl_table array calculation when it gets introduced for > > each caller. And (2) the size that will be used as the additional > > stopping criteria in the list_for_each_table_entry macro (to be added > > when all the callers are migrated) > > > > 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()" > > + * > > + */ > > Hi Joel, > > Please consider also adding kernel doc entries for the other fields of > struct ctl_table_header. According to ./scripts/kernel-doc -none > they are: > > unregistering > ctl_table_arg > root > set > parent > node > inodes Sorry, I now realise that I made the same comment on v1. And I didn't see your response to that until after I wrote the above. > > > > struct ctl_table_header { > > union { > > struct { > > struct ctl_table *ctl_table; > > + int ctl_table_size; > > int used; > > int count; > > int nreg; > > -- > > 2.30.2 > >
On Mon, Jul 31, 2023 at 09:07:06PM +0200, Simon Horman wrote: > On Mon, Jul 31, 2023 at 08:30:34PM +0200, Simon Horman wrote: > > On Mon, Jul 31, 2023 at 09:17:17AM +0200, Joel Granados wrote: > > > The new ctl_table_size element will hold the size of the ctl_table > > > arrays contained in the ctl_table_header. This value should eventually > > > be passed by the callers to the sysctl register infrastructure. And > > > while this commit introduces the variable, it does not set nor use it > > > because that requires case by case considerations for each caller. > > > > > > It provides two important things: (1) A place to put the > > > result of the ctl_table array calculation when it gets introduced for > > > each caller. And (2) the size that will be used as the additional > > > stopping criteria in the list_for_each_table_entry macro (to be added > > > when all the callers are migrated) > > > > > > 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()" > > > + * > > > + */ > > > > Hi Joel, > > > > Please consider also adding kernel doc entries for the other fields of > > struct ctl_table_header. According to ./scripts/kernel-doc -none > > they are: > > > > unregistering > > ctl_table_arg > > root > > set > > parent > > node > > inodes > > Sorry, I now realise that I made the same comment on v1. > And I didn't see your response to that until after I wrote the above. np. I'll hold off adding this to the set until I get feedback from my proposal. I do this as I'm unsure about the contents of the docs. > > > > > > > > struct ctl_table_header { > > > union { > > > struct { > > > struct ctl_table *ctl_table; > > > + int ctl_table_size; > > > int used; > > > int count; > > > int nreg; > > > -- > > > 2.30.2 > > >
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;
The new ctl_table_size element will hold the size of the ctl_table arrays contained in the ctl_table_header. This value should eventually be passed by the callers to the sysctl register infrastructure. And while this commit introduces the variable, it does not set nor use it because that requires case by case considerations for each caller. It provides two important things: (1) A place to put the result of the ctl_table array calculation when it gets introduced for each caller. And (2) the size that will be used as the additional stopping criteria in the list_for_each_table_entry macro (to be added when all the callers are migrated) Signed-off-by: Joel Granados <j.granados@samsung.com> --- include/linux/sysctl.h | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)