linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: "Huang, Ying" <ying.huang@linux.alibaba.com>
To: Joshua Hahn <joshua.hahnjy@gmail.com>
Cc: Gregory Price <gourry@gourry.net>,
	 hyeonggon.yoo@sk.com, rafael@kernel.org,  lenb@kernel.org,
	 gregkh@linuxfoundation.org, akpm@linux-foundation.org,
	 honggyu.kim@sk.com,  rakie.kim@sk.com, dan.j.williams@intel.com,
	 Jonathan.Cameron@huawei.com, dave.jiang@intel.com,
	 horen.chuang@linux.dev,  hannes@cmpxchg.org,
	linux-kernel@vger.kernel.org,  linux-acpi@vger.kernel.org,
	linux-mm@kvack.org,  kernel-team@meta.com
Subject: Re: [PATCH v3] Weighted interleave auto-tuning
Date: Thu, 23 Jan 2025 11:32:37 +0800	[thread overview]
Message-ID: <8734hakxwq.fsf@DESKTOP-5N7EMDA> (raw)
In-Reply-To: <20250122155935.1282897-1-joshua.hahnjy@gmail.com> (Joshua Hahn's message of "Wed, 22 Jan 2025 07:59:34 -0800")

Joshua Hahn <joshua.hahnjy@gmail.com> writes:

> On Wed, 22 Jan 2025 09:37:20 +0800 "Huang, Ying" <ying.huang@linux.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.

This sounds good to me.

We still need to finalize the interface, 'mode' or 'auto'.  Personally,
I prefer 'auto', because it's simpler.

>> > 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,

I see.  You cache the nodes bandwidth instead of default weights.  That
works.  I am fine with either way too.

---
Best Regards,
Huang, Ying


  parent reply	other threads:[~2025-01-23  3:33 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-15 18:58 Joshua Hahn
2025-01-20  4:47 ` Hyeonggon Yoo
2025-01-21 21:24   ` Joshua Hahn
2025-01-21 11:17 ` Huang, Ying
2025-01-21 11:27   ` Honggyu Kim
2025-01-21 20:02     ` Gregory Price
2025-01-22  1:24     ` Huang, Ying
2025-02-05  5:34       ` Honggyu Kim
2025-01-21 19:56   ` Gregory Price
2025-01-22  1:37     ` Huang, Ying
2025-01-22 15:59       ` Joshua Hahn
2025-01-22 16:53         ` Gregory Price
2025-01-23  3:32         ` Huang, Ying [this message]
2025-01-24  5:58 ` Matthew Wilcox
2025-01-24 15:48   ` Gregory Price
2025-01-24 15:53   ` Gregory Price

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8734hakxwq.fsf@DESKTOP-5N7EMDA \
    --to=ying.huang@linux.alibaba.com \
    --cc=Jonathan.Cameron@huawei.com \
    --cc=akpm@linux-foundation.org \
    --cc=dan.j.williams@intel.com \
    --cc=dave.jiang@intel.com \
    --cc=gourry@gourry.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=hannes@cmpxchg.org \
    --cc=honggyu.kim@sk.com \
    --cc=horen.chuang@linux.dev \
    --cc=hyeonggon.yoo@sk.com \
    --cc=joshua.hahnjy@gmail.com \
    --cc=kernel-team@meta.com \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=rafael@kernel.org \
    --cc=rakie.kim@sk.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox