Re: [PATCH v2 0/6] sysctl: add typed field descriptors

[Alexey Gladkov <legion@kernel.org>]

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

On Thu, Sep 24, 2026 at 03:29:28PM +0200, Joel Granados wrote:
> On Mon, Sep 21, 2026 at 12:54:47PM +0200, Alexey Gladkov wrote:
> > Hi,
> > 
> > Some sysctl users allocate a private copy of an otherwise static ctl_table
> > for every namespace or device, then rewrite data and limit pointers before
> > registration.  Besides the per-instance allocation, these fixups are often
> > addressed by table index, so changing the table can silently associate an
> > entry with the wrong data or limits.
> 
> In other words the motivation is to:
> 
> 1. Remove the per-instance allocation thereby reducing the memory
>    footprint
> 2. Remove the possibility of mistakenly associating one variable with a
>    different sysctl name (because of un-synced changes)
> 
> Only those two. Right?

Another reason is type checking during compile time. Currently, .data,
.extra1, and .extra2 are pointers to void. The sysctl_check_table()
attempts to validate extra fields, but this is done at runtime and
essentially involves checking .maxlen.

I thought that we could not only eliminate the need for memory allocation
but also make the parameter checks more strict.

> > 
> > This series adds struct sysctl_field as an alternative descriptor for such
> > tables.  A field records the value type and an offset into an object
> > selected at registration time.  The type-specific offset helpers are small
> > wrappers around offsetof() and only add a compile-time check of the
> > referenced member type.  The sysctl core derives the handler, size, data
> > and limits, and builds a temporary ctl_table when invoking existing
> > handler, permission and BPF interfaces.
> 
> Building a temporary ctl_table entry just before calling the
> proc_handler is the way to go, IMO. But do you need to adjust all those
> members (handler, size, data and limits) always? Sometimes everything
> stays the same except for the one ".data" member.

The `struct ctl_table/sysctl_field` array may contain entries with
different types and other fields. I figured it would be easier to reassign
the values to keep things simple.

> PS: Any meat in sashiko's comments?
>     https://sashiko.dev/#/patchset/cover.1789987960.git.legion%40kernel.org

Hm. For some reason, sashiko has stopped sending review emails. I used to
receive them, but now I haven't received anything. I'll check his review
and address his comments.

By the way, I heard from Oleg Nesterov that he isn't getting any messages
from sashiko either.

> > 
> > A struct sysctl_context identifies the object shared by the whole
> > registration and is copied into the table header.  Subsystems which need
> > more than a namespace can embed it as the first member of a larger context
> > and select the object to which field offsets apply.
> > 
> > Existing ctl_table users are unchanged, and subsystems can migrate one
> > table at a time.  Converted tables remain static and read-only instead of
> > being copied and patched for every instance.
> > 
> > Compared with the previous RFC [1], this drops the per-field accessor
> > functions and the macros which generated them.  The first two patches
> > prepare the sysctl core without changing its external behaviour.  The third
> > patch adds the new descriptor, followed by conversions of the IPC, mqueue
> > and ucount tables as small users of the interface.
> > 
> > This is only the first part of the conversion.  Network sysctls and the
> > other subsystem-specific users will be submitted as separate follow-up
> > series so they can be reviewed by their respective maintainers without
> > making this initial series excessively large.
> > 
> > [1] https://lore.kernel.org/all/cover.1787770053.git.legion@kernel.org/
> > 
> > Alexey Gladkov (6):
> >   proc: sysctl: address table entries by index
> >   sysctl: add unsigned int limit constants
> >   sysctl: add typed field descriptors
> >   sysctl: ipc: use typed fields for IPC namespace sysctls
> >   sysctl: mq: use typed fields for IPC namespace sysctls
> >   sysctl: use typed fields for ucount limits
> > 
> >  fs/proc/inode.c        |   2 +-
> >  fs/proc/internal.h     |   2 +-
> >  fs/proc/proc_sysctl.c  | 601 ++++++++++++++++++++++++++++++-----------
> >  include/linux/sysctl.h | 150 +++++++++-
> >  ipc/ipc_sysctl.c       | 188 ++++++-------
> >  ipc/mq_sysctl.c        | 104 +++----
> >  kernel/sysctl.c        |   3 +
> >  kernel/ucount.c        |  64 ++---
> >  8 files changed, 750 insertions(+), 364 deletions(-)
> > 
> > 
> > base-commit: 587858367581b9c55c3690f4e63382ad622719d4
> > -- 
> > 2.55.0
> > 
> 
> 



-- 
Rgrds, legion


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