All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mike Rapoport <rppt@kernel.org>
To: "Lorenzo Stoakes (Oracle)" <ljs@kernel.org>
Cc: Breno Leitao <leitao@debian.org>,
	"David Hildenbrand (Arm)" <david@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	"Liam R. Howlett" <Liam.Howlett@oracle.com>,
	Vlastimil Babka <vbabka@kernel.org>,
	Suren Baghdasaryan <surenb@google.com>,
	Michal Hocko <mhocko@suse.com>, Shuah Khan <shuah@kernel.org>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	linux-kselftest@vger.kernel.org, kernel-team@meta.com
Subject: Re: [PATCH] selftests/mm: add THP sysfs interface test
Date: Tue, 17 Mar 2026 07:43:41 +0200	[thread overview]
Message-ID: <abjqDUpkezMfvCHz@kernel.org> (raw)
In-Reply-To: <c4cb6dfe-59b2-44e6-adbc-9ee63bed79fd@lucifer.local>

On Mon, Mar 16, 2026 at 07:53:46PM +0000, Lorenzo Stoakes (Oracle) wrote:
> On Mon, Mar 16, 2026 at 09:02:33AM -0700, Breno Leitao wrote:
> > On Mon, Mar 16, 2026 at 03:44:14PM +0100, David Hildenbrand (Arm) wrote:
> > > On 3/16/26 14:47, Breno Leitao wrote:
> > > > On Mon, Mar 16, 2026 at 12:55:13PM +0000, Lorenzo Stoakes (Oracle) wrote:
> > > >> On Mon, Mar 09, 2026 at 05:00:34AM -0700, Breno Leitao wrote:
> > > >>> Add a shell-based selftest that exercises the full set of THP sysfs
> > > >>> knobs: enabled (global and per-size anon), defrag, use_zero_page,
> > > >>> hpage_pmd_size, shmem_enabled (global and per-size), shrink_underused,
> > > >>> khugepaged/ tunables, and per-size stats files.
> > > >>>
> > > >>> Each writable knob is tested for valid writes, invalid-input rejection,
> > > >>> idempotent writes, and mode transitions where applicable. All original
> > > >>> values are saved before testing and restored afterwards.
> > > >>>
> > > >>> The test uses the kselftest KTAP framework (ktap_helpers.sh) for
> > > >>> structured TAP 13 output, making results parseable by the kselftest
> > > >>> harness. The test plan is printed at the end since the number of test
> > > >>> points is dynamic (depends on available hugepage sizes and sysfs files).
> > > >>>
> > > >>> This is particularly useful for validating the refactoring of
> > > >>> enabled_store() and anon_enabled_store() to use sysfs_match_string()
> > > >>> and the new change_enabled()/change_anon_orders() helpers.
> > > >>>
> > > >>> Signed-off-by: Breno Leitao <leitao@debian.org>
> > > >>
> > > >> The test is broken locally for me, returning error code 127.
> > > >>
> > > >> I do appreciate the effort here, so I'm sorry to push back negatively, but I
> > > >> feel a bash script here is pretty janky, and frankly if any of these interfaces
> > > >> were as broken as this it'd be a major failure that would surely get picked up
> > > >> far sooner elsewhere.
> > > >>
> > > >> So while I think this might be useful as a local test for your sysfs interface
> > > >> changes, I don't think this is really suited to the mm selftests.
> > > >
> > > > That is totally fine. This test is what I have been using to test the
> > > > changes, and I decide to share it in case someone find it useful.
> > > >
> > > > Let's drop it.
> > >
> > > Out of interest, to we know why the test is failing for Lorenzo?
> >
> > I really don't know, but, it sounds like ktap was not found?
> 
> Yeah CONFIG_KUNIT is not set so could be :)

Nah, CONFIG_KUNIT has nothing to do with ktap_helpers.sh, probably your
environment does not bring in tools/testing/selftests/kselftest
 
> >
> > Then the first early-exit path hit:
> > ktap_skip_all "..."   # undefined → returns 127 exit "$KSFT_SKIP"
> > # expands to: exit "" → exits with last $? = 127
> >
> > > I agree that the test is a bit excessive, in particular when it comes to
> > > invalid/idempotent values etc. I could see some value for testing
> > > whether setting the modes keeps working, but also then I wonder if that
> > > is really something we'll be changing frequently (and that breaks easily).
> >
> > yea, I make it very excessive, because there were some intrinsics in
> > those sysfs that I was gettingit wrong when doing the intial conversion.
> >
> > So, the test is something that I trust now, and I found it useful when
> > finding regressiosn.
> >
> > Is is something that will chagne frequently? probably not!
> >
> > That said, would you like to have a simplified/different version of this
> > test?
> 
> In an ideal world we'd use kunit or something to assert it internal to the
> kernel I guess, but if we do have something scaled down it'd at least be nice to
> have in C? :)
> 
> I am not sure how useful it'd be though overall, I don't see us changing this
> too often and really we're more interested in asserting behaviour.
> 
> Sadly THP is inherently tricky to test generally because of its very nature, I
> wish we could have better test isolation etc.
> 
> See tools/testing/vma for a forlorn dream of kernel code being run in userland
> (but oh how the stubs/duplicate declarations/etc. are a pain).
> 
> I suspect THP could never be given the same treatment though! :)
> 
> Cheers, Lorenzo

-- 
Sincerely yours,
Mike.

  reply	other threads:[~2026-03-17  5:43 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-09 12:00 [PATCH] selftests/mm: add THP sysfs interface test Breno Leitao
2026-03-16 12:55 ` Lorenzo Stoakes (Oracle)
2026-03-16 13:47   ` Breno Leitao
2026-03-16 14:44     ` David Hildenbrand (Arm)
2026-03-16 16:02       ` Breno Leitao
2026-03-16 19:53         ` Lorenzo Stoakes (Oracle)
2026-03-17  5:43           ` Mike Rapoport [this message]
2026-03-17  8:45             ` Lorenzo Stoakes (Oracle)

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=abjqDUpkezMfvCHz@kernel.org \
    --to=rppt@kernel.org \
    --cc=Liam.Howlett@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=david@kernel.org \
    --cc=kernel-team@meta.com \
    --cc=leitao@debian.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=ljs@kernel.org \
    --cc=mhocko@suse.com \
    --cc=shuah@kernel.org \
    --cc=surenb@google.com \
    --cc=vbabka@kernel.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 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.