Message-ID: <Z5PMG_F9wJXCb8ri@gourry-fedora-PF4VCD3F>
Date: Fri, 24 Jan 2025 12:21:31 -0500
From: Gregory Price <gourry@...rry.net>
To: SeongJae Park <sj@...nel.org>
Cc: lsf-pc@...ts.linux-foundation.org, damon@...ts.linux.dev,
	linux-mm@...ck.org, linux-kernel@...r.kernel.org,
	kernel-team@...a.com, Raghavendra K T <raghavendra.kt@....com>,
	Yuanchu Xie <yuanchu@...gle.com>,
	Jonathan Cameron <Jonathan.Cameron@...wei.com>,
	Kaiyang Zhao <kaiyang2@...cmu.edu>,
	Jiaming Yan <jiamingy@...zon.com>, Honggyu Kim <honggyu.kim@...com>
Subject: Re: [LSF/MM/BPF TOPIC] DAMON Requirements for Access-aware MM of
 Future

On Thu, Jan 23, 2025 at 06:11:53PM -0800, SeongJae Park wrote:
> Hello Gregory,
> 
> > 1) The cost of checking promotion candidacy can be problematic
> > 
> >    In my microbenchmark in the last RFC version, I showed that while
> >    the performance upside (~22-25%) is substantial, there was a
> >    non-trivial cost associated with injecting even a single global
> >    boolean check in the file_read() path.  This was unexpected.
> > 
> >    I can probably optimize the disabled case with a likely() clause,
> >    but I did not expect such sensitivity.  This tells me injecting
> >    an unconditional call into DAMON may be too much overhead. 
> 
> I cannot agree more with you about the point that the mechanism for finding the
> promotion/demotion (and any access-aware system operation) candidates should
> induce only modest or at least controllable overhead.  Actually it was the one
> of biggest motivations of DAMON design, and I haven't imagined adding
> unconditional calls to DAMON here.
> 
> Nonetheless, injecting an unconditional call here should be avoided for not
> only DAMON calls but any expensive calls?  I'm also not pretty sure what DAMON
> call you are thinking about.
> 

Just any call, DAMON or otherwise.  The explicit check injecting ~2-3%
overhead on my microbench was a simple

+       } else if (!folio_test_isolated(folio) &&
+                  (sysctl_numa_balancing_mode & NUMA_BALANCING_MEMORY_TIERING) &&

If this is causing additional overhead, call me skeptical that trying
anything more complicated will turn out better.

> > 
> >    I would need to explore this further - including whether it is
> >    feasible to inject such a large dependency into swap.c
> 
> I understand DAMON is not small in terms of the code size, and has many
> limitations that makes it unusable in many use cases.  But, again, I'm not
> pretty sure what kind of DAMON usage in swap.c you're thinking about, and
> therefore not easy to understyand what part of DAMON is considered as a large
> dependency that concerns you.  It would be great if we can make more concrete
> example as a result of this topic session at LSFMMBPF.
> 

It's not a matter of code size - it's a matter of tightly coupling core
components of the kernel to extraneous ones.  Adding additional
dependencies between components increases overall system complexity and
makes it hard to reason about the behavior of the system.

For example, in the prior snippet:

+       } else if (!folio_test_isolated(folio) &&
+                  (sysctl_numa_balancing_mode & NUMA_BALANCING_MEMORY_TIERING) &&
+                  numa_pagecache_promotion_enabled) {
+               promotion_candidate(folio);

This amounts to

if (some condition && feature enabled)
	mark a folio as a candidate for promotion

The promotion_candidate() function is contained within migrate.c and
uses (mostly) migrate.c mechanisms (from a task_work).

All you need to understand the behavior is between swap.c and migrate.c.

If instead you aggregate this to DAMON, understanding the behavior of
swap can require you to understand what DAMON is actually doing with
this information.

Now you need to understand swap.c, migrate.c, AND DAMON.

It makes it more difficult to reason about the system when something
goes wrong. This increases the maintenance burden for maintainers (and
onboarding complexity for anyone new to the kernel, for that matter).

That doesn't mean we shouldn't consider doing this - it just means that
benefit needs to outweight the complexity/maintenance cost.

> FYI, I also not having specific idea for helping unmapped pages promotion for
> now.  That's my assignment that I will do by LSFMMBPF.  But, a few things that
> I naively thinking DAMON might be able to help unmapped promotions are,
> 
> 1. Using DAMON for profiling how much hot and cold unmapped pages are in which
>    tier, and use the information for unmapped pages promotion optimization.
> 2. Using DAMOS to target-promote hot unmapped pages while using page
>    faults-based promotion for mapped pages.
> 3. Using DAMOS to promote both mapped and unmapped hot pages.
>

This missing the scenario where DAMOS/DAMON is not suitible for
deployment in someone's environment.  The kernel should still do
*something*.

And that is kind of the point - we can expose more complexity to the
users with DAMON, but the kernel should be able to do some reasonable
promotion action without this additional system.

> >    but I
> >    also see value is making tasks handle promotion as a form of fairness.
> 
> I agree that could be good in terms of fairness.  I want to learn more about
> the significance of it, though.
>

Fairness in this scenario is simple.

If one task is causing an outsizes number of promotions to occur, and it
causes some ASYNC system to handle those promotions, it is effectively
acquiring more CPU time via that ASYNC system than other residents.

Trying to charge this time back to the noisey task is harder than just
having the task incur the cost of migration.  But doing it inline can
cause the task to slow down.

So it's difficult to predict how it's going to pan out.  Need evidence.

> I agree.  DAMON allows combining multiple different mechanisms with its core
> logic, so I beleive it migt be a place that can aggregate the different
> identification mechanisms.
> 
> DAMON's access monitoring results based system operations feature, namely
> DAMOS, also has its own aggressiveness control logic, and resides in the core
> layer, so could be used consistently with different promotion candidates
> identification mechanisms.
> 

Without data this is a nice thought, but we have existing mechanisms
that work and can be improved - lets not disrupt that.

Finding an aggregated promotion solution helps everyone move forward
without disrupting development in these areas (and makes the different
indentification mechanisms play nice with each other).

Trying to also create a voltron "one indentification system to rule them
all" is a nice thought, but it's heavy-weight compared to adding a folio
flag check and a call to mpol_migrate_misplaced().  We need to respect
that reality and not regress the existing mechanisms by trying to
over-engineer a generalized solution.

~Gregory

Powered by blists - more mailing lists