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

[Rakie Kim]

On Wed, 9 Apr 2025 13:52:28 +0200 David Hildenbrand <david@xxxxxxxxxx> wrote:
> 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@xxxxxx>
> >>> Signed-off-by: Honggyu Kim <honggyu.kim@xxxxxx>
> >>> Signed-off-by: Yunjeong Mun <yunjeong.mun@xxxxxx>
> >>
> >>
> >> 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@xxxxxx>
> >     Signed-off-by: Honggyu Kim <honggyu.kim@xxxxxx>
> > 
> > For Yunjeong, the following can be added.
> > 
> >     Tested-by: Yunjeong Mun <yunjeong.mun@xxxxxx>
> 
> 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
> 

Hi David,

Thank you for reviewing this patch series and providing your Acked-by tag.
As you pointed out, I agree that the Signed-off-by tags in this patch
series are not clearly aligned with the actual contributions.

Coincidentally, Dan Williams has requested an additional fix for Patch 1
in this series. Therefore, I am planning to prepare a new version, v8.

In that version, I will reorganize the Signed-off-by tags as you suggested
to accurately reflect the authorship and contributions.

Thank you again for your guidance.

Rakie