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@xxxxxxxxxxxxxx>
> ---
> 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
>
--
Joel Granados
Attachment:
signature.asc
Description: PGP signature
