From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Ahern Subject: Re: [PATCH net-next 0/2] rtnetlink: avoid a warning in rtnl_newlink() Date: Wed, 28 Nov 2018 10:52:53 -0700 Message-ID: References: <20181128063231.12907-1-jakub.kicinski@netronome.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: jiri@resnulli.us, roopa@cumulusnetworks.com, christian.brauner@ubuntu.com, netdev@vger.kernel.org, oss-drivers@netronome.com To: Jakub Kicinski , davem@davemloft.net Return-path: Received: from mail-pf1-f195.google.com ([209.85.210.195]:41894 "EHLO mail-pf1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727867AbeK2EzX (ORCPT ); Wed, 28 Nov 2018 23:55:23 -0500 Received: by mail-pf1-f195.google.com with SMTP id b7so10523895pfi.8 for ; Wed, 28 Nov 2018 09:52:57 -0800 (PST) In-Reply-To: <20181128063231.12907-1-jakub.kicinski@netronome.com> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 11/27/18 11:32 PM, Jakub Kicinski wrote: > Hi! > > I've been hoping for some time that someone more competent would fix > the stack frame size warning in rtnl_newlink(), but looks like I'll > have to take a stab at it myself :) That's the only warning I see > in most of my builds. Somehow my CONFIG_FRAME_WARN got set to 2048 in all of my config files, so I don't see the warning. > > First patch refactors away a somewhat surprising if (1) code block. > Reindentation will most likely cause cherry-pick problems but OTOH > rtnl_newlink() doesn't seem to be changed often, so perhaps we can > risk it in the name of cleaner code? The unnecessary indentation with the if(1) has always annoyed me. I like the cleanup, but strictly speaking if Dave objects patch 2 can be done without it. > > Second patch fixes the warning in simplest possible way. I was > pondering if there is any more clever solution, but I can't see it.. > rtnl_newlink() is quite long with a lot of possible execution paths > so doing memory allocations half way through leads to very ugly > results. Seems like a reasonable first step and in time slave_attr can follow suit. Those are the 2 high runners for stack usage.