linux-kernel - Re: [PATCH 4/4] interconnect: qcom: icc-rpm: Remodel how QoS settings are stored

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]

Date: Tue, 26 Mar 2024 22:12:16 +0100
From: Konrad Dybcio <konrad.dybcio@...aro.org>
To: Stephan Gerhold <stephan@...hold.net>
Cc: Bjorn Andersson <andersson@...nel.org>, Georgi Djakov
 <djakov@...nel.org>, Shawn Guo <shawn.guo@...aro.org>,
 Marijn Suijten <marijn.suijten@...ainline.org>,
 linux-arm-msm@...r.kernel.org, linux-pm@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH 4/4] interconnect: qcom: icc-rpm: Remodel how QoS settings
 are stored

On 26.03.2024 9:57 PM, Stephan Gerhold wrote:
> On Tue, Mar 26, 2024 at 08:42:35PM +0100, Konrad Dybcio wrote:
>> Currently, the QoS settings are stored in the node data, even though
>> they're a property of the bus/provider instead. Moreover, they are only
>> needed during the probe step, so they can be easily moved into struct
>> qcom_icc_desc.
>>
>> Reshuffle things around to make it anywhere near readable & comparable
>> with a reference. As a nice bonus, a lot of bytes are shaved off and
>> a few miliseconds are shaved off here and there.
>>
>> As an example, bloat-o-meter reports this on sm6115.o:
>> Total: Before=14799, After=13263, chg -10.38%
>>
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@...aro.org>
>> ---

[...]


> Nitpick: Why is the u16 const when the other (non-pointer) members are
> not? The u16 also feels a bit like overkill here. The struct would have
> exactly the same size with a full unsigned int because of padding.

That's just my brain performing premature (and as you can see invalid)
optimizations.. I can change it to u32

> 
> Alternatively, you could consider using an empty last entry as sentinel
> instead of adding the count (i.e. with NOC_QOS_MODE_INVALID = 0). Not
> sure what is cleaner here.

Nah, let's keep the counter

> 
> I haven't looked closely at the actual conversion of the definitions in
> the drivers. What is the chance that you made an accidental mistake in
> there? Or was it scripted? :D

By hand. After this change, it should hopefully be very easy and
convenient to check against downstream.

Konrad