linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: David Hildenbrand <david@redhat.com>
To: Brendan Jackman <jackmanb@google.com>,
	Brendan Higgins <brendan.higgins@linux.dev>,
	David Gow <davidgow@google.com>, Rae Moar <rmoar@google.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Oscar Salvador <osalvador@suse.de>
Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>,
	Vlastimil Babka <vbabka@suse.cz>,
	Michal Hocko <mhocko@kernel.org>,
	linux-kselftest@vger.kernel.org, kunit-dev@googlegroups.com,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	Yosry Ahmed <yosry.ahmed@linux.dev>
Subject: Re: [PATCH RFC 0/4] mm: KUnit tests for the page allocator
Date: Tue, 25 Feb 2025 11:01:47 +0100	[thread overview]
Message-ID: <0449ff75-0a6b-4c1e-bf12-ff052aad5287@redhat.com> (raw)
In-Reply-To: <20250224-page-alloc-kunit-v1-0-d337bb440889@google.com>

On 24.02.25 15:47, Brendan Jackman wrote:
> The page allocator does a lot of stuff that is not visible to the user
> in any deterministic way. But this stuff is still important and it would
> be nice to test that behaviour.
> 
> KUnit is a tool for unit-testing kernel-internal APIs. This is an
> attempt to adopt it the page allocator.
> 
> I have been hacking on this as a way to try and test the code I'm
> writing for my ASI page_alloc integration proposal [0]. It's been
> extremely useful to be able to "just call it and see what it does". So I
> wanna gather some feedback on whether this basic idea is of interest
> before I invest too much more time in it.
> 
> You can run these tests like this:
> 
> tools/testing/kunit/kunit.py run \
> 	--arch=x86_64 --kernel_args="movablecore=2G" \
> 	--qemu_args="-m 4G" --kunitconfig mm/.kunitconfig
> 
> Unit-testing code that has mutable global variables can be a pain.
> Unit-testing code with mutable global variables _that can change
> concurrently with the tests_ is basically impossible. So, we need some
> way to isolate an "instance" of the allocator that doesn't refer to any
> such concurrently-mutated state.
> 
> Luckily, the allocator only has one really important global variable:
> node_data. So, the approach here is to carve out a subset of that
> variable which is as isolated as possible from the rest of rthe system,
> which can be used for deterministic testing. This is achieved by crating
> a fake "isolated" node at boot, and plugging in memory at test init
> time.
> 
> This is an RFC and not a PATCH because:
> 
> 1. I have not taken much care to ensure the isolation is complete.
>     There are probably sources of flakiness and nondeterminism in here.
> 
> 2. I suspect the the basic idea might be over-complicated: do we really
>     need memory hotplug here? Do we even need the instance of the
>     allocator we're testing to actual memory behind the pages it's
>     allocating, or could we just hallucinate a new region of vmemmap
>     without any of that awkwardness?
> 
>     One significant downside of relying on memory hotplug is that the
>     test won't run if we can't hotplug anything out. That means you have
>     to fiddle with the platform to even run the tests - see the
>     --kernel_args and --qemu_args I had to add to my kunit.py command
>     above.
> 
>     So yeah, other suggestions welcome.
> 
>     2b. I'm not very confident I'm using the hotplug API properly.

Me neither ;)

Dynamically adding memory to that "fake" node is certainly interesting, 
but which guarantees do we have that some other features (page 
migration, memory offlining, page reporting ...) don't interact in weird 
ways with this "fake" node? Adding special-casing all over the place for 
that feels wrong. I assume this is point 1. you note above.

So I don't quite love the idea on first sight ... but I haven't grasped 
all details of the full picture yet I'm afraid.

-- 
Cheers,

David / dhildenb



  parent reply	other threads:[~2025-02-25 10:01 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-24 14:47 Brendan Jackman
2025-02-24 14:47 ` [PATCH RFC 1/4] kunit: Allocate assertion data with GFP_ATOMIC Brendan Jackman
2025-02-24 14:47 ` [PATCH RFC 2/4] mm/page_alloc_test: Add empty KUnit boilerplate Brendan Jackman
2025-02-24 14:47 ` [PATCH RFC 3/4] mm/page_alloc_test: Add logic to isolate a node for testing Brendan Jackman
2025-02-24 18:33   ` Yosry Ahmed
2025-02-25 11:20     ` Brendan Jackman
2025-02-26 10:33       ` Brendan Jackman
2025-02-24 14:47 ` [PATCH RFC 4/4] mm/page_alloc_test: Add smoke-test for page allocation Brendan Jackman
2025-02-24 18:26   ` Yosry Ahmed
2025-02-25 11:14     ` Brendan Jackman
2025-02-26 10:47   ` Brendan Jackman
2025-02-25 10:01 ` David Hildenbrand [this message]
2025-02-25 12:56   ` [PATCH RFC 0/4] mm: KUnit tests for the page allocator Brendan Jackman
2025-02-26 11:52     ` David Hildenbrand
2025-02-26 12:21       ` Brendan Jackman

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=0449ff75-0a6b-4c1e-bf12-ff052aad5287@redhat.com \
    --to=david@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=brendan.higgins@linux.dev \
    --cc=davidgow@google.com \
    --cc=jackmanb@google.com \
    --cc=kunit-dev@googlegroups.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=lorenzo.stoakes@oracle.com \
    --cc=mhocko@kernel.org \
    --cc=osalvador@suse.de \
    --cc=rmoar@google.com \
    --cc=vbabka@suse.cz \
    --cc=yosry.ahmed@linux.dev \
    /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