Re: [PATCH] mm/mempolicy: recompute automatic interleave weights on memory hotplug

["David Hildenbrand (Arm)" <david@kernel.org>]

On 10/26/26 16:47, Gregory Price wrote:
> On Mon, Oct 26, 2026 at 12:19:21PM +0100, David Hildenbrand (Arm) wrote:
>>> +/* Recompute the weights from node_bw_table, unless they were set manually */
>>> +static int wi_state_refresh(void)
>>> +{
>>> +	struct weighted_interleave_state *new_wi_state, *old_wi_state;
>>> +	int i;
>>> +
>>> +	new_wi_state = kmalloc_flex(*new_wi_state, iw_table, nr_node_ids);
>>> +	if (!new_wi_state)
>>> +		return -ENOMEM;
>>> +	new_wi_state->mode_auto = true;
>>> +	for (i = 0; i < nr_node_ids; i++)
>>> +		new_wi_state->iw_table[i] = 1;
>>> +
>>> +	mutex_lock(&wi_state_lock);
>>> +	old_wi_state = rcu_dereference_protected(wi_state,
>>> +					lockdep_is_held(&wi_state_lock));
>>> +	/* NULL wi_state assumes auto=true */
>>> +	if (!node_bw_table || (old_wi_state && !old_wi_state->mode_auto)) {
>>
>>
>> I was confused about the node_bw_table check at first. Maybe
>>
>> /*
>>  * We can get hotplug events before any BW data was recorded.
>>  * ...
>>  */
>>
> 
> I went back and forth on whether to do the mutex in or outside this
> function, but ultimately decided inside is just better structurally.
> 
> But yes, this comment seems warranted in this case.
> 
>>> +	/*
>>> +	 * HMAT and CXL report a node's bandwidth only when it first gains
>>> +	 * memory, so recompute the weights whenever N_MEMORY changes.
>>> +	 */
>>> +	wi_state_refresh();
>>
>> If the function fails, should we at least pr_warn_once() ?
>>
> 
> The only failure state is -ENOMEM, wasn't sure that was worth warning
> on.  I actually debated whether to just make this a void function.
> 
> I don't have a strong feeling here, but happy to add a warn if you
> prefer it.

I'd assume we'd at least want to tell the world somehow that something is now
imprecise?

Or is not updating that harmless?

-- 
Cheers,

David