Re: [PATCH 1/3] mm/mempolicy: implement the sysfs-based weighted_interleave interface

Gregory Price [off-list ref] writes:

On Wed, Jan 17, 2024 at 02:58:08PM +0800, Huang, Ying wrote:

quoted

Gregory Price [off-list ref] writes:

quoted

We haven't had the discussion on how/when this should happen yet,
though, and there's some research to be done.  (i.e. when should DRAM
weights be set? should the entire table be reweighted on hotplug? etc)

Before that, I'm OK to remove default_iw_table and use hard coded "1" as
default weight for now.

Can't quite do that. default_iw_table is a static structure because we
need a reliable default structure not subject to module initialization
failure.  Otherwise we can end up in a situation where iw_table is NULL
during some allocation path if the sysfs structure fails to setup fully.

As the first simplest implementation, we can avoid default_iw_table[].
Becuse it's constant.

There's no good reason to fail allocations just because sysfs failed to
initialization for some reason.  I'll leave default_iw_table with a size
of MAX_NUMNODES for now (nr_node_ids is set up at runtime per your
reference to `setup_nr_node_ids` below, so we can't use it for this).

We allocate memory during module initialization all over the places in
kernel.  I don't think it will cause any issue in practice.  Just some
additional checking for "default_iw_table == NULL".

And, we cannot make it just static, because we need to use RCU to keep
it consistent.  Otherwise, it may be changed during reading.

quoted

quoted

quoted

u8 __rcu *iw_table;

Then, we only need to allocate nr_node_ids elements now.

We need nr_possible_nodes to handle hotplug correctly.

nr_node_ids >= num_possible_nodes().  It's larger than any possible node
ID.

nr_node_ids gets setup at runtime, while the default_iw_table needs
to be a static structure (see above).  I can make default_iw_table
MAX_NUMNODES and subsequent allocations of iw_table be nr_node_ids,
but that makes iw_table a different size at any given time.

This *will* break if "true hotplug" ever shows up and possible_nodes !=
MAX_NUMNODES. But I can write it up if it's a sticking point for you.

I don't think it is an issue for "true hotplug".  Because we can set
nr_node_ids = MAX_NUMNODES even if there is something called "true
hotplug".

Ultimately we're squabbling over, at most, about ~3kb of memory, just
keep that in mind. (I guess if you spawn 3000 threads and each tries a
concurrent write to sysfs/node1, you'd eat 3MB view briefly, but that
is a truly degenerate case and I can think of more denegerate things).

Not just for memory wastage, it's about proper API too.

quoted

When "true node hotplug" becomes reality, we can make nr_node_ids ==
MAX_NUMNODES.  So, it's safe to use it.  Please take a look at
setup_nr_node_ids().

--
Best Regards,
Huang, Ying