LVS
lvs-devel
Google
 
Web LinuxVirtualServer.org

Re: [PATCH 11/11] sysctl: treewide: constify the ctl_table argument of h

To: Thomas Weißschuh <linux@xxxxxxxxxxxxxx>
Subject: Re: [PATCH 11/11] sysctl: treewide: constify the ctl_table argument of handlers
Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>, Kees Cook <keescook@xxxxxxxxxxxx>, Alexei Starovoitov <ast@xxxxxxxxxx>, Daniel Borkmann <daniel@xxxxxxxxxxxxx>, John Fastabend <john.fastabend@xxxxxxxxx>, Andrii Nakryiko <andrii@xxxxxxxxxx>, Martin KaFai Lau <martin.lau@xxxxxxxxx>, Eduard Zingerman <eddyz87@xxxxxxxxx>, Song Liu <song@xxxxxxxxxx>, Yonghong Song <yonghong.song@xxxxxxxxx>, KP Singh <kpsingh@xxxxxxxxxx>, Stanislav Fomichev <sdf@xxxxxxxxxx>, Hao Luo <haoluo@xxxxxxxxxx>, Jiri Olsa <jolsa@xxxxxxxxxx>, Muchun Song <muchun.song@xxxxxxxxx>, Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>, "David S. Miller" <davem@xxxxxxxxxxxxx>, Eric Dumazet <edumazet@xxxxxxxxxx>, Jakub Kicinski <kuba@xxxxxxxxxx>, Paolo Abeni <pabeni@xxxxxxxxxx>, David Ahern <dsahern@xxxxxxxxxx>, Simon Horman <horms@xxxxxxxxxxxx>, Julian Anastasov <ja@xxxxxx>, Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>, Jozsef Kadlecsik <kadlec@xxxxxxxxxxxxx>, Florian Westphal <fw@xxxxxxxxx>, Luis Chamberlain <mcgrof@xxxxxxxxxx>, Joel Granados <j.granados@xxxxxxxxxxx>, Catalin Marinas <catalin.marinas@xxxxxxx>, Will Deacon <will@xxxxxxxxxx>, Heiko Carstens <hca@xxxxxxxxxxxxx>, Vasily Gorbik <gor@xxxxxxxxxxxxx>, Alexander Gordeev <agordeev@xxxxxxxxxxxxx>, Christian Borntraeger <borntraeger@xxxxxxxxxxxxx>, Sven Schnelle <svens@xxxxxxxxxxxxx>, Gerald Schaefer <gerald.schaefer@xxxxxxxxxxxxx>, Thomas Gleixner <tglx@xxxxxxxxxxxxx>, Ingo Molnar <mingo@xxxxxxxxxx>, Borislav Petkov <bp@xxxxxxxxx>, Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>, x86@xxxxxxxxxx, "H. Peter Anvin" <hpa@xxxxxxxxx>, Phillip Potter <phil@xxxxxxxxxxxxxxxx>, Theodore Ts'o <tytso@xxxxxxx>, "Jason A. Donenfeld" <Jason@xxxxxxxxx>, Sudip Mukherjee <sudipm.mukherjee@xxxxxxxxx>, Mark Rutland <mark.rutland@xxxxxxx>, Atish Patra <atishp@xxxxxxxxxxxxxx>, Anup Patel <anup@xxxxxxxxxxxxxx>, Paul Walmsley <paul.walmsley@xxxxxxxxxx>, Palmer Dabbelt <palmer@xxxxxxxxxxx>, Albert Ou <aou@xxxxxxxxxxxxxxxxx>, Alexander Viro <viro@xxxxxxxxxxxxxxxxxx>, Christian Brauner <brauner@xxxxxxxxxx>, Jan Kara <jack@xxxxxxx>, Eric Biederman <ebiederm@xxxxxxxxxxxx>, Chandan Babu R <chandan.babu@xxxxxxxxxx>, "Darrick J. Wong" <djwong@xxxxxxxxxx>, Steven Rostedt <rostedt@xxxxxxxxxxx>, Masami Hiramatsu <mhiramat@xxxxxxxxxx>, Peter Zijlstra <peterz@xxxxxxxxxxxxx>, Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>, Namhyung Kim <namhyung@xxxxxxxxxx>, Alexander Shishkin <alexander.shishkin@xxxxxxxxxxxxxxx>, Ian Rogers <irogers@xxxxxxxxxx>, Adrian Hunter <adrian.hunter@xxxxxxxxx>, Balbir Singh <bsingharora@xxxxxxxxx>, "Naveen N. Rao" <naveen.n.rao@xxxxxxxxxxxxx>, Anil S Keshavamurthy <anil.s.keshavamurthy@xxxxxxxxx>, Petr Mladek <pmladek@xxxxxxxx>, John Ogness <john.ogness@xxxxxxxxxxxxx>, Sergey Senozhatsky <senozhatsky@xxxxxxxxxxxx>, Juri Lelli <juri.lelli@xxxxxxxxxx>, Vincent Guittot <vincent.guittot@xxxxxxxxxx>, Dietmar Eggemann <dietmar.eggemann@xxxxxxx>, Ben Segall <bsegall@xxxxxxxxxx>, Mel Gorman <mgorman@xxxxxxx>, Daniel Bristot de Oliveira <bristot@xxxxxxxxxx>, Valentin Schneider <vschneid@xxxxxxxxxx>, Andy Lutomirski <luto@xxxxxxxxxxxxxx>, Will Drewry <wad@xxxxxxxxxxxx>, John Stultz <jstultz@xxxxxxxxxx>, Stephen Boyd <sboyd@xxxxxxxxxx>, Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx>, "Matthew Wilcox (Oracle)" <willy@xxxxxxxxxxxxx>, Roopa Prabhu <roopa@xxxxxxxxxx>, Nikolay Aleksandrov <razor@xxxxxxxxxxxxx>, Remi Denis-Courmont <courmisch@xxxxxxxxx>, Allison Henderson <allison.henderson@xxxxxxxxxx>, Marcelo Ricardo Leitner <marcelo.leitner@xxxxxxxxx>, Xin Long <lucien.xin@xxxxxxxxx>, Chuck Lever <chuck.lever@xxxxxxxxxx>, Jeff Layton <jlayton@xxxxxxxxxx>, Neil Brown <neilb@xxxxxxx>, Olga Kornievskaia <kolga@xxxxxxxxxx>, Dai Ngo <Dai.Ngo@xxxxxxxxxx>, Tom Talpey <tom@xxxxxxxxxx>, Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>, Anna Schumaker <anna@xxxxxxxxxx>, John Johansen <john.johansen@xxxxxxxxxxxxx>, Paul Moore <paul@xxxxxxxxxxxxxx>, James Morris <jmorris@xxxxxxxxx>, "Serge E. Hallyn" <serge@xxxxxxxxxx>, Alexander Popov <alex.popov@xxxxxxxxx>, linux-hardening@xxxxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx, bpf@xxxxxxxxxxxxxxx, linux-mm@xxxxxxxxx, netdev@xxxxxxxxxxxxxxx, lvs-devel@xxxxxxxxxxxxxxx, netfilter-devel@xxxxxxxxxxxxxxx, coreteam@xxxxxxxxxxxxx, linux-fsdevel@xxxxxxxxxxxxxxx, linux-arm-kernel@xxxxxxxxxxxxxxxxxxx, linux-s390@xxxxxxxxxxxxxxx, linuxppc-dev@xxxxxxxxxxxxxxxx, linux-riscv@xxxxxxxxxxxxxxxxxxx, linux-xfs@xxxxxxxxxxxxxxx, linux-trace-kernel@xxxxxxxxxxxxxxx, linux-perf-users@xxxxxxxxxxxxxxx, kexec@xxxxxxxxxxxxxxxxxxx, bridge@xxxxxxxxxxxxxxx, linux-rdma@xxxxxxxxxxxxxxx, rds-devel@xxxxxxxxxxxxxx, linux-sctp@xxxxxxxxxxxxxxx, linux-nfs@xxxxxxxxxxxxxxx, apparmor@xxxxxxxxxxxxxxxx, linux-security-module@xxxxxxxxxxxxxxx
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Sat, 16 Mar 2024 13:52:48 +1100
On Fri, Mar 15, 2024 at 09:48:09PM +0100, Thomas Weißschuh wrote:
> Adapt the proc_hander function signature to make it clear that handlers
> are not supposed to modify their ctl_table argument.
> 
> This is a prerequisite to moving the static ctl_table structs into
> .rodata.
> By migrating all handlers at once a lengthy transition can be avoided.
> 
> The patch was mostly generated by coccinelle with the following script:
> 
>     @@
>     identifier func, ctl, write, buffer, lenp, ppos;
>     @@
> 
>     int func(
>     - struct ctl_table *ctl,
>     + const struct ctl_table *ctl,
>       int write, void *buffer, size_t *lenp, loff_t *ppos)
>     { ... }

Which seems to have screwed up the formatting of the XFS code...

> diff --git a/fs/xfs/xfs_sysctl.c b/fs/xfs/xfs_sysctl.c
> index a191f6560f98..a3ca192eca79 100644
> --- a/fs/xfs/xfs_sysctl.c
> +++ b/fs/xfs/xfs_sysctl.c
> @@ -10,12 +10,11 @@ static struct ctl_table_header *xfs_table_header;
>  
>  #ifdef CONFIG_PROC_FS
>  STATIC int
> -xfs_stats_clear_proc_handler(
> -     struct ctl_table        *ctl,
> -     int                     write,
> -     void                    *buffer,
> -     size_t                  *lenp,
> -     loff_t                  *ppos)
> +xfs_stats_clear_proc_handler(const struct ctl_table *ctl,
> +                          int                        write,
> +                          void                       *buffer,
> +                          size_t                     *lenp,
> +                          loff_t                     *ppos)

... because this doesn't match any format I've ever seen in the
kernel. The diff for this change shold be just:

@@ -10,7 +10,7 @@ static struct ctl_table_header *xfs_table_header;
 #ifdef CONFIG_PROC_FS
 STATIC int
 xfs_stats_clear_proc_handler(
-       struct ctl_table        *ctl,
+       const struct ctl_table  *ctl,
        int                     write,
        void                    *buffer,
        size_t                  *lenp,

>  {
>       int             ret, *valp = ctl->data;
>  
> @@ -30,12 +29,11 @@ xfs_stats_clear_proc_handler(
>  }
>  
>  STATIC int
> -xfs_panic_mask_proc_handler(
> -     struct ctl_table        *ctl,
> -     int                     write,
> -     void                    *buffer,
> -     size_t                  *lenp,
> -     loff_t                  *ppos)
> +xfs_panic_mask_proc_handler(const struct ctl_table *ctl,
> +                         int                 write,
> +                         void                        *buffer,
> +                         size_t                      *lenp,
> +                         loff_t                      *ppos)
>  {
>       int             ret, *valp = ctl->data;
>  
> @@ -51,12 +49,11 @@ xfs_panic_mask_proc_handler(
>  #endif /* CONFIG_PROC_FS */
>  
>  STATIC int
> -xfs_deprecated_dointvec_minmax(
> -     struct ctl_table        *ctl,
> -     int                     write,
> -     void                    *buffer,
> -     size_t                  *lenp,
> -     loff_t                  *ppos)
> +xfs_deprecated_dointvec_minmax(const struct ctl_table *ctl,
> +                            int                      write,
> +                            void                     *buffer,
> +                            size_t                   *lenp,
> +                            loff_t                   *ppos)
>  {
>       if (write) {
>               printk_ratelimited(KERN_WARNING

And these need fixing as well.

A further quick glance at the patch reveals that there are other
similar screwed up conversions as well.

> diff --git a/kernel/delayacct.c b/kernel/delayacct.c
> index 6f0c358e73d8..513791ef573d 100644
> --- a/kernel/delayacct.c
> +++ b/kernel/delayacct.c
> @@ -44,8 +44,9 @@ void delayacct_init(void)
>  }
>  
>  #ifdef CONFIG_PROC_SYSCTL
> -static int sysctl_delayacct(struct ctl_table *table, int write, void *buffer,
> -                  size_t *lenp, loff_t *ppos)
> +static int sysctl_delayacct(const struct ctl_table *table, int write,
> +                         void *buffer,
> +                         size_t *lenp, loff_t *ppos)
>  {
>       int state = delayacct_on;
>       struct ctl_table t;

Like this.

> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 724e6d7e128f..e2955e0d9f44 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -450,7 +450,8 @@ static void update_perf_cpu_limits(void)
>  
>  static bool perf_rotate_context(struct perf_cpu_pmu_context *cpc);
>  
> -int perf_event_max_sample_rate_handler(struct ctl_table *table, int write,
> +int perf_event_max_sample_rate_handler(const struct ctl_table *table,
> +                                    int write,
>                                      void *buffer, size_t *lenp, loff_t *ppos)
>  {
>       int ret;

And this.

> @@ -474,8 +475,10 @@ int perf_event_max_sample_rate_handler(struct ctl_table 
> *table, int write,
>  
>  int sysctl_perf_cpu_time_max_percent __read_mostly = 
> DEFAULT_CPU_TIME_MAX_PERCENT;
>  
> -int perf_cpu_time_max_percent_handler(struct ctl_table *table, int write,
> -             void *buffer, size_t *lenp, loff_t *ppos)
> +int perf_cpu_time_max_percent_handler(const struct ctl_table *table,
> +                                   int write,
> +                                   void *buffer, size_t *lenp,
> +                                   loff_t *ppos)
>  {
>       int ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos);
>  

And this.

> diff --git a/kernel/hung_task.c b/kernel/hung_task.c
> index b2fc2727d654..003f0f5cb111 100644
> --- a/kernel/hung_task.c
> +++ b/kernel/hung_task.c
> @@ -239,9 +239,10 @@ static long hung_timeout_jiffies(unsigned long 
> last_checked,
>  /*
>   * Process updating of timeout sysctl
>   */
> -static int proc_dohung_task_timeout_secs(struct ctl_table *table, int write,
> -                               void *buffer,
> -                               size_t *lenp, loff_t *ppos)
> +static int proc_dohung_task_timeout_secs(const struct ctl_table *table,
> +                                      int write,
> +                                      void *buffer,
> +                                      size_t *lenp, loff_t *ppos)
>  {
>       int ret;
>  

And this.

> diff --git a/kernel/latencytop.c b/kernel/latencytop.c
> index 781249098cb6..0a5c22b19821 100644
> --- a/kernel/latencytop.c
> +++ b/kernel/latencytop.c
> @@ -65,8 +65,9 @@ static struct latency_record latency_record[MAXLR];
>  int latencytop_enabled;
>  
>  #ifdef CONFIG_SYSCTL
> -static int sysctl_latencytop(struct ctl_table *table, int write, void 
> *buffer,
> -             size_t *lenp, loff_t *ppos)
> +static int sysctl_latencytop(const struct ctl_table *table, int write,
> +                          void *buffer,
> +                          size_t *lenp, loff_t *ppos)
>  {
>       int err;
>  

And this.

I could go on, but there are so many examples of this in the patch
that I think that it needs to be toosed away and regenerated in a
way that doesn't trash the existing function parameter formatting.

-Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx


<Prev in Thread] Current Thread [Next in Thread>