public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Wen Yang <wen.yang@linux.dev>
To: Joel Granados <joel.granados@kernel.org>
Cc: linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH 1/4] sysctl: add SYSCTL_ENTRY/SYSCTL_RANGE_ENTRY helpers for ctl_table init
Date: Mon, 26 Jan 2026 04:08:46 +0800	[thread overview]
Message-ID: <88f18992-8408-4c4c-aa57-07025256b466@linux.dev> (raw)
In-Reply-To: <psot4oeauxi3yyj2w4ajm3tfgtcsvao4rhv5sgd5s6ymmjgojk@p3vrj3qluban>



On 1/22/26 17:56, Joel Granados wrote:
> On Wed, Jan 14, 2026 at 01:40:30AM +0800, wen.yang@linux.dev wrote:
>> From: Wen Yang <wen.yang@linux.dev>
>>
>> Introduce SYSCTL_ENTRY() and SYSCTL_RANGE_ENTRY() (built on __SYSCTL_ENTRY())
>> to consolidate common struct ctl_table initializers in one place.
>> This is a preparatory refactor in support of future tree-wide sysctl
>> cleanups by making the how to build a ctl_table entry logic uniform.
>>
>> Also add SYSCTL_NULL for explicitly passing a NULL .data pointer through
>> the new macros, and convert a few in-tree users (kernel/sysctl.c) to
>> validate the new helpers.
>>
>> No functional change intended.
>>
>> Selftest were also made:
>>
>> [19:31:40] Starting KUnit Kernel (1/1)...
>> [19:31:40] ============================================================
>> Running tests with:
>> $ .kunit/linux kunit.filter_glob=sysctl_test kunit.enable=1 mem=1G console=tty kunit_shutdown=halt
>> [19:31:40] ================ sysctl_test (10 subtests) =================
>> [19:31:40] [PASSED] sysctl_test_api_dointvec_null_tbl_data
>> [19:31:40] [PASSED] sysctl_test_api_dointvec_table_maxlen_unset
>> [19:31:40] [PASSED] sysctl_test_api_dointvec_table_len_is_zero
>> [19:31:40] [PASSED] sysctl_test_api_dointvec_table_read_but_position_set
>> [19:31:40] [PASSED] sysctl_test_dointvec_read_happy_single_positive
>> [19:31:40] [PASSED] sysctl_test_dointvec_read_happy_single_negative
>> [19:31:40] [PASSED] sysctl_test_dointvec_write_happy_single_positive
>> [19:31:40] [PASSED] sysctl_test_dointvec_write_happy_single_negative
>> [19:31:40] [PASSED] sysctl_test_api_dointvec_write_single_less_int_min
>> [19:31:40] [PASSED] sysctl_test_api_dointvec_write_single_greater_int_max
>> [19:31:40] =================== [PASSED] sysctl_test ===================
>> [19:31:40] ============================================================
>> [19:31:40] Testing complete. Ran 10 tests: passed: 10
>> [19:31:40] Elapsed time: 26.755s total, 0.002s configuring, 26.632s building, 0.102s running
>>
>> Suggested-by: Joel Granados <joel.granados@kernel.org>
>> Link: https://lore.kernel.org/all/7mevcgca7xsd5gckrap22byjinhrgqnelu4y7hzo2r5t2cq7w4@nyaa42khwqez/#t
>> Signed-off-by: Wen Yang <wen.yang@linux.dev>
>> ---
>>   include/linux/sysctl.h |  18 ++++++
>>   kernel/sysctl-test.c   | 131 ++++++++++-------------------------------
>>   kernel/sysctl.c        |  43 ++------------
>>   3 files changed, 56 insertions(+), 136 deletions(-)
>>
>> diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
>> index 2886fbceb5d6..683422bc3e7f 100644
>> --- a/include/linux/sysctl.h
>> +++ b/include/linux/sysctl.h
>> @@ -53,6 +53,7 @@ struct ctl_dir;
>>   #define SYSCTL_MAXOLDUID		((void *)&sysctl_vals[10])
>>   #define SYSCTL_NEG_ONE			((void *)&sysctl_vals[11])
>>   
>> +#define SYSCTL_NULL			((void *)NULL)
>>   extern const int sysctl_vals[];
>>   
>>   #define SYSCTL_LONG_ZERO	((void *)&sysctl_long_vals[0])
>> @@ -175,6 +176,23 @@ struct ctl_table {
>>   	void *extra2;
>>   } __randomize_layout;
>>   
>> +#define __SYSCTL_ENTRY(NAME, DATA, TYPE, MODE, HANDLER, SMIN, SMAX)\
>> +	{							\
>> +		.procname	= NAME,				\
>> +		.data		= DATA,				\
> I would like to see at least one lvalue check for the macro parameters.
> Something like this
> 
> .data = (*(typeof(DATA)*)&(DATA))
> 
> In such a way that the compiler fails if an expression is passed.
> We can handle these cases (if they exist) in some other way
> 
>> +		.maxlen		= sizeof(TYPE),			\
>> +		.mode		= MODE,				\
>> +		.proc_handler	= HANDLER,			\
>> +		.extra1		= SMIN,				\
>> +		.extra2		= SMAX,				\
>> +	}
>> +
>> +#define SYSCTL_ENTRY(NAME, DATA, TYPE, MODE)			\
>> +	__SYSCTL_ENTRY(NAME, DATA, TYPE, MODE, proc_do##TYPE##vec, NULL, NULL)
>> +
>> +#define SYSCTL_RANGE_ENTRY(NAME, DATA, TYPE, MODE, SMIN, SMAX)	\
> I'm leaning more towards SYSCTL_ENTRY_RANGE or SYSCTL_ENTRY_R.
> 
> I am also keeping notes here if you are interested
> https://sysctl-dev-rtd.readthedocs.io/en/latest/notes/ctltable_entry_macro.html#table-entry-macro
> 

Great, thank you for your detailed comments.
These suggestions are indeed the best, implementable, robust, and highly 
readable.
I will soon make these changes according to them soon, probably within a 
day or two.


--
Best wishes,
Wen

> 
> 
>> +	__SYSCTL_ENTRY(NAME, DATA, TYPE, MODE, proc_do##TYPE##vec_minmax, SMIN, SMAX)
>> +
>>   struct ctl_node {
>>   	struct rb_node node;
>>   	struct ctl_table_header *header;
>> diff --git a/kernel/sysctl-test.c b/kernel/sysctl-test.c
>> index 92f94ea28957..7fb0e7f1e62f 100644
>> --- a/kernel/sysctl-test.c
>> +++ b/kernel/sysctl-test.c
>> @@ -15,20 +15,14 @@
> ....
>> -	{
>> -		.procname	= "ignore-unaligned-usertrap",
>> -		.data		= &no_unaligned_warning,
>> -		.maxlen		= sizeof (int),
>> -		.mode		= 0644,
>> -		.proc_handler	= proc_dointvec,
>> -	},
>> +	SYSCTL_ENTRY("ignore-unaligned-usertrap", &no_unaligned_warning, int, 0644),
>>   #endif
>>   };
>>   
>> -- 
>> 2.25.1
>>
> 

  reply	other threads:[~2026-01-25 20:08 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-13 17:40 [RFC PATCH 0/4] sysctl: refactor ctl_table creation and change extra{1,2} type wen.yang
2026-01-13 17:40 ` [RFC PATCH 1/4] sysctl: add SYSCTL_ENTRY/SYSCTL_RANGE_ENTRY helpers for ctl_table init wen.yang
2026-01-21 14:30   ` Joel Granados
2026-01-22  9:56   ` Joel Granados
2026-01-25 20:08     ` Wen Yang [this message]
2026-01-13 17:40 ` [RFC PATCH 2/4] sysctl: add helper functions to extract table->extra1/extra2 wen.yang
2026-01-13 17:40 ` [RFC PATCH 3/4] sysctl: support encoding values directly in the table entry wen.yang
2026-01-13 17:40 ` [RFC PATCH 4/4] scripts/coccinelle: add sysctl_table_init.cocci wen.yang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=88f18992-8408-4c4c-aa57-07025256b466@linux.dev \
    --to=wen.yang@linux.dev \
    --cc=joel.granados@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox