All of lore.kernel.org
 help / color / mirror / Atom feed
From: SeongJae Park <sj@kernel.org>
To: Bijan Tabatabai <bijan311@gmail.com>
Cc: SeongJae Park <sj@kernel.org>,
	damon@lists.linux.dev, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org, akpm@linux-foundation.org,
	david@redhat.com, ziy@nvidia.com, matthew.brost@intel.com,
	joshua.hahnjy@gmail.com, rakie.kim@sk.com, byungchul@sk.com,
	gourry@gourry.net, ying.huang@linux.alibaba.com,
	apopple@nvidia.com, bijantabatab@micron.com,
	venkataravis@micron.com, emirakhur@micron.com,
	ajayjoshi@micron.com, vtavarespetr@micron.com
Subject: Re: [RFC PATCH v2 0/2] mm/damon/paddr: Allow interleaving in migrate_{hot,cold} actions
Date: Fri, 20 Jun 2025 13:21:55 -0700	[thread overview]
Message-ID: <20250620202155.98021-1-sj@kernel.org> (raw)
In-Reply-To: <20250620180458.5041-1-bijan311@gmail.com>

Hi Bijan,

On Fri, 20 Jun 2025 13:04:56 -0500 Bijan Tabatabai <bijan311@gmail.com> wrote:

[...]
> This patch set adds the mechanism for dynamically changing how application
> data is interleaved across nodes while leaving the policy of what the
> interleave weights should be to userspace. It does this by modifying the
> migrate_{hot,cold} DAMOS actions to allow passing in a list of migration
> targets to their target_nid parameter. When this is done, the
> migrate_{hot,cold} actions will migrate pages between the specified nodes
> using the global interleave weights found at
> /sys/kernel/mm/mempolicy/weighted_interleave/node<N>. This functionality
> can be used to dynamically adjust how pages are interleaved by changing the
> global weights. When only a single migration target is passed to
> target_nid, the migrate_{hot,cold} actions will act the same as before.

This means users are required to manipulate two interfaces.  DAMON sysfs for
target nodes, and weighted_interleave sysfs for weights.  I don't think this
coupling is very ideal.

Off the opt of my head, I concern if users could mistakenly forget updating one
of those, since the requirement is not very clear.  I think the interface
should clearly explain that.  For example, writing a special keywords, say,
"use_interleave_weights" to target_nid parameter sysfs file.  But, even in the
case, users who update weighted_interleave might foget updating target nodes on
DAMON interface.

I think letting DAMOS_MIGRATE_{HOT,COLD} to use all nodes as migration target
when the special keyword is given is one of better options.  This is what I
suggested to the previous version of this patch series.  But now I think it
would be better if we could just remove this coupling.

I understand a sort of this coupling is inevitable if the kernel should make
the connection between DAMON and weighted interleaving itself, without
user-space help.  But now I think we could get user-space help, according to
below.  Please keep reading.

[...]
> As a toy example, imagine some application that uses 75% of the local
> bandwidth. Assuming sufficient capacity, when running alone, we want to
> keep that application's data in local memory. However, if a second
> instance of that application begins, using the same amount of bandwidth,
> it would be best to interleave the data of both processes to alleviate the
> bandwidth pressure from the local node. Likewise, when one of the processes
> ends, the data should be moves back to local memory.
> 
> We imagine there would be a userspace application that would monitor system
> performance characteristics, such as bandwidth utilization or memory access
> latency, and uses that information to tune the interleave weights. Others
> seem to have come to a similar conclusion in previous discussions [3].
> We are currently working on a userspace program that does this, but it is
> not quite ready to be published yet.

So, at least in this toy example, we have user-space control.  Then, I think we
could decouple DAMON and weighted interleaving, and ask the usr-space tool to
be the connection between those.  That is, extend DAMOS_MIGRATE_{HOT,COLD} to
let users specify migration target nodes and their weights.  And ask the
user-space tool to periodically read weighted interleaving parameters that
could be auto-tuned, and update DAMOS_MIGRATE_{HOT,COLD} parameters
accordingly.  Actually the user-space tool on this example is making the
weights by itself, so this should be easy work to do?

Also, even for general use case, I think such user-space intervention is not
too much request.  Please let me know if I'm wrong.

> 
> We believe DAMON is the correct venue for the interleaving mechanism for a
> few reasons. First, we noticed that we don't ahve to migrate all of the
> application's pages to improve performance. we just need to migrate the
> frequently accessed pages. DAMON's existing hotness traching is very useful
> for this. Second, DAMON's quota system can be used to ensure we are not
> using too much bandwidth for migrations. Finally, as Ying pointed out [4],
> a complete solution must also handle when a memory node is at capacity. The
> existing migrate_cold action can be used in conjunction with the
> functionality added in this patch set to provide that complete solution.

These make perfect sense to me.  Thank you for adding this great summary.

> 
> Functionality Test
> ==================
[...]
> Performance Test
> ================
[...]
> Updating the interleave weights, and having DAMON migrate the workload
> data according to the weights resulted in an approximately 25% speedup.

Awesome.  Thank you for conducting this great tests and sharing the results!

> 
> Questions for Reviewers
> =======================
> 1. Are you happy with the changes to the DAMON sysfs interface?

I'm happy with it for RFC level implementation.  And in my opinion, you now
proved this is a good idea.  For next steps toward mainline landing, I'd like
to suggest below interface change.

Let's allow users specify DAMOS_MIGRATE_{HOT,COLD} target nodes and weights
using only DAMON interface.  And let the user-space tool do the synchronization
with weighted interleaving or other required works.

This may require writing not small amount of code, especially for DAMON sysfs
interface.  I think it is doable, though.  If you don't mind, I'd like to
quickly make a prototype and share with you.

What do you think?

> 2. Setting an interleave weight to 0 is currently not allowed. This makes
>    sense when the weights are only used for allocation. Does it make sense
>    to allow 0 weights now?

I have no opinion, and would like to let mempolicy folks make voices.  But if
we go on the decoupling approach as I suggested above, we can do this
discussion in a separate thread :)

[...]
> Revision History
> ================
> Changes from v1
> (https://lore.kernel.org/linux-mm/20250612181330.31236-1-bijan311@gmail.com/)
> - Reuse migrate_{hot,cold} actions instead of creating a new action
> - Remove vaddr implementation
> - Remove most of the use of mempolicy, instead duplicate the interleave
>   logic and access interleave weights directly
> - Write more about the use case in the cover letter
> - Write about why DAMON was used for this in the cover letter
> - Add correctness test to the cover letter
> - Add performance test

Again, thank you for revisioning.  Please bear in mind with me at next steps.
I believe this work is very promising.


Thanks,
SJ

[...]

  parent reply	other threads:[~2025-06-20 20:21 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-20 18:04 [RFC PATCH v2 0/2] mm/damon/paddr: Allow interleaving in migrate_{hot,cold} actions Bijan Tabatabai
2025-06-20 18:04 ` [RFC PATCH v2 1/2] mm/mempolicy: Expose get_il_weight() to MM Bijan Tabatabai
2025-06-23 19:06   ` Gregory Price
2025-06-23 19:14   ` David Hildenbrand
2025-06-23 19:38     ` Gregory Price
2025-06-24 10:58   ` Huang, Ying
2025-06-20 18:04 ` [RFC PATCH v2 2/2] mm/damon/paddr: Allow multiple migrate targets Bijan Tabatabai
2025-06-21 18:02   ` SeongJae Park
2025-06-21 18:11     ` SeongJae Park
2025-06-23 14:08       ` Joshua Hahn
2025-06-23 16:50         ` SeongJae Park
2025-06-23 14:27       ` Bijan Tabatabai
2025-06-23 16:52         ` SeongJae Park
2025-06-23 14:16     ` Bijan Tabatabai
2025-06-23 17:52       ` SeongJae Park
2025-06-23 23:15         ` Bijan Tabatabai
2025-06-24  0:34           ` SeongJae Park
2025-06-24 16:01             ` Bijan Tabatabai
2025-06-24 22:33               ` SeongJae Park
2025-06-20 20:21 ` SeongJae Park [this message]
2025-06-20 21:47   ` [RFC PATCH v2 0/2] mm/damon/paddr: Allow interleaving in migrate_{hot,cold} actions Bijan Tabatabai
2025-06-20 23:13     ` SeongJae Park
2025-06-21 17:36       ` SeongJae Park
2025-06-23 14:39         ` Bijan Tabatabai
2025-06-23 16:32           ` SeongJae Park
2025-06-23 19:28   ` Gregory Price
2025-06-23 23:21     ` Bijan Tabatabai
2025-06-26 19:13       ` Gregory Price
2025-06-23 13:45 ` Joshua Hahn
2025-06-23 14:57   ` Bijan Tabatabai

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=20250620202155.98021-1-sj@kernel.org \
    --to=sj@kernel.org \
    --cc=ajayjoshi@micron.com \
    --cc=akpm@linux-foundation.org \
    --cc=apopple@nvidia.com \
    --cc=bijan311@gmail.com \
    --cc=bijantabatab@micron.com \
    --cc=byungchul@sk.com \
    --cc=damon@lists.linux.dev \
    --cc=david@redhat.com \
    --cc=emirakhur@micron.com \
    --cc=gourry@gourry.net \
    --cc=joshua.hahnjy@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=matthew.brost@intel.com \
    --cc=rakie.kim@sk.com \
    --cc=venkataravis@micron.com \
    --cc=vtavarespetr@micron.com \
    --cc=ying.huang@linux.alibaba.com \
    --cc=ziy@nvidia.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.