Re: [PATCH v7 3/3] mm/mempolicy: Support memory hotplug in weighted interleave

[David Hildenbrand <david@redhat.com>]

On 09.04.25 13:39, Honggyu Kim wrote:
> Hi David,
> 
> On 4/9/2025 6:05 PM, David Hildenbrand wrote:
>> On 08.04.25 09:32, Rakie Kim wrote:
>>> The weighted interleave policy distributes page allocations across multiple
>>> NUMA nodes based on their performance weight, thereby improving memory
>>> bandwidth utilization. The weight values for each node are configured
>>> through sysfs.
>>>
>>> Previously, sysfs entries for configuring weighted interleave were created
>>> for all possible nodes (N_POSSIBLE) at initialization, including nodes that
>>> might not have memory. However, not all nodes in N_POSSIBLE are usable at
>>> runtime, as some may remain memoryless or offline.
>>> This led to sysfs entries being created for unusable nodes, causing
>>> potential misconfiguration issues.
>>>
>>> To address this issue, this patch modifies the sysfs creation logic to:
>>> 1) Limit sysfs entries to nodes that are online and have memory, avoiding
>>>      the creation of sysfs entries for nodes that cannot be used.
>>> 2) Support memory hotplug by dynamically adding and removing sysfs entries
>>>      based on whether a node transitions into or out of the N_MEMORY state.
>>>
>>> Additionally, the patch ensures that sysfs attributes are properly managed
>>> when nodes go offline, preventing stale or redundant entries from persisting
>>> in the system.
>>>
>>> By making these changes, the weighted interleave policy now manages its
>>> sysfs entries more efficiently, ensuring that only relevant nodes are
>>> considered for interleaving, and dynamically adapting to memory hotplug
>>> events.
>>>
>>> Signed-off-by: Rakie Kim <rakie.kim@sk.com>
>>> Signed-off-by: Honggyu Kim <honggyu.kim@sk.com>
>>> Signed-off-by: Yunjeong Mun <yunjeong.mun@sk.com>
>>
>>
>> Why are the other SOF in there? Are there Co-developed-by missing?
> 
> I initially found the problem and fixed it with my internal implementation but
> Rakie also had his idea so he started working on it.  His initial implementation
> has almost been similar to mine.
> 
> I thought Signed-off-by is a way to express the patch series contains our
> contribution, but if you think it's unusual, then I can add this.

Please see Documentation/process/submitting-patches.rst, and note that these
are not "patch delivery" SOB.

"
The Signed-off-by: tag indicates that the signer was involved in the
development of the patch, or that he/she was in the patch's delivery path.
"

and

"
Co-developed-by: states that the patch was co-created by multiple developers;
it is used to give attribution to co-authors (in addition to the author
attributed by the From: tag) when several people work on a single patch.  Since
Co-developed-by: denotes authorship, every Co-developed-by: must be immediately
followed by a Signed-off-by: of the associated co-author.  Standard sign-off
procedure applies, i.e. the ordering of Signed-off-by: tags should reflect the
chronological history of the patch insofar as possible, regardless of whether
the author is attributed via From: or Co-developed-by:.  Notably, the last
Signed-off-by: must always be that of the developer submitting the patch.
"

The SOB order here is also not correct.

> 
>     Co-developed-by: Honggyu Kim <honggyu.kim@sk.com>
>     Signed-off-by: Honggyu Kim <honggyu.kim@sk.com>
> 
> For Yunjeong, the following can be added.
> 
>     Tested-by: Yunjeong Mun <yunjeong.mun@sk.com>

That is probably the right thing to do if contribution was focused on testing.

> 
> However, this patch series is already in Andrew's mm-new so I don't want to
> bother him again unless we need to update this patches for other reasons.

mm-new is exactly for these kind of things. We can ask Andrew to fix it up.

-- 
Cheers,

David / dhildenb