Date:   Mon, 13 Mar 2023 14:16:03 +0100
From:   Vasily Gorbik <gor@linux.ibm.com>
To:     Luis Chamberlain <mcgrof@kernel.org>
Cc:     hca@linux.ibm.com, agordeev@linux.ibm.com,
        borntraeger@linux.ibm.com, svens@linux.ibm.com,
        linux-s390@vger.kernel.org, sudipm.mukherjee@gmail.com,
        ebiederm@xmission.com, keescook@chromium.org, yzaikin@google.com,
        j.granados@samsung.com, patches@lists.linux.dev,
        linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 0/6] s390: simplify sysctl registration
Message-ID: <your-ad-here.call-01678713363-ext-0026@work.hours>
References: <20230310234525.3986352-1-mcgrof@kernel.org>
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20230310234525.3986352-1-mcgrof@kernel.org>
MIME-Version: 1.0
Precedence: bulk

On Fri, Mar 10, 2023 at 03:45:19PM -0800, Luis Chamberlain wrote:
> s390 is the last architecture and one of the last users of
> register_sysctl_table(). It was last becuase it had one use case
> with dynamic memory allocation and it just required a bit more
> thought.
> 
> This is all being done to help reduce code and avoid usage of API
> calls for sysctl registration that can incur recusion. The recursion
> only happens when you have subdirectories with entries and s390 does
> not have any of that so either way recursion is avoided. Long term
> though we can do away with just removing register_sysctl_table()
> and then using ARRAY_SIZE() and save us tons of memory all over the
> place by not having to add an extra empty entry all over.
> 
> Hopefully that commit log suffices for the dynamic allocation
> conversion, but I would really like someone to test it as I haven't
> tested a single patch, I'm super guiltly to accept I've just waited for
> patches to finish compile testing and that's not over yet.
> 
> Anyway the changes other than the dynamic allocation one are pretty
> trivial. That one could use some good review.
> 
> With all this out of the way we have just one stupid last user of
> register_sysctl_table(): drivers/parport/procfs.c
> 
> That one is also dynamic. Hopefully the maintainer will be motivated
> to do that conversion with all the examples out there now and this
> having a complex one.
> 
> For more information refer to the new docs:
> 
> https://lore.kernel.org/all/20230310223947.3917711-1-mcgrof@kernel.org/T/#u     
>  
> Luis Chamberlain (6):
>   s390: simplify one-level sysctl registration for topology_ctl_table
>   s390: simplify one-level syctl registration for s390dbf_table
>   s390: simplify one-level sysctl registration for appldata_table
>   s390: simplify one level sysctl registration for cmm_table
>   s390: simplify one-level sysctl registration for page_table_sysctl
>   s390: simplify dynamic sysctl registration for appldata_register_ops
> 
>  arch/s390/appldata/appldata_base.c | 30 ++++++++----------------------
>  arch/s390/kernel/debug.c           | 12 +-----------
>  arch/s390/kernel/topology.c        | 12 +-----------
>  arch/s390/mm/cmm.c                 | 12 +-----------
>  arch/s390/mm/pgalloc.c             | 12 +-----------
>  5 files changed, 12 insertions(+), 66 deletions(-)

I've added my
Reviewed-by: Vasily Gorbik <gor@linux.ibm.com>
for the entire patch series.

And applied with the fixup for last change (see corresponding reply).
Thank you!