[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87ede8v554.fsf@yhuang6-desk2.ccr.corp.intel.com>
Date: Tue, 23 Jan 2024 16:13:43 +0800
From: "Huang, Ying" <ying.huang@...el.com>
To: Gregory Price <gregory.price@...verge.com>
Cc: Gregory Price <gourry.memverge@...il.com>, <linux-mm@...ck.org>,
<linux-kernel@...r.kernel.org>, <linux-doc@...r.kernel.org>,
<linux-fsdevel@...r.kernel.org>, <linux-api@...r.kernel.org>,
<corbet@....net>, <akpm@...ux-foundation.org>, <honggyu.kim@...com>,
<rakie.kim@...com>, <hyeongtak.ji@...com>, <mhocko@...nel.org>,
<vtavarespetr@...ron.com>, <jgroves@...ron.com>,
<ravis.opensrc@...ron.com>, <sthanneeru@...ron.com>,
<emirakhur@...ron.com>, <Hasan.Maruf@....com>,
<seungjun.ha@...sung.com>, <hannes@...xchg.org>,
<dan.j.williams@...el.com>, Srinivasulu Thanneeru
<sthanneeru.opensrc@...ron.com>
Subject: Re: [PATCH v2 3/3] mm/mempolicy: introduce MPOL_WEIGHTED_INTERLEAVE
for weighted interleaving
Gregory Price <gregory.price@...verge.com> writes:
> On Tue, Jan 23, 2024 at 11:02:03AM +0800, Huang, Ying wrote:
>> Gregory Price <gourry.memverge@...il.com> writes:
>>
>> > + int prev_node = NUMA_NO_NODE;
>>
>> It appears that we should initialize prev_node with me->il_prev?
>> Details are as below.
>>
>
> yeah good catch, was a rebase error from my tested code, where this is
> the case. patching now.
>
>> > + if (rem_pages <= pol->wil.cur_weight) {
>> > + pol->wil.cur_weight -= rem_pages;
>>
>> If "pol->wil.cur_weight == 0" here, we need to change me->il_prev?
>>
> you are right, and also need to fetch the next cur_weight. Seems I
> missed this specific case in my tests. (had this tested with a single
> node but not 2, so it looked right).
>
> Added to my test suite.
>
>> We can replace "weight_nodes" with "i" and use a "for" loop?
>>
>> > + while (weight_nodes < nnodes) {
>> > + node = next_node_in(prev_node, nodes);
>>
>> IIUC, "node" will not change in the loop, so all "weight" below will be
>> the same value. To keep it simple, I think we can just copy weights
>> from the global iw_table and consider the default value?
>>
>
> another rebase error here from my tested code, this should have been
> node = prev_node;
> while (...)
> node = next_node_in(node, nodes);
>
> I can change it to a for loop as suggested, but for more info on why I
> did it this way, see the chunk below
>
>> > + } else if (!delta_depleted) {
>> > + /* if there was no delta, track last allocated node */
>> > + resume_node = node;
>> > + resume_weight = i < (nnodes - 1) ? weights[i+1] :
>> > + weights[0];
> ^ this line acquires the weight of the *NEXT* node
> another chunk prior to this does the same
> thing. I suppose i can use next_node_in()
> instead and just copy the entire weigh array
> though, if that is preferable.
Yes. I think copy the entire weight array make code logic simpler.
--
Best Regards,
Huang, Ying
Powered by blists - more mailing lists