Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.34 as permitted sender) client-ip=23.128.96.34;
From:   "Huang, Ying" <ying.huang@intel.com>
To:     Gregory Price <gregory.price@memverge.com>
Cc:     Michal Hocko <mhocko@suse.com>, "tj@kernel.org" <tj@kernel.org>,
        "John Groves" <john@jagalactic.com>,
        Gregory Price <gourry.memverge@gmail.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-cxl@vger.kernel.org" <linux-cxl@vger.kernel.org>,
        "linux-mm@kvack.org" <linux-mm@kvack.org>,
        "cgroups@vger.kernel.org" <cgroups@vger.kernel.org>,
        "linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
        "akpm@linux-foundation.org" <akpm@linux-foundation.org>,
        "lizefan.x@bytedance.com" <lizefan.x@bytedance.com>,
        "hannes@cmpxchg.org" <hannes@cmpxchg.org>,
        "corbet@lwn.net" <corbet@lwn.net>,
        "roman.gushchin@linux.dev" <roman.gushchin@linux.dev>,
        "shakeelb@google.com" <shakeelb@google.com>,
        "muchun.song@linux.dev" <muchun.song@linux.dev>,
        "jgroves@micron.com" <jgroves@micron.com>
Subject: Re: [RFC PATCH v4 0/3] memcg weighted interleave mempolicy control
In-Reply-To: <ZW1IdPI11nhKcdZl@memverge.com> (Gregory Price's message of "Sun,
        3 Dec 2023 22:33:08 -0500")
References: <0100018bb64636ef-9daaf0c0-813c-4209-94e4-96ba6854f554-000000@email.amazonses.com>
        <ZU6pR46kiuzPricM@slm.duckdns.org> <ZU6uxSrj75EiXise@memverge.com>
        <ZU7vjsSkGbRLza-K@slm.duckdns.org> <ZU74L9oxWOoTTfpM@memverge.com>
        <ZVNBMW8iJIGDyp0y@tiehlicka> <ZVOXWx8XNJJNC23A@memverge.com>
        <ZVOn2T_Qg_NTKlB2@tiehlicka> <ZVOzMEtDYB4l8qFy@memverge.com>
        <87o7fveeze.fsf@yhuang6-desk2.ccr.corp.intel.com>
        <ZW1IdPI11nhKcdZl@memverge.com>
Date:   Mon, 04 Dec 2023 16:19:02 +0800
Message-ID: <87sf4i2xe1.fsf@yhuang6-desk2.ccr.corp.intel.com>
User-Agent: Gnus/5.13 (Gnus v5.13)
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Gregory Price <gregory.price@memverge.com> writes:

> On Wed, Nov 15, 2023 at 01:56:53PM +0800, Huang, Ying wrote:
>> Gregory Price <gregory.price@memverge.com> writes:
>>=20
>> Because we usually have multiple nodes in one mem-tier, I still think
>> mem-tier-based interface is simpler than node-based.  But, it seems more
>> complex to introduce mem-tier into mempolicy.  Especially if we have
>> per-task weights.  So, I am fine to go with node-based interface.
>>=20
>> > * cgroups: "this doesn't involve dynamic resource accounting /
>> >             enforcement at all" and "these aren't resource
>> > 	    allocations, it's unclear what the hierarchical
>> > 	    relationship mean".
>> >
>> > * node: too global, explore smaller scope first then expand.
>>=20
>> Why is it too global?  I understand that it doesn't cover all possible
>> use cases (although I don't know whether these use cases are practical
>> or not).  But it can provide a reasonable default per-node weight based
>> on available node performance information (such as, HMAT, CDAT, etc.).
>> And, quite some workloads can just use it.  I think this is an useful
>> feature.
>>
>
> Have been sharing notes with more folks.  Michal thinks a global set of
> weights is unintuitive and not useful, and would prefer to see the
> per-task weights first.
>
> Though this may have been in response to adding it as an attribute of
> nodes directly.=20
>
> Another proposal here suggested adding a new sysfs setting
> https://github.com/skhynix/linux/commit/61d2fcc7a880185df186fa2544edcd2f8=
785952a
>
>   $ tree /sys/kernel/mm/interleave_weight/
>   /sys/kernel/mm/interleave_weight/
>   =E2=94=9C=E2=94=80=E2=94=80 enabled [1]
>   =E2=94=9C=E2=94=80=E2=94=80 possible [2]
>   =E2=94=94=E2=94=80=E2=94=80 node
>       =E2=94=9C=E2=94=80=E2=94=80 node0
>       =E2=94=82   =E2=94=94=E2=94=80=E2=94=80 interleave_weight [3]
>       =E2=94=94=E2=94=80=E2=94=80 node1
>           =E2=94=94=E2=94=80=E2=94=80 interleave_weight [3]
>
> (this could be changed to /sys/kernel/mm/mempolicy/...)
>
> I think the internal representation of this can be simplified greatly,
> over what the patch provides now, but maybe this solves the "it doesn't
> belong in these other components" issue.
>
> Answer: Simply leave it as a static global kobject in mempolicy, which
> also deals with many of the issues regarding race conditions.

Although personally I prefer to add interleave weight as an attribute of
nodes.  I understand that some people think it's not appropriate to
place anything node-specific there.  So, some place under /sys/kernel/mm
sounds reasonable too.

> If a user provides weights, use those.  If they do not, use globals.

Yes.  That is the target use case.

> On a cpuset rebind event (container migration, mems_allowed changes),
> manually set weights would have to remain, so in a bad case, the
> weights would be very out of line with the real distribution of memory.
>
> Example: if your nodemask is (0,1,2) and a migration changes it to
> (3,4,5), then unfortunately your weights will likely revert to [1,1,1]
>
> If set with global weights, they could automatically adjust.  It
> would not be perfect, but it would be better than the potential worst
> case above.  If that same migration occurs, the next allocation would
> simply use whatever the target node weights are in the global config.
>
> So if globally you have weights [3,2,1,1,2,3], and you move from
> nodemask (0,1,2) to (3,4,5), your weights change from [3,2,1] to
> [1,2,3].

That is nice.  And I prefer to emphasize the simple use case.  Users
don't need to specify interleave weight always.  Just use
MPOL_WEIGHTED_INTERLEAVE policy, and system will provide reasonable
default weight.

> If the structure is built as a matrix of (cpu_node,mem_nodes),
> the you can also optimize based on the node the task is running on.

The matrix stuff makes the situation complex.  If people do need
something like that, they can just use set_memorypolicy2() with user
specified weights.  I still believe that "make simple stuff simple, and
complex stuff possible".

> That feels very intuitive, deals with many race condition issues, and
> the global setting can actually be implemented without the need for
> set_mempolicy2 at all - which is certainly a bonus.
>
> Would love more thoughts here.  Will have a new RFC with set_mempolicy2,
> mbind2, and MPOL_WEIGHTED_INTERLEAVE soon that demonstrate the above.

Thanks for doing all these!

--
Best Regards,
Huang, Ying