linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: SeongJae Park <sj@kernel.org>
To: Sang-Heon Jeon <ekffu200098@gmail.com>
Cc: SeongJae Park <sj@kernel.org>,
	honggyu.kim@sk.com, damon@lists.linux.dev, linux-mm@kvack.org
Subject: Re: [PATCH 1/2] selftests/damon: test no-op commit broke DAMON status
Date: Sat,  9 Aug 2025 10:35:59 -0700	[thread overview]
Message-ID: <20250809173559.71327-1-sj@kernel.org> (raw)
In-Reply-To: <CABFDxMH0LDLLdJf-t2DEHCD40EJDfcq35cJM-HS5GkLX7KqsHA@mail.gmail.com>

On Sat, 9 Aug 2025 15:39:41 +0900 Sang-Heon Jeon <ekffu200098@gmail.com> wrote:

> Hi SeongJae
> 
> On Sat, Aug 9, 2025 at 7:05 AM SeongJae Park <sj@kernel.org> wrote:
> >
> > Hello Sang-Heon,
> >
> > On Sat,  9 Aug 2025 04:55:17 +0900 Sang-Heon Jeon <ekffu200098@gmail.com> wrote:
> >
> > > Add test to verify that DAMON status is not changed after a no-op
> > > commit.
> >
> > Thank you for this patch!  I have some comments below, though.
> >
> > > Currently, it is failing. but after fix the bug it should be
> > > success.
> >
> > To keep bisecting goes smoothly, let's introduce failing tests _after_ their
> > fixes are landed.
> 
> I see. As mentioned in another comment, I'll merge this patch set into
> one patch. I think this comment will be resolved then.

That could make the patch be unnecessarily difficult to apply on older kernels
due to the test part.  Please keep those separated.

[...]
> > > +    if rc != 0:
> > > +        return None, f'drgn fail : return code({rc})'
> >
> > Let's not add a space before ':', for the consistency.
> 
> I see. I made so many mistakes like these; (I worked this very late
> night...) I will fix them all. Thank you for checking this moment.

I'm sorry to hear that you had to work on late night :'(  I hope you to have
sufficient rests.

[...]
> To be honest, I didn't use kselftest framework while making these
> patch set because I don't know how kernel developers use it.
> 
> If you are okay, could you share how you use it?

Sure.  I do below for running DAMON selftests.

    $ sudo make -C tools/testing/selftests/damon run_tsets

You could also run DAMON kunit tests like below.

    $ ./tools/testing/kunit/kunit.py run --kunitconfig mm/damon/tests/

I usually run all those together with additional tests, using DAMON tests
suite[2].

> I just build kernel
> with custom patch, and execute output images by vm, and setup env,
> then just execute tests(it's so raw.. i know)

It's ok.  No one fits all, so different tests are needed for different hacks.
Please keep using whatever works best for you :)

> 
> Maybe is there a kernel build option that automatically includes tests
> to output image? Or just a general or smarter way to execute the whole
> test?

To be honest I'm also not an expert of kselftest and kunit :)  You coule refer
to the kselftest documentation[1] for more details.

[...]
> Thanks for reviewing! Have a nice weekend. I'll do this job after my
> personal schedule finishes.

No worry, take your time and fun! :)

[...]

[1] https://docs.kernel.org/dev-tools/kselftest.html
[2] https://github.com/damonitor/damon-tests/blob/master/corr/run.sh


Thanks,
SJ


  reply	other threads:[~2025-08-09 17:36 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-08 19:55 [PATCH 0/2] fix damos_commit_ops_filters iteration problem with Sang-Heon Jeon
2025-08-08 19:55 ` [PATCH 1/2] selftests/damon: test no-op commit broke DAMON status Sang-Heon Jeon
2025-08-08 22:05   ` SeongJae Park
2025-08-09  6:27     ` Sang-Heon Jeon
2025-08-09  6:39     ` Sang-Heon Jeon
2025-08-09 17:35       ` SeongJae Park [this message]
2025-08-10  5:43         ` Sang-Heon Jeon
2025-08-08 19:55 ` [PATCH 2/2] mm/damon/core : fix commit_ops_filters by using correct nth function Sang-Heon Jeon
2025-08-08 22:08   ` SeongJae Park
2025-08-08 23:26     ` SeongJae Park

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=20250809173559.71327-1-sj@kernel.org \
    --to=sj@kernel.org \
    --cc=damon@lists.linux.dev \
    --cc=ekffu200098@gmail.com \
    --cc=honggyu.kim@sk.com \
    --cc=linux-mm@kvack.org \
    /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;
as well as URLs for NNTP newsgroup(s).