Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 28 Feb 2022 07:09:00 -0800
From:   Luis Chamberlain <mcgrof@kernel.org>
To:     tangmeng <tangmeng@uniontech.com>
Cc:     Matthew Wilcox <willy@infradead.org>,
        "Eric W. Biederman" <ebiederm@xmission.com>, keescook@chromium.org,
        yzaikin@google.com, nixiaoming@huawei.com, nizhen@uniontech.com,
        zhanglianjie@uniontech.com, sujiaxun@uniontech.com,
        linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH] fs/proc: optimize exactly register one ctl_table
Message-ID: <YhzljEnznlZjw53K@bombadil.infradead.org>
References: <20220224093201.12440-1-tangmeng@uniontech.com>
 <YhvsgZesRNQmfkIB@bombadil.infradead.org>
 <f8820396-ff02-7589-a2f0-be542fbc2c3d@uniontech.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <f8820396-ff02-7589-a2f0-be542fbc2c3d@uniontech.com>
Sender: Luis Chamberlain <mcgrof@infradead.org>
Precedence: bulk

On Mon, Feb 28, 2022 at 10:42:19AM +0800, tangmeng wrote:
> 
> 
> On 2022/2/28 05:26, Luis Chamberlain wrote:
> 
> > 
> > This effort is trying to save space. But now you are adding a new bool
> > for every single struct ctl_table.... So doesn't the math work out
> > against us if you do a build size comparison?
> > 
> Currently,

You mean after your patch.

> the definition of the ctl_table structure and the size of the
> members are as follows.
> /* In 64-bit system*/
> struct ctl_table {
...
>         bool register_one;               /* 1 bytes */
...
> } __randomize_layout;
> 
> Before 

Before it the bool was not there. How can it be you are not increasing
the size?

> > Can you just instead add a new helper which deals with one entry?
> > Perhaps then make the other caller which loops use that? That way
> > we don't bloat the kernel with an extra bool per ctl_table?
> > 
> I have considered add a new helper which deals with one entry. But
> considered that the code will be similar to array,

That's fine, if we have tons of these.

> > Or can you add a new parameter which specififes the size of the array?
> > 
> When I considered add a new parameter which specififes the size of the
> array. I have encountered the following difficulties.
> 
> The current status is that during the ctl_table registration process, the
> method of traversing the table is implemented by a movement of the pointer
> entry pointing to the struct ctl_table. When entry->procname is empty, it
> considers table traversal.
> 
> This leads to that when the ctl_tables have child tables in the table, it is
> not possible to get the child table's size by ARRAY_SIZE(*entry), so
> transmitting the Child Table Size becomes very difficult.

I see.

A simple routine for dealing with single entries might be best then.
And while at it, see if you can add a DECLARE_SYSCTL_SINGLE or something
which will wrap up all the ugly stuff.

  Luis