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

[David Hildenbrand]

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