linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Rakie Kim <rakie.kim@sk.com>
To: Gregory Price <gourry@gourry.net>
Cc: akpm@linux-foundation.org, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org, linux-cxl@vger.kernel.org,
	joshua.hahnjy@gmail.com, dan.j.williams@intel.com,
	ying.huang@linux.alibaba.com, kernel_team@skhynix.com,
	honggyu.kim@sk.com, yunjeong.mun@sk.com,
	Rakie Kim <rakie.kim@sk.com>
Subject: Re: [PATCH 0/4] mm/mempolicy: Add memory hotplug support in weighted interleave
Date: Mon, 10 Mar 2025 18:03:34 +0900	[thread overview]
Message-ID: <20250310090340.620-1-rakie.kim@sk.com> (raw)
In-Reply-To: <Z8sXFGBYFlG2Z1s4@gourry-fedora-PF4VCD3F>

On Fri, 7 Mar 2025 10:56:04 -0500 Gregory Price <gourry@gourry.net> wrote:

Hi Gregory
Thank you for your response regarding this patch.

> On Fri, Mar 07, 2025 at 03:35:29PM +0900, Rakie Kim wrote:
> > Unnecessary sysfs entries: Nodes without memory were included in sysfs
> > at boot.
> > Missing hotplug support: Nodes that became online after initialization
> > were not recognized, causing incomplete interleave configurations.
> 
> This comment is misleading.  Nodes can "come online" but they are
> absolutely detected during init - as nodes cannot be "hotplugged"
> themselves.  Resources can be added *to* nodes, but nodes themselves
> cannot be added or removed.
> 
> I think what you're trying to say here is:
> 
> 1) The current system creates 1 entry per possible node (explicitly)
> 2) Not all nodes may have memory at all times (memory can be hotplugged)
> 3) It would be nice to let sysfs and weighted interleave omit memoryless
>    nodes until those nodes had memory hotplugged into them.
> 
> > Dynamic sysfs updates for hotplugged nodes  New memory nodes are
> > recognized and integrated via the memory hotplug mechanism.
> > Subsequent patches refine this functionality:
> >
> 
> Just going to reiterate that that there's no such this as a hotplug node
> or "new nodes" - only nodes that have their attributes changed (i.e.
> !N_MEMORY -> N_MEMORY).  The node exists, it may just not have anything
> associated with it.
> 
> Maybe semantic nits, but it matters.  The nodes are present and can be
> operated on before memory comes online, and that has implications for
> users.  Depending on how that hardware comes online, it may or may not
> report its performance data prior to memory hotplug.

I agree with your assessment. The existing comments, as you pointed out,
might indeed be confusing or misleading. I'll make sure this issue is
addressed in version 2.

> 
> If it doesn't report its performance data, then hiding the node before
> it hotplugs memory means a user can't pre-configure the system for when
> the memory is added (which could be used immediately).
> 
> Hiding the node until hotplug also means we have hidden state.  We need
> to capture pre-hotplug reported performance data so that if it comes
> online the auto-calculation of weights is correct.  But if the user has
> already switched from auto to manual mode, then a node suddenly
> appearing will have an unknown state.
> 
> This is why I initially chose to just expose N_POSSIBLE entries in
> sysfs, because the transition state causes hidden information - and that
> felt worse than extra entries.  I suppose I should add some
> documentation somewhere that discusses this issue.
> 
> I think the underlying issue you're dealing with is that the system is
> creating more nodes for you than it should.

I will reply to your next comment on this issue soon.

> 
> ~Gregory

Rakie


      parent reply	other threads:[~2025-03-10  9:03 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-07  6:35 Rakie Kim
2025-03-07  6:35 ` [PATCH 1/4] mm/mempolicy: Support memory hotplug " Rakie Kim
2025-03-07  6:35 ` [PATCH 2/4] mm/mempolicy: Enable sysfs support for " Rakie Kim
2025-03-07 18:19   ` Joshua Hahn
2025-03-10  8:28     ` Rakie Kim
2025-03-07  6:35 ` [PATCH 3/4] mm/mempolicy: Fix duplicate node addition in sysfs for " Rakie Kim
2025-03-07  6:35 ` [PATCH 4/4] mm/mempolicy: Fix memory leaks in mempolicy_sysfs_init() Rakie Kim
2025-03-07 15:23   ` Gregory Price
2025-03-10  8:23     ` Rakie Kim
2025-03-07 15:56 ` [PATCH 0/4] mm/mempolicy: Add memory hotplug support in weighted interleave Gregory Price
2025-03-07 21:55   ` Gregory Price
2025-03-10  9:03     ` Rakie Kim
2025-03-10 14:13       ` Gregory Price
2025-03-12  8:18         ` Rakie Kim
2025-03-10  9:03   ` Rakie Kim [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250310090340.620-1-rakie.kim@sk.com \
    --to=rakie.kim@sk.com \
    --cc=akpm@linux-foundation.org \
    --cc=dan.j.williams@intel.com \
    --cc=gourry@gourry.net \
    --cc=honggyu.kim@sk.com \
    --cc=joshua.hahnjy@gmail.com \
    --cc=kernel_team@skhynix.com \
    --cc=linux-cxl@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=ying.huang@linux.alibaba.com \
    --cc=yunjeong.mun@sk.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox