* [RFC PATCH v2 0/2] sysctl: refactor ctl_table creation and change extra{1,2} type
@ 2026-02-07 20:35 wen.yang
2026-02-07 20:35 ` [RFC PATCH v2 1/2] sysctl: introduce SYSCTL_ENTRY() helper macro wen.yang
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: wen.yang @ 2026-02-07 20:35 UTC (permalink / raw)
To: Joel Granados; +Cc: linux-kernel, Wen Yang
From: Wen Yang <wen.yang@linux.dev>
This series introduces macros to centralize ctl_table initialization,
preparing for future type changes to extra1/extra2 fields.
Following Joel's suggestion [1], we start with helpers that work for all
existing patterns before making invasive changes.
[1] https://lore.kernel.org/all/sytyqb3ajm6ysoifwp57ga7gzlnzodhdsbgizbn3hqnlwytn5a@pbscftrq2qwl/
[2] https://sysctl-dev-rtd.readthedocs.io/en/latest/notes/ctltable_entry_macro.html#table-entry-macro
Wen Yang (2):
sysctl: introduce SYSCTL_ENTRY() helper macro
sysctl: convert kernel/sysctl-test.c to use SYSCTL_TBL_ENTRY()
include/linux/sysctl.h | 131 ++++++++++++++++++++++++++++++++++++++
kernel/sysctl-test.c | 139 ++++++++++++-----------------------------
2 files changed, 171 insertions(+), 99 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 5+ messages in thread* [RFC PATCH v2 1/2] sysctl: introduce SYSCTL_ENTRY() helper macro 2026-02-07 20:35 [RFC PATCH v2 0/2] sysctl: refactor ctl_table creation and change extra{1,2} type wen.yang @ 2026-02-07 20:35 ` wen.yang 2026-02-19 10:33 ` Joel Granados 2026-02-07 20:35 ` [RFC PATCH v2 2/2] sysctl: convert kernel/sysctl-test.c to use SYSCTL_TBL_ENTRY() wen.yang 2026-02-19 8:59 ` [RFC PATCH v2 0/2] sysctl: refactor ctl_table creation and change extra{1,2} type Joel Granados 2 siblings, 1 reply; 5+ messages in thread From: wen.yang @ 2026-02-07 20:35 UTC (permalink / raw) To: Joel Granados; +Cc: linux-kernel, Wen Yang From: Wen Yang <wen.yang@linux.dev> Add SYSCTL_ENTRY() macro to simplify struct ctl_table initialization. This macro provides automatic type detection, handler selection, and sensible defaults, reducing boilerplate and potential errors. Based on discussion and suggestions from: https://sysctl-dev-rtd.readthedocs.io/en/latest/notes/ctltable_entry_macro.html#table-entry-macro https://lore.kernel.org/all/psot4oeauxi3yyj2w4ajm3tfgtcsvao4rhv5sgd5s6ymmjgojk@p3vrj3qluban/ Features: - Automatic type detection and handler selection via _Generic() - Supports int, unsigned int, long, unsigned long - Auto-selects range-checking handlers when min/max provided - Flexible calling conventions (1-7 arguments): - SYSCTL_TBL_ENTRY(var) -> readonly, auto-handler - SYSCTL_TBL_ENTRY(name, var) -> custom name - SYSCTL_TBL_ENTRY(name, var, mode) -> custom mode - SYSCTL_TBL_ENTRY(name, var, mode, handler) -> custom handler - SYSCTL_TBL_ENTRY(name, var, mode, min, max) -> with range - SYSCTL_TBL_ENTRY(name, var, mode, handler, min, max) -> full control - SYSCTL_TBL_ENTRY(name, var, mode, handler, min, max, maxlen) -> explicit maxlen - Smart defaults: - Auto address-of: SYSCTL_ENTRY(my_var) -> .data = &my_var - Auto maxlen: uses sizeof(var) when not specified - SYSCTL_NULL marker for entries without data - Compile-time validation: - NAME must be string literal - MODE, HANDLER validated via type compatibility checks - VAR must be lvalue Example usage: static int my_int = 256; /* Before */ { .procname = "my_int", .data = &my_int, .maxlen = sizeof(int), .mode = 0644, .proc_handler = proc_dointvec, } /* After */ SYSCTL_TBL_ENTRY("my_int", my_int, 0644) No functional change intended. Suggested-by: Joel Granados <joel.granados@kernel.org> Signed-off-by: Wen Yang <wen.yang@linux.dev> --- include/linux/sysctl.h | 131 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 131 insertions(+) diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h index 2886fbceb5d6..3d10e2e9d6dc 100644 --- a/include/linux/sysctl.h +++ b/include/linux/sysctl.h @@ -175,6 +175,137 @@ struct ctl_table { void *extra2; } __randomize_layout; +/* Special marker type to represent NULL data in sysctl entries */ +struct __sysctl_null_type { char __dummy; }; +extern const struct __sysctl_null_type __sysctl_null_marker; + +/* Use SYSCTL_NULL to indicate a sysctl entry without associated data */ +#define SYSCTL_NULL (__sysctl_null_marker) + +/* Validate NAME is a string literal and return it (fails on non-literals) */ +#define __SYSCTL_PROCNAME(NAME) \ + ("" NAME "") + +/* Generate data pointer: NULL for SYSCTL_NULL, &VAR for actual variables */ +#define __SYSCTL_DATA(VAR) \ + _Generic((VAR), \ + struct __sysctl_null_type : ((void *)NULL), \ + default : \ + ((void *)&(VAR)) \ + ) + +/* Compute maxlen for NULL entries: use explicit MAXLEN if >0, else 0 */ +#define __SYSCTL_MAXLEN_NULL(MAXLEN) \ + ((MAXLEN) > 0 ? (size_t)(MAXLEN) : (size_t)0) + +/* Compute maxlen: use explicit MAXLEN if >0, else sizeof(VAR) */ +#define __SYSCTL_MAXLEN_VAR(VAR, MAXLEN) \ + ((MAXLEN) >= 0 ? (size_t)(MAXLEN) : (size_t)sizeof(VAR)) + +/* Compute maxlen: auto-detect based on entry type (NULL vs variable) */ +#define __SYSCTL_MAXLEN(VAR, MAXLEN) \ + _Generic((VAR), \ + struct __sysctl_null_type : __SYSCTL_MAXLEN_NULL(MAXLEN), \ + default : \ + __SYSCTL_MAXLEN_VAR(VAR, MAXLEN) \ + ) + +/* Validate MODE is compatible with umode_t and return it */ +#define __SYSCTL_MODE(MODE) \ + (0 ? (umode_t)0 : (MODE)) + +/* Validate HANDLER matches proc_handler signature and return it */ +#define __SYSCTL_PROC_HANDLER(HANDLER) \ + (0 ? (proc_handler *)0 : (HANDLER)) + +/* Auto-select appropriate proc_handler based on VAR's type */ +#define __SYSCTL_AUTO_HANDLER(VAR) \ + _Generic((VAR), \ + int : (proc_handler *)proc_dointvec, \ + unsigned int : (proc_handler *)proc_douintvec, \ + long : (proc_handler *)proc_dointvec, \ + unsigned long : (proc_handler *)proc_douintvec, \ + default : \ + (proc_handler *)NULL \ + ) + +/* Auto-select range-checking proc_handler based on VAR's type */ +#define __SYSCTL_AUTO_HANDLER_MINMAX(VAR) \ + _Generic((VAR), \ + int : (proc_handler *)proc_dointvec_minmax, \ + unsigned int : (proc_handler *)proc_douintvec_minmax, \ + long : (proc_handler *)proc_doulongvec_minmax, \ + unsigned long : (proc_handler *)proc_doulongvec_minmax, \ + default : \ + (proc_handler *)NULL \ + ) + +/* Validate PTR is pointer-compatible and return it as void* */ +#define __SYSCTL_EXTRA(PTR) \ + (0 ? (void *)0 : (PTR)) + +/* Initialize a ctl_table entry with all parameters */ +#define __SYSCTL_TBL_ENTRY(NAME, VAR, MODE, HANDLER, MIN, MAX, MAXLEN) \ + { \ + .procname = __SYSCTL_PROCNAME(NAME), \ + .data = __SYSCTL_DATA(VAR), \ + .maxlen = __SYSCTL_MAXLEN(VAR, MAXLEN), \ + .mode = __SYSCTL_MODE(MODE), \ + .proc_handler = __SYSCTL_PROC_HANDLER(HANDLER), \ + .poll = NULL, \ + .extra1 = __SYSCTL_EXTRA(MIN), \ + .extra2 = __SYSCTL_EXTRA(MAX), \ + } + +/* Count the number of variadic arguments (supports 1-7 arguments) */ +#define __SYSCTL_NARG(...) \ + __SYSCTL_NARG_(__VA_ARGS__, 7, 6, 5, 4, 3, 2, 1, 0) + +/* Helper for __SYSCTL_NARG - maps arguments to their count */ +#define __SYSCTL_NARG_(_1, _2, _3, _4, _5, _6, _7, N, ...) N + +/* Concatenate two tokens */ +#define __SYSCTL_CONCAT(A, B) A##B + +/* Select macro variant based on argument count */ +#define __SYSCTL_SELECT(NAME, N) __SYSCTL_CONCAT(NAME, N) + +/* SYSCTL_TBL_ENTRY(var) - use variable name as procname, readonly, auto handler */ +#define __SYSCTL_TBL_ENTRY_1(VAR) \ + __SYSCTL_TBL_ENTRY(#VAR, VAR, 0444, \ + __SYSCTL_AUTO_HANDLER(VAR), NULL, NULL, -1) + +/* SYSCTL_TBL_ENTRY(name, var) - custom name, readonly, auto handler */ +#define __SYSCTL_TBL_ENTRY_2(NAME, VAR) \ + __SYSCTL_TBL_ENTRY(NAME, VAR, 0444, \ + __SYSCTL_AUTO_HANDLER(VAR), NULL, NULL, -1) + +/* SYSCTL_TBL_ENTRY(name, var, mode) - custom name and mode, auto handler */ +#define __SYSCTL_TBL_ENTRY_3(NAME, VAR, MODE) \ + __SYSCTL_TBL_ENTRY(NAME, VAR, MODE, \ + __SYSCTL_AUTO_HANDLER(VAR), NULL, NULL, -1) + +/* SYSCTL_TBL_ENTRY(name, var, mode, handler) - custom handler */ +#define __SYSCTL_TBL_ENTRY_4(NAME, VAR, MODE, HANDLER) \ + __SYSCTL_TBL_ENTRY(NAME, VAR, MODE, HANDLER, NULL, NULL, -1) + +/* SYSCTL_TBL_ENTRY(name, var, mode, min, max) - auto range-checking handler */ +#define __SYSCTL_TBL_ENTRY_5(NAME, VAR, MODE, MIN, MAX) \ + __SYSCTL_TBL_ENTRY(NAME, VAR, MODE, \ + __SYSCTL_AUTO_HANDLER_MINMAX(VAR), MIN, MAX, -1) + +/* SYSCTL_TBL_ENTRY(name, var, mode, handler, min, max) - custom handler with range */ +#define __SYSCTL_TBL_ENTRY_6(NAME, VAR, MODE, HANDLER, MIN, MAX) \ + __SYSCTL_TBL_ENTRY(NAME, VAR, MODE, HANDLER, MIN, MAX, -1) + +/* SYSCTL_TBL_ENTRY(name, var, mode, handler, min, max, maxlen) - full control */ +#define __SYSCTL_TBL_ENTRY_7(NAME, VAR, MODE, HANDLER, MIN, MAX, MAXLEN) \ + __SYSCTL_TBL_ENTRY(NAME, VAR, MODE, HANDLER, MIN, MAX, MAXLEN) + +/* Define a sysctl table entry with automatic type detection and parameter handling */ +#define SYSCTL_TBL_ENTRY(...) \ + __SYSCTL_SELECT(__SYSCTL_TBL_ENTRY_, __SYSCTL_NARG(__VA_ARGS__))(__VA_ARGS__) + struct ctl_node { struct rb_node node; struct ctl_table_header *header; -- 2.25.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [RFC PATCH v2 1/2] sysctl: introduce SYSCTL_ENTRY() helper macro 2026-02-07 20:35 ` [RFC PATCH v2 1/2] sysctl: introduce SYSCTL_ENTRY() helper macro wen.yang @ 2026-02-19 10:33 ` Joel Granados 0 siblings, 0 replies; 5+ messages in thread From: Joel Granados @ 2026-02-19 10:33 UTC (permalink / raw) To: wen.yang; +Cc: linux-kernel [-- Attachment #1: Type: text/plain, Size: 11005 bytes --] On Sun, Feb 08, 2026 at 04:35:16AM +0800, wen.yang@linux.dev wrote: > From: Wen Yang <wen.yang@linux.dev> > > Add SYSCTL_ENTRY() macro to simplify struct ctl_table initialization. > This macro provides automatic type detection, handler selection, and > sensible defaults, reducing boilerplate and potential errors. > > Based on discussion and suggestions from: > https://sysctl-dev-rtd.readthedocs.io/en/latest/notes/ctltable_entry_macro.html#table-entry-macro This "read the docs" is something that I use to help me keep track of things and is not "official" yet. So I would keep it out of the commit messages for now. Its OK to put it in the cover letter, but not in the commit messages. > https://lore.kernel.org/all/psot4oeauxi3yyj2w4ajm3tfgtcsvao4rhv5sgd5s6ymmjgojk@p3vrj3qluban/ > > Features: > - Automatic type detection and handler selection via _Generic() > - Supports int, unsigned int, long, unsigned long > - Auto-selects range-checking handlers when min/max provided > > - Flexible calling conventions (1-7 arguments): > - SYSCTL_TBL_ENTRY(var) -> readonly, auto-handler > - SYSCTL_TBL_ENTRY(name, var) -> custom name > - SYSCTL_TBL_ENTRY(name, var, mode) -> custom mode > - SYSCTL_TBL_ENTRY(name, var, mode, handler) -> custom handler > - SYSCTL_TBL_ENTRY(name, var, mode, min, max) -> with range > - SYSCTL_TBL_ENTRY(name, var, mode, handler, min, max) -> full control > - SYSCTL_TBL_ENTRY(name, var, mode, handler, min, max, maxlen) -> explicit maxlen Use a unique macro name ----------------------- I see two problems with differentiating on number of args: 1. We will run into issues when we need to different macros with the same number of args 2. Having the same name for the different semantics will just confuse the caller. I have modified [1] to describe a "capital letter approach" to the naming. Use "V" for variable, "N" for name, "M" for mode, "R" for range (min, max), "H" for handler and "L for maxlen. In this way you append the argument expectation at the end of the macro name: (e.g. CTLTBL_ENTRY_VN would expect a variable and a name). The order of the arguments should follow the order in the name. Reduce the name further ----------------------- SYSCTL_TBL_ENTRY_* is getting a bit too long, so lets reduce it even further to CTLTBL_ENTRY_*. [1] https://sysctl-dev-rtd.readthedocs.io/en/latest/notes/ctltable_entry_macro.html#table-entry-macro > > - Smart defaults: > - Auto address-of: SYSCTL_ENTRY(my_var) -> .data = &my_var > - Auto maxlen: uses sizeof(var) when not specified > - SYSCTL_NULL marker for entries without data > > - Compile-time validation: > - NAME must be string literal > - MODE, HANDLER validated via type compatibility checks > - VAR must be lvalue > > Example usage: > > static int my_int = 256; > > /* Before */ > { > .procname = "my_int", > .data = &my_int, > .maxlen = sizeof(int), > .mode = 0644, > .proc_handler = proc_dointvec, > } > > /* After */ > SYSCTL_TBL_ENTRY("my_int", my_int, 0644) > > No functional change intended. > > Suggested-by: Joel Granados <joel.granados@kernel.org> > Signed-off-by: Wen Yang <wen.yang@linux.dev> > --- > include/linux/sysctl.h | 131 +++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 131 insertions(+) > > diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h > index 2886fbceb5d6..3d10e2e9d6dc 100644 > --- a/include/linux/sysctl.h > +++ b/include/linux/sysctl.h > @@ -175,6 +175,137 @@ struct ctl_table { > void *extra2; > } __randomize_layout; > > +/* Special marker type to represent NULL data in sysctl entries */ > +struct __sysctl_null_type { char __dummy; }; > +extern const struct __sysctl_null_type __sysctl_null_marker; > + > +/* Use SYSCTL_NULL to indicate a sysctl entry without associated data */ > +#define SYSCTL_NULL (__sysctl_null_marker) > + > +/* Validate NAME is a string literal and return it (fails on non-literals) */ > +#define __SYSCTL_PROCNAME(NAME) \ > + ("" NAME "") > + > +/* Generate data pointer: NULL for SYSCTL_NULL, &VAR for actual variables */ > +#define __SYSCTL_DATA(VAR) \ > + _Generic((VAR), \ > + struct __sysctl_null_type : ((void *)NULL), \ > + default : \ > + ((void *)&(VAR)) \ > + ) > + > +/* Compute maxlen for NULL entries: use explicit MAXLEN if >0, else 0 */ > +#define __SYSCTL_MAXLEN_NULL(MAXLEN) \ > + ((MAXLEN) > 0 ? (size_t)(MAXLEN) : (size_t)0) > + > +/* Compute maxlen: use explicit MAXLEN if >0, else sizeof(VAR) */ > +#define __SYSCTL_MAXLEN_VAR(VAR, MAXLEN) \ > + ((MAXLEN) >= 0 ? (size_t)(MAXLEN) : (size_t)sizeof(VAR)) > + > +/* Compute maxlen: auto-detect based on entry type (NULL vs variable) */ > +#define __SYSCTL_MAXLEN(VAR, MAXLEN) \ > + _Generic((VAR), \ > + struct __sysctl_null_type : __SYSCTL_MAXLEN_NULL(MAXLEN), \ > + default : \ > + __SYSCTL_MAXLEN_VAR(VAR, MAXLEN) \ > + ) > + > +/* Validate MODE is compatible with umode_t and return it */ > +#define __SYSCTL_MODE(MODE) \ > + (0 ? (umode_t)0 : (MODE)) > + > +/* Validate HANDLER matches proc_handler signature and return it */ > +#define __SYSCTL_PROC_HANDLER(HANDLER) \ > + (0 ? (proc_handler *)0 : (HANDLER)) > + > +/* Auto-select appropriate proc_handler based on VAR's type */ > +#define __SYSCTL_AUTO_HANDLER(VAR) \ > + _Generic((VAR), \ > + int : (proc_handler *)proc_dointvec, \ > + unsigned int : (proc_handler *)proc_douintvec, \ > + long : (proc_handler *)proc_dointvec, \ > + unsigned long : (proc_handler *)proc_douintvec, \ > + default : \ > + (proc_handler *)NULL \ > + ) > + > +/* Auto-select range-checking proc_handler based on VAR's type */ > +#define __SYSCTL_AUTO_HANDLER_MINMAX(VAR) \ > + _Generic((VAR), \ > + int : (proc_handler *)proc_dointvec_minmax, \ > + unsigned int : (proc_handler *)proc_douintvec_minmax, \ > + long : (proc_handler *)proc_doulongvec_minmax, \ > + unsigned long : (proc_handler *)proc_doulongvec_minmax, \ > + default : \ > + (proc_handler *)NULL \ > + ) > + > +/* Validate PTR is pointer-compatible and return it as void* */ > +#define __SYSCTL_EXTRA(PTR) \ > + (0 ? (void *)0 : (PTR)) > + > +/* Initialize a ctl_table entry with all parameters */ > +#define __SYSCTL_TBL_ENTRY(NAME, VAR, MODE, HANDLER, MIN, MAX, MAXLEN) \ Change the arg list to (VAR, NAME, MODE, HANDLER, MIN, MAX, MAXLEN) This seems superfluous but I want the variable to be the center of attention. sysctl is, after all, a way to interact with internal kernel variables. Also, change the name to __CTLTBL_ENTRY to be consistent. > + { \ > + .procname = __SYSCTL_PROCNAME(NAME), \ > + .data = __SYSCTL_DATA(VAR), \ > + .maxlen = __SYSCTL_MAXLEN(VAR, MAXLEN), \ > + .mode = __SYSCTL_MODE(MODE), \ > + .proc_handler = __SYSCTL_PROC_HANDLER(HANDLER), \ > + .poll = NULL, \ > + .extra1 = __SYSCTL_EXTRA(MIN), \ > + .extra2 = __SYSCTL_EXTRA(MAX), \ ^^^^^^^^ Please use tabs after the entry names > + } > + > +/* Count the number of variadic arguments (supports 1-7 arguments) */ > +#define __SYSCTL_NARG(...) \ > + __SYSCTL_NARG_(__VA_ARGS__, 7, 6, 5, 4, 3, 2, 1, 0) This is no longer needed since we are not selecting on number of args? > + > +/* Helper for __SYSCTL_NARG - maps arguments to their count */ > +#define __SYSCTL_NARG_(_1, _2, _3, _4, _5, _6, _7, N, ...) N This is no longer needed since we are not selecting on number of args? > + > +/* Concatenate two tokens */ > +#define __SYSCTL_CONCAT(A, B) A##B > + > +/* Select macro variant based on argument count */ > +#define __SYSCTL_SELECT(NAME, N) __SYSCTL_CONCAT(NAME, N) This is no longer needed since we are not selecting on number of args? > + > +/* SYSCTL_TBL_ENTRY(var) - use variable name as procname, readonly, auto handler */ > +#define __SYSCTL_TBL_ENTRY_1(VAR) \ This should be #define CTLTBL_ENTRY_V(VAR) \ > + __SYSCTL_TBL_ENTRY(#VAR, VAR, 0444, \ > + __SYSCTL_AUTO_HANDLER(VAR), NULL, NULL, -1) > + > +/* SYSCTL_TBL_ENTRY(name, var) - custom name, readonly, auto handler */ > +#define __SYSCTL_TBL_ENTRY_2(NAME, VAR) \ This should be #define CTLTBL_ENTRY_VN(VAR, NAME) > + __SYSCTL_TBL_ENTRY(NAME, VAR, 0444, \ > + __SYSCTL_AUTO_HANDLER(VAR), NULL, NULL, -1) > + > +/* SYSCTL_TBL_ENTRY(name, var, mode) - custom name and mode, auto handler */ > +#define __SYSCTL_TBL_ENTRY_3(NAME, VAR, MODE) \ This should be #define CTLTBL_ENTRY_VNM(VAR, NAME, MODE) > + __SYSCTL_TBL_ENTRY(NAME, VAR, MODE, \ > + __SYSCTL_AUTO_HANDLER(VAR), NULL, NULL, -1) > + > +/* SYSCTL_TBL_ENTRY(name, var, mode, handler) - custom handler */ > +#define __SYSCTL_TBL_ENTRY_4(NAME, VAR, MODE, HANDLER) \ This should be #define CTLTBL_ENTRY_VNMH(VAR, NAME, MODE, HANDLER) > + __SYSCTL_TBL_ENTRY(NAME, VAR, MODE, HANDLER, NULL, NULL, -1) > + > +/* SYSCTL_TBL_ENTRY(name, var, mode, min, max) - auto range-checking handler */ > +#define __SYSCTL_TBL_ENTRY_5(NAME, VAR, MODE, MIN, MAX) \ This should be #define CTLTBL_ENTRY_VNMR(VAR, NAME, MODE, MIN, MAX) > + __SYSCTL_TBL_ENTRY(NAME, VAR, MODE, \ > + __SYSCTL_AUTO_HANDLER_MINMAX(VAR), MIN, MAX, -1) > + > +/* SYSCTL_TBL_ENTRY(name, var, mode, handler, min, max) - custom handler with range */ > +#define __SYSCTL_TBL_ENTRY_6(NAME, VAR, MODE, HANDLER, MIN, MAX) \ This should be #define CTLTBL_ENTRY_VNMHR(VAR, NAME, MODE, HANDLER, MIN, MAX) > + __SYSCTL_TBL_ENTRY(NAME, VAR, MODE, HANDLER, MIN, MAX, -1) > + > +/* SYSCTL_TBL_ENTRY(name, var, mode, handler, min, max, maxlen) - full control */ > +#define __SYSCTL_TBL_ENTRY_7(NAME, VAR, MODE, HANDLER, MIN, MAX, MAXLEN) \ This should be #define CTLTBL_ENTRY_VNMHRL(VAR, NAME, MODE, HANDLER, MIN, MAX, MAXLEN) > + __SYSCTL_TBL_ENTRY(NAME, VAR, MODE, HANDLER, MIN, MAX, MAXLEN) > + > +/* Define a sysctl table entry with automatic type detection and parameter handling */ > +#define SYSCTL_TBL_ENTRY(...) \ > + __SYSCTL_SELECT(__SYSCTL_TBL_ENTRY_, __SYSCTL_NARG(__VA_ARGS__))(__VA_ARGS__) This is no longer needed since we are not selecting on number of args? > + > struct ctl_node { > struct rb_node node; > struct ctl_table_header *header; > -- > 2.25.1E, > I don't see (maybe I missed it) the macro that takes care of the case where there is no variable. There are several places where callers set the data as a NULL ptr and pass a custom proc handler that "finds" the correct variable to modify. Do you have an option for this case? I think it would look something like this: #define CTLTBL_ENTRY_NH(NAME, HANDLER) \ .... Its very nice! thx again for taking this. Hope to see your RFC V3. Best -- Joel Granados PS: I believe that after we go through your V3, we can share and see what other folks have to say. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 659 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* [RFC PATCH v2 2/2] sysctl: convert kernel/sysctl-test.c to use SYSCTL_TBL_ENTRY() 2026-02-07 20:35 [RFC PATCH v2 0/2] sysctl: refactor ctl_table creation and change extra{1,2} type wen.yang 2026-02-07 20:35 ` [RFC PATCH v2 1/2] sysctl: introduce SYSCTL_ENTRY() helper macro wen.yang @ 2026-02-07 20:35 ` wen.yang 2026-02-19 8:59 ` [RFC PATCH v2 0/2] sysctl: refactor ctl_table creation and change extra{1,2} type Joel Granados 2 siblings, 0 replies; 5+ messages in thread From: wen.yang @ 2026-02-07 20:35 UTC (permalink / raw) To: Joel Granados; +Cc: linux-kernel, Wen Yang From: Wen Yang <wen.yang@linux.dev> Convert sysctl test cases to use the new SYSCTL_TBL_ENTRY() macro, demonstrating the macro's benefits in reducing boilerplate and improving code readability. This conversion shows typical usage patterns: - Standard entries with range checking: Before (11 lines): struct ctl_table table = { .procname = "foo", .data = &data, .maxlen = sizeof(int), .mode = 0644, .proc_handler = proc_dointvec, .extra1 = SYSCTL_ZERO, .extra2 = SYSCTL_ONE_HUNDRED, }; After (2 lines): struct ctl_table table = SYSCTL_TBL_ENTRY("foo", data, 0644, SYSCTL_ZERO, SYSCTL_ONE_HUNDRED); - NULL data entries: Before (11 lines with explicit .data = NULL): struct ctl_table null_data_table = { .procname = "foo", .data = NULL, .maxlen = sizeof(int), ... }; After (3 lines using SYSCTL_NULL marker): struct ctl_table null_data_table = SYSCTL_TBL_ENTRY("foo", SYSCTL_NULL, 0644, proc_dointvec, SYSCTL_ZERO, SYSCTL_ONE_HUNDRED); - Explicit maxlen override: SYSCTL_TBL_ENTRY("foo", SYSCTL_NULL, 0644, proc_dointvec, SYSCTL_ZERO, SYSCTL_ONE_HUNDRED, 0); Benefits demonstrated: - 60% reduction in lines of code (from ~110 to ~44 lines for table definitions) - Automatic address-of operator (&data) - macro handles it - Automatic maxlen calculation via sizeof() when appropriate - Improved readability - focus on important parameters - Consistent formatting across all test cases The macro automatically: - Takes the address of 'data' variable - Computes maxlen as sizeof(int) - Selects proc_dointvec based on type (though explicitly provided here) - Validates all parameters at compile time No functional change - all tests continue to pass. [17:55:08] ================ sysctl_test (10 subtests) ================= [17:55:08] [PASSED] sysctl_test_api_dointvec_null_tbl_data [17:55:08] [PASSED] sysctl_test_api_dointvec_table_maxlen_unset [17:55:08] [PASSED] sysctl_test_api_dointvec_table_len_is_zero [17:55:08] [PASSED] sysctl_test_api_dointvec_table_read_but_position_set [17:55:08] [PASSED] sysctl_test_dointvec_read_happy_single_positive [17:55:08] [PASSED] sysctl_test_dointvec_read_happy_single_negative [17:55:08] [PASSED] sysctl_test_dointvec_write_happy_single_positive [17:55:08] [PASSED] sysctl_test_dointvec_write_happy_single_negative [17:55:08] [PASSED] sysctl_test_api_dointvec_write_single_less_int_min [17:55:08] [PASSED] sysctl_test_api_dointvec_write_single_greater_int_max [17:55:08] =================== [PASSED] sysctl_test =================== [17:55:08] ============================================================ [17:55:08] Testing complete. Ran 10 tests: passed: 10 Suggested-by: Joel Granados <joel.granados@kernel.org> Signed-off-by: Wen Yang <wen.yang@linux.dev> --- kernel/sysctl-test.c | 139 +++++++++++++------------------------------ 1 file changed, 40 insertions(+), 99 deletions(-) diff --git a/kernel/sysctl-test.c b/kernel/sysctl-test.c index 92f94ea28957..bacd782d8cee 100644 --- a/kernel/sysctl-test.c +++ b/kernel/sysctl-test.c @@ -15,20 +15,15 @@ */ static void sysctl_test_api_dointvec_null_tbl_data(struct kunit *test) { - struct ctl_table null_data_table = { - .procname = "foo", - /* - * Here we are testing that proc_dointvec behaves correctly when - * we give it a NULL .data field. Normally this would point to a - * piece of memory where the value would be stored. - */ - .data = NULL, - .maxlen = sizeof(int), - .mode = 0644, - .proc_handler = proc_dointvec, - .extra1 = SYSCTL_ZERO, - .extra2 = SYSCTL_ONE_HUNDRED, - }; + /* + * Here we are testing that proc_dointvec behaves correctly when + * we give it a NULL .data field. Normally this would point to a + * piece of memory where the value would be stored. + */ + struct ctl_table null_data_table = SYSCTL_TBL_ENTRY("foo", SYSCTL_NULL, 0644,\ + proc_dointvec, \ + SYSCTL_ZERO, \ + SYSCTL_ONE_HUNDRED); /* * proc_dointvec expects a buffer in user space, so we allocate one. We * also need to cast it to __user so sparse doesn't get mad. @@ -66,19 +61,14 @@ static void sysctl_test_api_dointvec_null_tbl_data(struct kunit *test) static void sysctl_test_api_dointvec_table_maxlen_unset(struct kunit *test) { int data = 0; - struct ctl_table data_maxlen_unset_table = { - .procname = "foo", - .data = &data, - /* - * So .data is no longer NULL, but we tell proc_dointvec its - * length is 0, so it still shouldn't try to use it. - */ - .maxlen = 0, - .mode = 0644, - .proc_handler = proc_dointvec, - .extra1 = SYSCTL_ZERO, - .extra2 = SYSCTL_ONE_HUNDRED, - }; + /* + * So .data is no longer NULL, but we tell proc_dointvec its + * length is 0, so it still shouldn't try to use it. + */ + struct ctl_table data_maxlen_unset_table = SYSCTL_TBL_ENTRY("foo", \ + data, 0644, proc_dointvec, SYSCTL_ZERO, \ + SYSCTL_ONE_HUNDRED, 0); + void __user *buffer = (void __user *)kunit_kzalloc(test, sizeof(int), GFP_USER); size_t len; @@ -113,15 +103,8 @@ static void sysctl_test_api_dointvec_table_len_is_zero(struct kunit *test) { int data = 0; /* Good table. */ - struct ctl_table table = { - .procname = "foo", - .data = &data, - .maxlen = sizeof(int), - .mode = 0644, - .proc_handler = proc_dointvec, - .extra1 = SYSCTL_ZERO, - .extra2 = SYSCTL_ONE_HUNDRED, - }; + struct ctl_table table = SYSCTL_TBL_ENTRY("foo", \ + data, 0644, SYSCTL_ZERO, SYSCTL_ONE_HUNDRED); void __user *buffer = (void __user *)kunit_kzalloc(test, sizeof(int), GFP_USER); /* @@ -147,15 +130,9 @@ static void sysctl_test_api_dointvec_table_read_but_position_set( { int data = 0; /* Good table. */ - struct ctl_table table = { - .procname = "foo", - .data = &data, - .maxlen = sizeof(int), - .mode = 0644, - .proc_handler = proc_dointvec, - .extra1 = SYSCTL_ZERO, - .extra2 = SYSCTL_ONE_HUNDRED, - }; + struct ctl_table table = SYSCTL_TBL_ENTRY("foo", data, 0644, \ + SYSCTL_ZERO, \ + SYSCTL_ONE_HUNDRED); void __user *buffer = (void __user *)kunit_kzalloc(test, sizeof(int), GFP_USER); /* @@ -182,15 +159,9 @@ static void sysctl_test_dointvec_read_happy_single_positive(struct kunit *test) { int data = 0; /* Good table. */ - struct ctl_table table = { - .procname = "foo", - .data = &data, - .maxlen = sizeof(int), - .mode = 0644, - .proc_handler = proc_dointvec, - .extra1 = SYSCTL_ZERO, - .extra2 = SYSCTL_ONE_HUNDRED, - }; + struct ctl_table table = SYSCTL_TBL_ENTRY("foo", data, 0644, \ + SYSCTL_ZERO, \ + SYSCTL_ONE_HUNDRED); size_t len = 4; loff_t pos = 0; char *buffer = kunit_kzalloc(test, len, GFP_USER); @@ -213,15 +184,9 @@ static void sysctl_test_dointvec_read_happy_single_negative(struct kunit *test) { int data = 0; /* Good table. */ - struct ctl_table table = { - .procname = "foo", - .data = &data, - .maxlen = sizeof(int), - .mode = 0644, - .proc_handler = proc_dointvec, - .extra1 = SYSCTL_ZERO, - .extra2 = SYSCTL_ONE_HUNDRED, - }; + struct ctl_table table = SYSCTL_TBL_ENTRY("foo", data, 0644, \ + SYSCTL_ZERO, \ + SYSCTL_ONE_HUNDRED); size_t len = 5; loff_t pos = 0; char *buffer = kunit_kzalloc(test, len, GFP_USER); @@ -242,15 +207,9 @@ static void sysctl_test_dointvec_write_happy_single_positive(struct kunit *test) { int data = 0; /* Good table. */ - struct ctl_table table = { - .procname = "foo", - .data = &data, - .maxlen = sizeof(int), - .mode = 0644, - .proc_handler = proc_dointvec, - .extra1 = SYSCTL_ZERO, - .extra2 = SYSCTL_ONE_HUNDRED, - }; + struct ctl_table table = SYSCTL_TBL_ENTRY("foo", data, 0644, \ + SYSCTL_ZERO, \ + SYSCTL_ONE_HUNDRED); char input[] = "9"; size_t len = sizeof(input) - 1; loff_t pos = 0; @@ -272,15 +231,9 @@ static void sysctl_test_dointvec_write_happy_single_positive(struct kunit *test) static void sysctl_test_dointvec_write_happy_single_negative(struct kunit *test) { int data = 0; - struct ctl_table table = { - .procname = "foo", - .data = &data, - .maxlen = sizeof(int), - .mode = 0644, - .proc_handler = proc_dointvec, - .extra1 = SYSCTL_ZERO, - .extra2 = SYSCTL_ONE_HUNDRED, - }; + struct ctl_table table = SYSCTL_TBL_ENTRY("foo", data, 0644, \ + SYSCTL_ZERO, \ + SYSCTL_ONE_HUNDRED); char input[] = "-9"; size_t len = sizeof(input) - 1; loff_t pos = 0; @@ -304,15 +257,9 @@ static void sysctl_test_api_dointvec_write_single_less_int_min( struct kunit *test) { int data = 0; - struct ctl_table table = { - .procname = "foo", - .data = &data, - .maxlen = sizeof(int), - .mode = 0644, - .proc_handler = proc_dointvec, - .extra1 = SYSCTL_ZERO, - .extra2 = SYSCTL_ONE_HUNDRED, - }; + struct ctl_table table = SYSCTL_TBL_ENTRY("foo", data, 0644, \ + SYSCTL_ZERO, \ + SYSCTL_ONE_HUNDRED); size_t max_len = 32, len = max_len; loff_t pos = 0; char *buffer = kunit_kzalloc(test, max_len, GFP_USER); @@ -342,15 +289,9 @@ static void sysctl_test_api_dointvec_write_single_greater_int_max( struct kunit *test) { int data = 0; - struct ctl_table table = { - .procname = "foo", - .data = &data, - .maxlen = sizeof(int), - .mode = 0644, - .proc_handler = proc_dointvec, - .extra1 = SYSCTL_ZERO, - .extra2 = SYSCTL_ONE_HUNDRED, - }; + struct ctl_table table = SYSCTL_TBL_ENTRY("foo", data, 0644, \ + SYSCTL_ZERO, \ + SYSCTL_ONE_HUNDRED); size_t max_len = 32, len = max_len; loff_t pos = 0; char *buffer = kunit_kzalloc(test, max_len, GFP_USER); -- 2.25.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [RFC PATCH v2 0/2] sysctl: refactor ctl_table creation and change extra{1,2} type 2026-02-07 20:35 [RFC PATCH v2 0/2] sysctl: refactor ctl_table creation and change extra{1,2} type wen.yang 2026-02-07 20:35 ` [RFC PATCH v2 1/2] sysctl: introduce SYSCTL_ENTRY() helper macro wen.yang 2026-02-07 20:35 ` [RFC PATCH v2 2/2] sysctl: convert kernel/sysctl-test.c to use SYSCTL_TBL_ENTRY() wen.yang @ 2026-02-19 8:59 ` Joel Granados 2 siblings, 0 replies; 5+ messages in thread From: Joel Granados @ 2026-02-19 8:59 UTC (permalink / raw) To: wen.yang; +Cc: linux-kernel [-- Attachment #1: Type: text/plain, Size: 1689 bytes --] Hey Wen Sorry for taking so long to come back to you and thx taking this on. On Sun, Feb 08, 2026 at 04:35:15AM +0800, wen.yang@linux.dev wrote: > From: Wen Yang <wen.yang@linux.dev> > > This series introduces macros to centralize ctl_table initialization, > preparing for future type changes to extra1/extra2 fields. Remember to include that the main motivations here is to avoid treewide patch series. These usually touch lots of subsystem, requires the attention of many maintainers and sometimes needs to be Pulled to mainline in a special way. They create lots of unnecessary churn. Defining the motivation in the cover letter is key to get ppl to understand why you are doing what you are doing. > > Following Joel's suggestion [1], we start with helpers that work for all > existing patterns before making invasive changes. Having these two patches makes the review much easier. thx for reducing it. > > > [1] https://lore.kernel.org/all/sytyqb3ajm6ysoifwp57ga7gzlnzodhdsbgizbn3hqnlwytn5a@pbscftrq2qwl/ > [2] https://sysctl-dev-rtd.readthedocs.io/en/latest/notes/ctltable_entry_macro.html#table-entry-macro Feel free to send a PR to https://github.com/Joelgranados/sysctl_dev_rtd if you want to include any notes in the sysctl-dev-rtd. > > > Wen Yang (2): > sysctl: introduce SYSCTL_ENTRY() helper macro > sysctl: convert kernel/sysctl-test.c to use SYSCTL_TBL_ENTRY() > > include/linux/sysctl.h | 131 ++++++++++++++++++++++++++++++++++++++ > kernel/sysctl-test.c | 139 ++++++++++++----------------------------- > 2 files changed, 171 insertions(+), 99 deletions(-) > > -- > 2.25.1 > -- Joel Granados [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 659 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-02-19 10:34 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-07 20:35 [RFC PATCH v2 0/2] sysctl: refactor ctl_table creation and change extra{1,2} type wen.yang
2026-02-07 20:35 ` [RFC PATCH v2 1/2] sysctl: introduce SYSCTL_ENTRY() helper macro wen.yang
2026-02-19 10:33 ` Joel Granados
2026-02-07 20:35 ` [RFC PATCH v2 2/2] sysctl: convert kernel/sysctl-test.c to use SYSCTL_TBL_ENTRY() wen.yang
2026-02-19 8:59 ` [RFC PATCH v2 0/2] sysctl: refactor ctl_table creation and change extra{1,2} type Joel Granados
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox