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;
Feedback-ID: i56a14606:Fastmail
User-Agent: Cyrus-JMAP/3.7.0-alpha0-1187-g678636ba0d-fm-20230113.001-g678636ba
Mime-Version: 1.0
Message-Id: <4c4478d3-5b11-450a-9b6d-9e30e52b8f6d@app.fastmail.com>
In-Reply-To: <98636010-fb0b-1771-e81f-cce90740d358@gmail.com>
References: <20230117172825.3170190-1-arnd@kernel.org>
 <98636010-fb0b-1771-e81f-cce90740d358@gmail.com>
Date:   Tue, 17 Jan 2023 21:01:03 +0100
From:   "Arnd Bergmann" <arnd@arndb.de>
To:     "Tariq Toukan" <ttoukan.linux@gmail.com>,
        "Arnd Bergmann" <arnd@kernel.org>,
        "Saeed Mahameed" <saeedm@nvidia.com>,
        "Leon Romanovsky" <leon@kernel.org>
Cc:     "David S . Miller" <davem@davemloft.net>,
        "Eric Dumazet" <edumazet@google.com>,
        "Jakub Kicinski" <kuba@kernel.org>,
        "Paolo Abeni" <pabeni@redhat.com>,
        "Nathan Chancellor" <nathan@kernel.org>,
        "Nick Desaulniers" <ndesaulniers@google.com>,
        "Tom Rix" <trix@redhat.com>, "Tariq Toukan" <tariqt@nvidia.com>,
        "Maxim Mikityanskiy" <maximmi@nvidia.com>,
        "Gal Pressman" <gal@nvidia.com>, "Lama Kayal" <lkayal@nvidia.com>,
        "Moshe Tal" <moshet@nvidia.com>, Netdev <netdev@vger.kernel.org>,
        linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org,
        llvm@lists.linux.dev
Subject: Re: [PATCH] mlx5: reduce stack usage in mlx5_setup_tc
Content-Type: text/plain
Precedence: bulk

On Tue, Jan 17, 2023, at 18:46, Tariq Toukan wrote:
> On 17/01/2023 19:28, Arnd Bergmann wrote:
>> From: Arnd Bergmann <arnd@arndb.de>
>> 
>> Clang warns about excessive stack usage on 32-bit targets:
>> 
>> drivers/net/ethernet/mellanox/mlx5/core/en_main.c:3597:12: error: stack frame size (1184) exceeds limit (1024) in 'mlx5e_setup_tc' [-Werror,-Wframe-larger-than]
>> static int mlx5e_setup_tc(struct net_device *dev, enum tc_setup_type type,
>> 
>> It turns out that both the mlx5e_setup_tc_mqprio_dcb() function and
>> the mlx5e_safe_switch_params() function it calls have a copy of
>> 'struct mlx5e_params' on the stack, and this structure is fairly
>> large.
>> 
>> Use dynamic allocation for both.
>> 

>>   
>> -	err = mlx5e_safe_switch_params(priv, &new_params,
>> +	err = mlx5e_safe_switch_params(priv, new_params,
>>   				       mlx5e_num_channels_changed_ctx, NULL, true);
>>   
>
> Is this change really required, even after new_chs are dynamically 
> allocated?
> As this code pattern of static local new_params repeats in all callers 
> of mlx5e_safe_switch_params, let's not change this one alone if not 
> necessary.

I'm not sure any more now, I actually did the patch a few weeks ago
and only now came across it while going through my backlog.

Generally speaking, the 'new_params' structure on the stack is
too large, but I no longer see warnings after my patch.

> Same for the noinline_for_stack. Are they really needed even after using 
> dynamic allocation for new_chs?

I've reverted both of those hunks now, let me try reproducing the
original randconfig reports and see what still happens.

   Arnd