| Message ID | 20240222-sysctl-empty-dir-v1-2-45ba9a6352e8@weissschuh.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | sysctl: move sysctl type to ctl_table_header | expand |
On Thu, Feb 22, 2024 at 08:07:37AM +0100, Thomas Weißschuh wrote: > As static initialization of the is not possible anymore move it into > init_header() where all the other header fields are also initialized. Please say what was done. Something like: "Moved the SYSCTL_TABLE_TYPE_{DEFAULT,PERMANENTLY_EMPTY} enumerations from ctl_table to ctl_table_header." And the then you can mention the why: "Removing the mutable memeber from ctl_table opens the ...." > > Reduce memory consumption as there are less instances of > ctl_table_header than ctl_table. This is indeed true, but the main reasoning behind this is constification. Right? If you want to leave this comment, I would suggest you add something that talks about the amount of bytes saved. Something like : "Reduce memory consumption by sizeof(int) for every ctl_table entry in ctl_table_header". Or something similar. > > Removing this mutable member also opens the way to constify static > instances of ctl_table. > > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> > --- > fs/proc/proc_sysctl.c | 10 ++++++---- > include/linux/sysctl.h | 22 +++++++++++----------- > 2 files changed, 17 insertions(+), 15 deletions(-) > > diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c > index 2f4d4329d83d..fde7a2f773f0 100644 > --- a/fs/proc/proc_sysctl.c > +++ b/fs/proc/proc_sysctl.c > @@ -31,7 +31,7 @@ static const struct inode_operations proc_sys_dir_operations; > > /* Support for permanently empty directories */ > static struct ctl_table sysctl_mount_point[] = { > - {.type = SYSCTL_TABLE_TYPE_PERMANENTLY_EMPTY } > + { } > }; > > /** > @@ -49,11 +49,11 @@ struct ctl_table_header *register_sysctl_mount_point(const char *path) > EXPORT_SYMBOL(register_sysctl_mount_point); > > #define sysctl_is_perm_empty_ctl_header(hptr) \ > - (hptr->ctl_table[0].type == SYSCTL_TABLE_TYPE_PERMANENTLY_EMPTY) > + (hptr->type == SYSCTL_TABLE_TYPE_PERMANENTLY_EMPTY) > #define sysctl_set_perm_empty_ctl_header(hptr) \ > - (hptr->ctl_table[0].type = SYSCTL_TABLE_TYPE_PERMANENTLY_EMPTY) > + (hptr->type = SYSCTL_TABLE_TYPE_PERMANENTLY_EMPTY) > #define sysctl_clear_perm_empty_ctl_header(hptr) \ > - (hptr->ctl_table[0].type = SYSCTL_TABLE_TYPE_DEFAULT) > + (hptr->type = SYSCTL_TABLE_TYPE_DEFAULT) > > void proc_sys_poll_notify(struct ctl_table_poll *poll) > { > @@ -208,6 +208,8 @@ static void init_header(struct ctl_table_header *head, > node++; > } > } > + if (table == sysctl_mount_point) > + sysctl_set_perm_empty_ctl_header(head); > } > > static void erase_header(struct ctl_table_header *head) > diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h > index ee7d33b89e9e..c87f73c06cb9 100644 > --- a/include/linux/sysctl.h > +++ b/include/linux/sysctl.h > @@ -137,17 +137,6 @@ struct ctl_table { > void *data; > int maxlen; > umode_t mode; > - /** > - * enum type - Enumeration to differentiate between ctl target types > - * @SYSCTL_TABLE_TYPE_DEFAULT: ctl target with no special considerations > - * @SYSCTL_TABLE_TYPE_PERMANENTLY_EMPTY: Used to identify a permanently > - * empty directory target to serve > - * as mount point. > - */ > - enum { > - SYSCTL_TABLE_TYPE_DEFAULT, > - SYSCTL_TABLE_TYPE_PERMANENTLY_EMPTY > - } type; > proc_handler *proc_handler; /* Callback for text formatting */ > struct ctl_table_poll *poll; > void *extra1; > @@ -188,6 +177,17 @@ struct ctl_table_header { > struct ctl_dir *parent; > struct ctl_node *node; > struct hlist_head inodes; /* head for proc_inode->sysctl_inodes */ > + /** > + * enum type - Enumeration to differentiate between ctl target types > + * @SYSCTL_TABLE_TYPE_DEFAULT: ctl target with no special considerations > + * @SYSCTL_TABLE_TYPE_PERMANENTLY_EMPTY: Used to identify a permanently > + * empty directory target to serve > + * as mount point. > + */ > + enum { > + SYSCTL_TABLE_TYPE_DEFAULT, > + SYSCTL_TABLE_TYPE_PERMANENTLY_EMPTY, > + } type; > }; > > struct ctl_dir { > > -- > 2.43.2 >
diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c index 2f4d4329d83d..fde7a2f773f0 100644 --- a/fs/proc/proc_sysctl.c +++ b/fs/proc/proc_sysctl.c @@ -31,7 +31,7 @@ static const struct inode_operations proc_sys_dir_operations; /* Support for permanently empty directories */ static struct ctl_table sysctl_mount_point[] = { - {.type = SYSCTL_TABLE_TYPE_PERMANENTLY_EMPTY } + { } }; /** @@ -49,11 +49,11 @@ struct ctl_table_header *register_sysctl_mount_point(const char *path) EXPORT_SYMBOL(register_sysctl_mount_point); #define sysctl_is_perm_empty_ctl_header(hptr) \ - (hptr->ctl_table[0].type == SYSCTL_TABLE_TYPE_PERMANENTLY_EMPTY) + (hptr->type == SYSCTL_TABLE_TYPE_PERMANENTLY_EMPTY) #define sysctl_set_perm_empty_ctl_header(hptr) \ - (hptr->ctl_table[0].type = SYSCTL_TABLE_TYPE_PERMANENTLY_EMPTY) + (hptr->type = SYSCTL_TABLE_TYPE_PERMANENTLY_EMPTY) #define sysctl_clear_perm_empty_ctl_header(hptr) \ - (hptr->ctl_table[0].type = SYSCTL_TABLE_TYPE_DEFAULT) + (hptr->type = SYSCTL_TABLE_TYPE_DEFAULT) void proc_sys_poll_notify(struct ctl_table_poll *poll) { @@ -208,6 +208,8 @@ static void init_header(struct ctl_table_header *head, node++; } } + if (table == sysctl_mount_point) + sysctl_set_perm_empty_ctl_header(head); } static void erase_header(struct ctl_table_header *head) diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h index ee7d33b89e9e..c87f73c06cb9 100644 --- a/include/linux/sysctl.h +++ b/include/linux/sysctl.h @@ -137,17 +137,6 @@ struct ctl_table { void *data; int maxlen; umode_t mode; - /** - * enum type - Enumeration to differentiate between ctl target types - * @SYSCTL_TABLE_TYPE_DEFAULT: ctl target with no special considerations - * @SYSCTL_TABLE_TYPE_PERMANENTLY_EMPTY: Used to identify a permanently - * empty directory target to serve - * as mount point. - */ - enum { - SYSCTL_TABLE_TYPE_DEFAULT, - SYSCTL_TABLE_TYPE_PERMANENTLY_EMPTY - } type; proc_handler *proc_handler; /* Callback for text formatting */ struct ctl_table_poll *poll; void *extra1; @@ -188,6 +177,17 @@ struct ctl_table_header { struct ctl_dir *parent; struct ctl_node *node; struct hlist_head inodes; /* head for proc_inode->sysctl_inodes */ + /** + * enum type - Enumeration to differentiate between ctl target types + * @SYSCTL_TABLE_TYPE_DEFAULT: ctl target with no special considerations + * @SYSCTL_TABLE_TYPE_PERMANENTLY_EMPTY: Used to identify a permanently + * empty directory target to serve + * as mount point. + */ + enum { + SYSCTL_TABLE_TYPE_DEFAULT, + SYSCTL_TABLE_TYPE_PERMANENTLY_EMPTY, + } type; }; struct ctl_dir {
As static initialization of the is not possible anymore move it into init_header() where all the other header fields are also initialized. Reduce memory consumption as there are less instances of ctl_table_header than ctl_table. Removing this mutable member also opens the way to constify static instances of ctl_table. Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> --- fs/proc/proc_sysctl.c | 10 ++++++---- include/linux/sysctl.h | 22 +++++++++++----------- 2 files changed, 17 insertions(+), 15 deletions(-)