[PATCH v2 0/2] DAMON multiple contexts support

Alex Rusuf posted 2 patches 1 year, 8 months ago
include/linux/damon.h        |  80 ++++--
include/trace/events/damon.h |  14 +-
mm/damon/core-test.h         |   2 +-
mm/damon/core.c              | 509 ++++++++++++++++++++++-------------
mm/damon/dbgfs-test.h        |   4 +-
mm/damon/dbgfs.c             | 342 ++++++++++++++---------
mm/damon/lru_sort.c          |  31 ++-
mm/damon/modules-common.c    |  35 ++-
mm/damon/modules-common.h    |   3 +-
mm/damon/reclaim.c           |  30 ++-
mm/damon/sysfs.c             | 303 +++++++++++++--------
11 files changed, 872 insertions(+), 481 deletions(-)
[PATCH v2 0/2] DAMON multiple contexts support
Posted by Alex Rusuf 1 year, 8 months ago
Currently kdamond uses only one context per kthread
and most of its time it sleeps, so utilizing several
contexts can scale kdamond and allow it to use
another set of operations.

This patch-set implements support for multiple contexts
per kdamond.

In pseudo code previous versions worked like
the following:

	while (!kdamond_should_stop()) {

		/* prepare accesses for only 1 context */
		prepare_accesses(damon_context);

		sleep(sample_interval);

		/* check accesses for only 1 context */
		check_accesses(damon_context);

		...
	}

With this patch kdamond workflow will look
like the following:

	while (!kdamond_shoule_stop()) {

		/* prepare accesses for all contexts in kdamond */
		damon_for_each_context(ctx, kdamond)
			prepare_accesses(ctx);

		sleep(sample_interval);

		/* check_accesses for all contexts in kdamond */
		damon_for_each_context(ctx, kdamond)
			check_accesses(ctx);

			...
	}

To try this you can use modified kernel[1] and
damo[2]. I also have written few simple shell scripts[3]
to collect data for damo.

	[1] https://github.com/onlyoneofme/damon-multi-contexts.git
	[2] https://github.com/onlyoneofme/damo/tree/multi-contexts
	[3] https://github.com/onlyoneofme/damon-multi-contexts-tests.git

---
Changes from v1 (https://lore.kernel.org/damon/20240515152457.603724-1-yorha.op@gmail.com/)
- Compatibility for DebugFS interface is kept
- Kunit tests build/execution issues are fixed
- Patches from v1 are sqaushed, so that consistency between patches is
kept
- Added/Fixed comments about data structures/functions

Alex Rusuf (2):
  mm/damon/core: add 'struct kdamond' abstraction layer
  mm/damon/core: implement multi-context support

 include/linux/damon.h        |  80 ++++--
 include/trace/events/damon.h |  14 +-
 mm/damon/core-test.h         |   2 +-
 mm/damon/core.c              | 509 ++++++++++++++++++++++-------------
 mm/damon/dbgfs-test.h        |   4 +-
 mm/damon/dbgfs.c             | 342 ++++++++++++++---------
 mm/damon/lru_sort.c          |  31 ++-
 mm/damon/modules-common.c    |  35 ++-
 mm/damon/modules-common.h    |   3 +-
 mm/damon/reclaim.c           |  30 ++-
 mm/damon/sysfs.c             | 303 +++++++++++++--------
 11 files changed, 872 insertions(+), 481 deletions(-)

-- 
2.42.0
Re: [PATCH v2 0/2] DAMON multiple contexts support
Posted by SeongJae Park 1 year, 8 months ago
Hello Alex,

On Fri, 31 May 2024 15:23:18 +0300 Alex Rusuf <yorha.op@gmail.com> wrote:

> Currently kdamond uses only one context per kthread
> and most of its time it sleeps, so utilizing several
> contexts can scale kdamond and allow it to use
> another set of operations.
> 
> This patch-set implements support for multiple contexts
> per kdamond.
>
[...]
> 
> ---
> Changes from v1 (https://lore.kernel.org/damon/20240515152457.603724-1-yorha.op@gmail.com/)
> - Compatibility for DebugFS interface is kept
> - Kunit tests build/execution issues are fixed
> - Patches from v1 are sqaushed, so that consistency between patches is
> kept

My request was to avoid unnecessary temporal changes that will be removed in
next patches.  Some of those are well removed in this version, but I still show
some.  E.g., nr_contexts field.  Also, this resulted in two big patches.

I'd also appreciate if you can separate changes into smaller ones of logical
single change.  For example, changes for lru_sort.c, reclaim.c, and sysfs.c on
first patch could be much smaller in my opinion.  Traceevent change can also be
separated from patch 2.  Some of multi-context support seems mixed in patch 1.

I'd suggest below patches flow.

Patch 1: Introduce new struct and control functions for the struct.  Don't
really use the struct and the functions.

Patch 2: Modify core.c to use the struct and implement multiple contexts
support.  Minimize changes to core.c users.  Just keep those work as before.
Don't implement multi contexts support on sysfs.c or trace events at this
point.

Patch 3: Update sysfs.c to support the multiple contexts.

Patch 4: Update trace events to better support it.

> - Added/Fixed comments about data structures/functions

Also, you don't need to put version history under '---' marker if it is a cover
letter.  You can put it on the body.

> 
> Alex Rusuf (2):
>   mm/damon/core: add 'struct kdamond' abstraction layer
>   mm/damon/core: implement multi-context support

I will try to put more detailed comments on each patch.

> 
>  include/linux/damon.h        |  80 ++++--
>  include/trace/events/damon.h |  14 +-
>  mm/damon/core-test.h         |   2 +-
>  mm/damon/core.c              | 509 ++++++++++++++++++++++-------------
>  mm/damon/dbgfs-test.h        |   4 +-
>  mm/damon/dbgfs.c             | 342 ++++++++++++++---------
>  mm/damon/lru_sort.c          |  31 ++-
>  mm/damon/modules-common.c    |  35 ++-
>  mm/damon/modules-common.h    |   3 +-
>  mm/damon/reclaim.c           |  30 ++-
>  mm/damon/sysfs.c             | 303 +++++++++++++--------
>  11 files changed, 872 insertions(+), 481 deletions(-)
> 
> -- 
> 2.42.0


Thanks,
SJ
Re: [PATCH v2 0/2] DAMON multiple contexts support
Posted by Alex Rusuf 1 year, 8 months ago
Hi SJ,

> Hello Alex,
> 
> On Fri, 31 May 2024 15:23:18 +0300 Alex Rusuf <yorha.op@gmail.com> wrote:
> 
> > Currently kdamond uses only one context per kthread
> > and most of its time it sleeps, so utilizing several
> > contexts can scale kdamond and allow it to use
> > another set of operations.
> > 
> > This patch-set implements support for multiple contexts
> > per kdamond.
> >
> [...]
> > 
> > ---
> > Changes from v1 (https://lore.kernel.org/damon/20240515152457.603724-1-yorha.op@gmail.com/)
> > - Compatibility for DebugFS interface is kept
> > - Kunit tests build/execution issues are fixed
> > - Patches from v1 are sqaushed, so that consistency between patches is
> > kept
> 
> My request was to avoid unnecessary temporal changes that will be removed in
> next patches.  Some of those are well removed in this version, but I still show
> some.  E.g., nr_contexts field.  Also, this resulted in two big patches.

This makes sense and I actually wanted that as well, so I tried to separate
them in previous version, looks like I misunderstood your request.

Anyway, don't you mind if lru_sort/traceevents/etc. will not function
correctly without applying the whole patch-set? I mean if we use the
approach below, once core.c is modified at least lru_sort and reclaim
will not work correctly, they even will not be built.

> 
> I'd also appreciate if you can separate changes into smaller ones of logical
> single change.  For example, changes for lru_sort.c, reclaim.c, and sysfs.c on
> first patch could be much smaller in my opinion.  Traceevent change can also be
> separated from patch 2.  Some of multi-context support seems mixed in patch 1.
> 
> I'd suggest below patches flow.
> 
> Patch 1: Introduce new struct and control functions for the struct.  Don't
> really use the struct and the functions.
> 
> Patch 2: Modify core.c to use the struct and implement multiple contexts
> support.  Minimize changes to core.c users.  Just keep those work as before.
> Don't implement multi contexts support on sysfs.c or trace events at this
> point.
> 
> Patch 3: Update sysfs.c to support the multiple contexts.
> 
> Patch 4: Update trace events to better support it.
> 
> > - Added/Fixed comments about data structures/functions
> 
> Also, you don't need to put version history under '---' marker if it is a cover
> letter.  You can put it on the body.
> 
> > 
> > Alex Rusuf (2):
> >   mm/damon/core: add 'struct kdamond' abstraction layer
> >   mm/damon/core: implement multi-context support
> 
> I will try to put more detailed comments on each patch.
> 
> > 
> >  include/linux/damon.h        |  80 ++++--
> >  include/trace/events/damon.h |  14 +-
> >  mm/damon/core-test.h         |   2 +-
> >  mm/damon/core.c              | 509 ++++++++++++++++++++++-------------
> >  mm/damon/dbgfs-test.h        |   4 +-
> >  mm/damon/dbgfs.c             | 342 ++++++++++++++---------
> >  mm/damon/lru_sort.c          |  31 ++-
> >  mm/damon/modules-common.c    |  35 ++-
> >  mm/damon/modules-common.h    |   3 +-
> >  mm/damon/reclaim.c           |  30 ++-
> >  mm/damon/sysfs.c             | 303 +++++++++++++--------
> >  11 files changed, 872 insertions(+), 481 deletions(-)
> > 
> > -- 
> > 2.42.0
> 
> 
> Thanks,
> SJ

BR,
Alex
Re: [PATCH v2 0/2] DAMON multiple contexts support
Posted by SeongJae Park 1 year, 8 months ago
On Sun,  2 Jun 2024 18:31:02 +0300 Alex Rusuf <yorha.op@gmail.com> wrote:

> Hi SJ,
> 
> > Hello Alex,
> > 
> > On Fri, 31 May 2024 15:23:18 +0300 Alex Rusuf <yorha.op@gmail.com> wrote:
> > 
> > > Currently kdamond uses only one context per kthread
> > > and most of its time it sleeps, so utilizing several
> > > contexts can scale kdamond and allow it to use
> > > another set of operations.
> > > 
> > > This patch-set implements support for multiple contexts
> > > per kdamond.
> > >
> > [...]
> > > 
> > > ---
> > > Changes from v1 (https://lore.kernel.org/damon/20240515152457.603724-1-yorha.op@gmail.com/)
> > > - Compatibility for DebugFS interface is kept
> > > - Kunit tests build/execution issues are fixed
> > > - Patches from v1 are sqaushed, so that consistency between patches is
> > > kept
> > 
> > My request was to avoid unnecessary temporal changes that will be removed in
> > next patches.  Some of those are well removed in this version, but I still show
> > some.  E.g., nr_contexts field.  Also, this resulted in two big patches.
> 
> This makes sense and I actually wanted that as well, so I tried to separate
> them in previous version, looks like I misunderstood your request.

No problem, I think I should also be more clear about the point.  Happy to have
a chance to develop my humble communication skill with the conversaions with
you.

> 
> Anyway, don't you mind if lru_sort/traceevents/etc. will not function
> correctly without applying the whole patch-set? I mean if we use the
> approach below, once core.c is modified at least lru_sort and reclaim
> will not work correctly, they even will not be built.

I mind those.  Everything should work without regression in the middle of the
patchset.  Nonetheless, we should avoid only regression.  We don't need to make
everything perfect.  Let's minimize changes to the other modules in the way.

I believe below suggested patches flow and my second reply to the second patch
of this patchset can clarify the point.  Please let me know if not.

> 
> > 
> > I'd also appreciate if you can separate changes into smaller ones of logical
> > single change.  For example, changes for lru_sort.c, reclaim.c, and sysfs.c on
> > first patch could be much smaller in my opinion.  Traceevent change can also be
> > separated from patch 2.  Some of multi-context support seems mixed in patch 1.
> > 
> > I'd suggest below patches flow.
> > 
> > Patch 1: Introduce new struct and control functions for the struct.  Don't
> > really use the struct and the functions.
> > 
> > Patch 2: Modify core.c to use the struct and implement multiple contexts
> > support.  Minimize changes to core.c users.  Just keep those work as before.
> > Don't implement multi contexts support on sysfs.c or trace events at this
> > point.
> > 
> > Patch 3: Update sysfs.c to support the multiple contexts.
> > 
> > Patch 4: Update trace events to better support it.

As I mentioned on my second reply to the second patch, you could swich patches
3 and 4 if you want trace events to work perfect from the beginning of
user-visible multi contexts support.


Thanks,
SJ

[...]