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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux