lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250410075316.538-1-rakie.kim@sk.com>
Date: Thu, 10 Apr 2025 16:53:08 +0900
From: Rakie Kim <rakie.kim@...com>
To: David Hildenbrand <david@...hat.com>
Cc: kernel_team@...ynix.com,
	gourry@...rry.net,
	linux-mm@...ck.org,
	linux-kernel@...r.kernel.org,
	linux-cxl@...r.kernel.org,
	joshua.hahnjy@...il.com,
	dan.j.williams@...el.com,
	ying.huang@...ux.alibaba.com,
	Jonathan.Cameron@...wei.com,
	osalvador@...e.de,
	yunjeong.mun@...com,
	Honggyu Kim <honggyu.kim@...com>,
	Rakie Kim <rakie.kim@...com>,
	akpm@...ux-foundation.org
Subject: Re: [PATCH v7 3/3] mm/mempolicy: Support memory hotplug in weighted interleave

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

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


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ