Re: [PATCH v2 3/3] mm/mempolicy: introduce MPOL_WEIGHTED_INTERLEAVE for weighted interleaving

Gregory Price [off-list ref] writes:

On Tue, Jan 23, 2024 at 11:02:03AM +0800, Huang, Ying wrote:

quoted

Gregory Price [off-list ref] writes:

quoted

+	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.

quoted

quoted

+		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.

quoted

We can replace "weight_nodes" with "i" and use a "for" loop?

quoted

+	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

quoted

quoted

+		} 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