Gregory Price <gregory.price@xxxxxxxxxxxx> writes: > On Wed, Jan 17, 2024 at 02:58:08PM +0800, Huang, Ying wrote: >> Gregory Price <gregory.price@xxxxxxxxxxxx> writes: >> >> > 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. >> > >> >> 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. >> >> 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
