linux-kernel - Re: [PATCH v3] Weighted interleave auto-tuning

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: <20250122155935.1282897-1-joshua.hahnjy@gmail.com>
Date: Wed, 22 Jan 2025 07:59:34 -0800
From: Joshua Hahn <joshua.hahnjy@...il.com>
To: "Huang, Ying" <ying.huang@...ux.alibaba.com>
Cc: Gregory Price <gourry@...rry.net>,
	hyeonggon.yoo@...com,
	rafael@...nel.org,
	lenb@...nel.org,
	gregkh@...uxfoundation.org,
	akpm@...ux-foundation.org,
	honggyu.kim@...com,
	rakie.kim@...com,
	dan.j.williams@...el.com,
	Jonathan.Cameron@...wei.com,
	dave.jiang@...el.com,
	horen.chuang@...ux.dev,
	hannes@...xchg.org,
	linux-kernel@...r.kernel.org,
	linux-acpi@...r.kernel.org,
	linux-mm@...ck.org,
	kernel-team@...a.com
Subject: Re: [PATCH v3] Weighted interleave auto-tuning

On Wed, 22 Jan 2025 09:37:20 +0800 "Huang, Ying" <ying.huang@...ux.alibaba.com> wrote:

Hi Gregory and Ying, thank you both for your insights!

[...snip...]

> >> I still prefer to use 2 iw_table, one is for default, the other is for
> >> manual.  The default one will be used if the manual one is NULL.  Both
> >> are protected by RCU.  The default one can be updated upon hotplug
> >> blindly.  This makes the whole model easier to be understood IMHO.
> > `cat auto node0 node1` -> `true 5 1`
> > and you do
> > echo 0 > auto

I think that when initially developing this patch, this was the intent
that I had as well (in the v1 of this RFC patch, there was an iw_table
and a separate default_iw_table). However, I think that the ideas of
having a "default" and "manual" table made less sense over time, given
that they behaved more like a "default" and "visible" table instead.
That is, the visible layer is directly manipulable by the user, but
does not necessarily only contain manually-set values; rather, most of
the time, it probably still has a lot of auto-generated weights.

I think that this analysis runs the risk of being a bit too semantically
nit-picky, but as I'll explain below, I think both the 1-layer approach
that I implemented in this RFC and the expected 2-layer behavior that
you outline below are essentially the same, functionally. In other
words, I think we agree on what the expected behavior should be : -)
We just have to agree on what presentation would make the most sense
for the user.

> > what should a subsequent `cat auto node0 node1` output?
> >
> > `false 5 1`
> > or
> > `false 1 1`
> 
> IMO, it should be
> 
> `false 5 1`
> 
> That is, we copy auto-generated weights to manual weights atomically and
> use it.
> 
> > Then lets say we do
> > echo 7 > node0
> 
> Now, `cat auto node0 node1` outputs,
> 
> `false 7 1`
> 
> That is, we delete manual weights and use the auto-generated ones.
> 
> > what should
> > echo true > auto
> > result in?
> >
> > `true 5 1`
> > or
> > `true 7 1`
> 
> It should be
> 
> `true 5 1`

I see, so I think we actually agree on what the behavior for this is.
Then there is no real "hidden state", it's either just using the
default weights, or turning that off and being able to edit the
states.

> > The current code makes sure that when you switch modes from auto
> > to manual, it inherits the current state - instead of there being
> > some hidden state that suddenly takes precedence.
> 
> I think that we can do that with two weight arrays.
> 
> > So I prefer to just have one IW array and no hidden state.
> 
> Then, when we switch from manual to auto mode, where to find
> auto-generated weights?  Re-calculate them?

Even in manual mode, incoming bandwidth data is continuously stored.
This way, when a user does decide to switch back to auto mode later,
the system doesn't have to retrieve the bandwidth data all over again.
As for the auto-generated weights, they are re-calculated based solely
on the bandwidth data available. (I will note that re-calculating
the weights are very quick, see reduce_interleave_weights)

Based on your description of the expected behavior, everything you
listed out is actually what currently happens in the one-layer system.
Switching from auto --> manual inherits the auto-generated weights, and
switching from manual --> auto wipes all previous user-stored data.

At this point, I think that I am happy with either option. I wrote and
re-wrote this a bunch of times, and came to the conclusion that now
that we agree on the behavior of the interface, I have no strong
opinion on whether we have a "hidden" default layer or a phantom
default layer that is just generated every time a user needs it : -)

Please let me know if I missed anything as well! Thank you all for
your continued feedback and interest! Have a great day,
Joshua