From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out-188.mta0.migadu.com (out-188.mta0.migadu.com [91.218.175.188]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 1BE022FFDEB for ; Sun, 25 Jan 2026 20:08:54 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=91.218.175.188 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1769371737; cv=none; b=CkGyYTzhDnfqysJu6/Rr66cWRlsvwN3jD8lu8lu2OK6yYXZ9eNIrqMvpCk/+NcUeyrOp6OAdjDHxKSBeViPSpYJk7ZKi+dzlDTi6OXxNAE9DUL1lccu57DkWPlPXabqrDdg1Cw1JRwkilprLYeGOhDrFfx3JTmfrUfyIoH2gtWo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1769371737; c=relaxed/simple; bh=/p86/aSzIEevKN+KYtFtR9YY8Yn36iVtJf5SWOVD4Ho=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=Jb8uHtxMZidphOOkiXUYiH0FSxDFAbRMqwzGemAkJh1NWRADFH1MtFZsUkImUBRSjCmmHfrF4l2qb7+GR9MxI9L+6TPS/TY4b//Sy4bZpn/hNssNTzsIEea2f3NEz/muKXdrpHHVEfuzUcegRtZ4ZQ78GDLL1kBlv0k2h7e6O9Q= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=fWn2dXwn; arc=none smtp.client-ip=91.218.175.188 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="fWn2dXwn" Message-ID: <88f18992-8408-4c4c-aa57-07025256b466@linux.dev> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1769371733; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=op3p49bFAklmzZKe/v9OUL0L2IqkWWvPSd4MpLmB3r0=; b=fWn2dXwnjcIdC8rfGao+LAA0Xke3v8EvGHXIVvdBpQXm9QhNOCP196Z4K0Q7unQpZYpwhg xtnpfblCGOtooU+BJkDbIbHHWkOikxrRSyUdZoFfFrtTZjG1uRDB9nRyjrXxihlD7Abbs4 pD+F6OdIV6Ix47McZgOOcmBIjEmItJc= Date: Mon, 26 Jan 2026 04:08:46 +0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Subject: Re: [RFC PATCH 1/4] sysctl: add SYSCTL_ENTRY/SYSCTL_RANGE_ENTRY helpers for ctl_table init To: Joel Granados Cc: linux-kernel@vger.kernel.org References: <09f6437ba1aeb067be4d295ec79f2d51f5355ce0.1768324215.git.wen.yang@linux.dev> Content-Language: en-US X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Wen Yang In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Migadu-Flow: FLOW_OUT 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 >> >> 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 >> Link: https://lore.kernel.org/all/7mevcgca7xsd5gckrap22byjinhrgqnelu4y7hzo2r5t2cq7w4@nyaa42khwqez/#t >> Signed-off-by: Wen Yang >> --- >> 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 >> >