linux-kernel - Re: [PATCH] sound: sof: ioc4-topology: avoid extra dai

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <b821f357-8a11-4814-8ae0-e0e8d8cd4afb@app.fastmail.com>
Date: Wed, 07 Aug 2024 10:47:28 +0200
From: "Arnd Bergmann" <arnd@...db.de>
To: "Pierre-Louis Bossart" <pierre-louis.bossart@...ux.intel.com>,
 "Arnd Bergmann" <arnd@...nel.org>, "Liam Girdwood" <lgirdwood@...il.com>,
 "Peter Ujfalusi" <peter.ujfalusi@...ux.intel.com>,
 "Bard Liao" <yung-chuan.liao@...ux.intel.com>,
 "Ranjani Sridharan" <ranjani.sridharan@...ux.intel.com>,
 "Daniel Baluta" <daniel.baluta@....com>,
 "Seppo Ingalsuo" <seppo.ingalsuo@...ux.intel.com>
Cc: "Kai Vehmanen" <kai.vehmanen@...ux.intel.com>,
 "Mark Brown" <broonie@...nel.org>, "Jaroslav Kysela" <perex@...ex.cz>,
 "Takashi Iwai" <tiwai@...e.com>, "Nathan Chancellor" <nathan@...nel.org>,
 "Nick Desaulniers" <ndesaulniers@...gle.com>,
 "Bill Wendling" <morbo@...gle.com>, "Justin Stitt" <justinstitt@...gle.com>,
 "Brent Lu" <brent.lu@...el.com>, sound-open-firmware@...a-project.org,
 linux-sound@...r.kernel.org, linux-kernel@...r.kernel.org,
 llvm@...ts.linux.dev
Subject: Re: [PATCH] sound: sof: ioc4-topology: avoid extra dai_params copy

On Wed, Aug 7, 2024, at 10:37, Pierre-Louis Bossart wrote:
> On 8/7/24 10:02, Arnd Bergmann wrote:
>> From: Arnd Bergmann <arnd@...db.de>
>> 
>> The snd_pcm_hw_params structure is really too large to fit on the
>> stack. Because of the way that clang inlines functions, it ends up
>> twice in one function, which exceeds the 1024 byte limit for 32-bit
>> architecutes:
>> 
>> sound/soc/sof/ipc4-topology.c:1700:1: error: stack frame size (1304) exceeds limit (1024) in 'sof_ipc4_prepare_copier_module' [-Werror,-Wframe-larger-than]
>> sof_ipc4_prepare_copier_module(struct snd_sof_widget *swidget,
>> 
>>>>From what I can tell, this was unintentional, as both
>> sof_ipc4_prepare_dai_copier() and sof_ipc4_prepare_copier_module() make a
>> copy for the same purpose, but copying it once has the exact same effect.
>
> Humm, not sure. I think the copy was intentional so that if one of the
> fixups fails, then the initial hw_params structure is not modified.

It's clear that one of the two copies was intentional, however
the same logic exists in the caller, which passes the copied
ref_params into sof_ipc4_prepare_dai_copier() instead of the live
'fe_params'. If sof_ipc4_prepare_dai_copier() fails, the copy
is discarded by returning the error, and otherwise it gets
passed on to sof_ipc4_init_input_audio_fmt().

> Also not sure why a compiler would think inlining such a large function
> is a good idea?

I think clang prefers to inline large functions in order to better
do larger scale optimizations. gcc starts by inlining small leaf
functions and doesn't get to this one.

Note that the actual problem of having two giant structures on the
stack does not change through inlining, the risk of overflowing
the 8kb thread stack and the cost of maintaining these variables
is the same. The only difference is that clang triggers the warning
because it sees the total stack size where gcc doesn't see it.

    Arnd