Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 7 Jan 2021 09:41:46 -0800
From:   Jakub Kicinski <kuba@kernel.org>
To:     Alexander Lobakin <alobakin@pm.me>
Cc:     "David S. Miller" <davem@davemloft.net>, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next] net: sysctl: cleanup net_sysctl_init()
Message-ID: <20210107094146.37f20e69@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
In-Reply-To: <20210107091318.2184-1-alobakin@pm.me>
References: <20210106204014.34730-1-alobakin@pm.me>
        <20210106163056.79d75ffa@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
        <20210107091318.2184-1-alobakin@pm.me>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Thu, 07 Jan 2021 09:13:40 +0000 Alexander Lobakin wrote:
> From: Jakub Kicinski <kuba@kernel.org>
> Date: Wed, 6 Jan 2021 16:30:56 -0800
> 
> > On Wed, 06 Jan 2021 20:40:28 +0000 Alexander Lobakin wrote:  
> >> 'net_header' is not used outside of this function, so can be moved
> >> from BSS onto the stack.
> >> Declarations of one-element arrays are discouraged, and there's no
> >> need to store 'empty' in BSS. Simply allocate it from heap at init.  
> >
> > Are you sure? It's passed as an argument to register_sysctl()
> > so it may well need to be valid for the lifetime of net_header.  
> 
> I just moved it from BSS to the heap and allocate it using kzalloc(),
> it's still valid through the lifetime of the kernel.

I see it now, please don't break the normal flow of error handling.

What's the point of moving objects allocated in __init from BSS to 
the heap? If anything I'd think it'll take up more space when allocated
in the heap because of the metadata that needs to be tracked for
dynamic allocations.

The move of net_header makes sense AFAICT, but we may have to annotate
it somehow so kmemleak doesn't complain.