From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id F2E48C021A4 for ; Mon, 24 Feb 2025 18:26:16 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 6B8526B0082; Mon, 24 Feb 2025 13:26:16 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 666AE28000D; Mon, 24 Feb 2025 13:26:16 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5070C28000A; Mon, 24 Feb 2025 13:26:16 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 2EE506B0082 for ; Mon, 24 Feb 2025 13:26:16 -0500 (EST) Received: from smtpin17.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id D29DE81ACC for ; Mon, 24 Feb 2025 18:26:15 +0000 (UTC) X-FDA: 83155667910.17.127F8BF Received: from out-180.mta0.migadu.com (out-180.mta0.migadu.com [91.218.175.180]) by imf04.hostedemail.com (Postfix) with ESMTP id D0A0740004 for ; Mon, 24 Feb 2025 18:26:13 +0000 (UTC) Authentication-Results: imf04.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=Ig0UXYrW; spf=pass (imf04.hostedemail.com: domain of yosry.ahmed@linux.dev designates 91.218.175.180 as permitted sender) smtp.mailfrom=yosry.ahmed@linux.dev; dmarc=pass (policy=none) header.from=linux.dev ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1740421574; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=5J2l0tJZk9nYL86YBBSDH7LqcNOXciko+RDvn9pcvbI=; b=wH4vyxmFohUmMC72KSfuWL7S3jePqBJZTHWCYqjpFseHWSHZC7tfNBpFv5R4Sjk628oAWK Q0qeOi79b7AbJILBA9eOi5RJbAMIJKgj/q1g4l/dwjHXaEOTPrTNM9ZJ0x32A7CZbhMEBs 4KaTueMdmVnS61hv3VsDEmM/TXOzh8E= ARC-Authentication-Results: i=1; imf04.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=Ig0UXYrW; spf=pass (imf04.hostedemail.com: domain of yosry.ahmed@linux.dev designates 91.218.175.180 as permitted sender) smtp.mailfrom=yosry.ahmed@linux.dev; dmarc=pass (policy=none) header.from=linux.dev ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1740421574; a=rsa-sha256; cv=none; b=CkDSgODMgoeNO6QNPHgYTMT7ndPLy4n+DTEPHkxfZb7ny44kyM0B2zGy7IoaassdV1c5lq /w+Fg9CzvrXdI9TLfqRY2mtAG4tTYH9xMoL8Hy3arxdtC/XaAUzgS/nP4LzVo1oJuVXC0I /ZqGgFTFa7uQdKJaeT18r3/Sph/lX0A= Date: Mon, 24 Feb 2025 18:26:04 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1740421571; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=5J2l0tJZk9nYL86YBBSDH7LqcNOXciko+RDvn9pcvbI=; b=Ig0UXYrWUdVAfcJ16aiZ5biBRkXiRRPtmDKip/HoNVI8AHkxL9MMuwK9Cus9SkJUc01Gf1 6Nmk2DSJC2yaCXYT+drfhGw0owjc7FNf8ky/+t8uESGlyhfpCQ5Xwp0fWUbqlG5yXGXKop t1GnSAurCiQpo2z2bMGzSQjkCxqmXSA= X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Yosry Ahmed To: Brendan Jackman Cc: Brendan Higgins , David Gow , Rae Moar , Andrew Morton , David Hildenbrand , Oscar Salvador , Lorenzo Stoakes , Vlastimil Babka , Michal Hocko , linux-kselftest@vger.kernel.org, kunit-dev@googlegroups.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH RFC 4/4] mm/page_alloc_test: Add smoke-test for page allocation Message-ID: References: <20250224-page-alloc-kunit-v1-0-d337bb440889@google.com> <20250224-page-alloc-kunit-v1-4-d337bb440889@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20250224-page-alloc-kunit-v1-4-d337bb440889@google.com> X-Migadu-Flow: FLOW_OUT X-Rspam-User: X-Stat-Signature: 9gj3zm6ugtdqttqjmk9kpsqdcgjnpj3f X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: D0A0740004 X-HE-Tag: 1740421573-636341 X-HE-Meta: U2FsdGVkX1/be64v5SF7xE+3ngZJy0Sj/N/iqPPEEQmVzXQbdkOFQ+6BPCecWG5E6E/Tsx4mEuLFIdaPdmsPqQGv6hGZ7nwnHm4gW/xiNjsQjJfJt+feWN6nMxvmSaswSeswh3DL2G8Z+ntiA6SF9XJhJwOqs2BMTnNX7+0b6b80LxnKZGW3GIeaA84UQ5brpOFR4FnF/qNCbyBWN1/wJT0Pv32CsYEDHtyGp4jVwD2FJYVx71x5aqIwNtw/PvjgyAdt1qAjsZuWvsPFsWqKYsNDwVR6Qx4+nYuhVXaKcVLKenarViBtBFHbZhnh+XfUed9iAhapS26sHxdimyAfJiivUZOrQOsjSKdXF3Q9Bw8+ooig9ksV8lKgzCxT29RP7mNP9wg/MmgIX9MxEfzcspj2okfkxvOJTZg+gRc/MA/r4Z8Sk2/yjNM5S7Kh5IMQjTwc3DLmm+lW3pP09uuZDYcR85PboCWnYTxI9j80mQf+gUDbxuxQb5vtdDnq/k1RvAcGAS1Wcvp5RqelkCr6osHrJJanLB6uL7+8LAIb4IKe5qoMI/gRpGjtWaN3Mo1BAxPBJraCGvWBMcAQ5Noq2UgNzkZxALQaRiAfRFZSR0lm7miZKEPTaFrHpMJDpqOxnJKMk0bafYNElCZSo891fvx6KdoAZJYwEgZn90Bn6gnPWOlRlDYFnvFBCx0v2iJmKBK9tRYNtVJch7tqCa0tIrHl9lPnoc5M7K//+cM2owY9DI524ikDarkS3M6TtzuBNb3putK5TxBU9D1unalRcMbsdLT3gGYzPRnQpGwoxN8USsPKiD+Cm61YXAOQlGoLHwm+DkWdmLWK3yZVVu+l+d2Kz9/3RRIB8bbaossTeB6J/frE0hfDyEtllYU/a20ohmaHXsp1abnRB1fiYJzqK4CWZ5HQ9b2kuE/kHbrpfpEp1v9g4N7sfYfNY/2w3rKSoQAljbvIRIr55kuB2KX UQk3vvC1 +7wAtm1xf7rKQyKmxSKbWRmKUXQE1k7kTuCkzAIaS/B7iL/KROVEtP4mow6jIxEfOHqbqh/JTGufZxgni5iW2MxUswh4628qMtPw28UL4yaFBY5sI6Ecc1JUys0TomRdt4KGi3o4mnRrjWBoPsEid3BXzb29f7Cf+I85V3Y3WstQLbdEi/7iArrwBFuQdt0oVDqFlc16RHEG0OAWrde+lNwZ7C8wR09OSl626vdQJJM/HyQAlDI7hPPLEC7EqLfwXv9Ml01FWECjoqtoNwuUmfGLT9XBrVb+9/JoL X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Mon, Feb 24, 2025 at 02:47:14PM +0000, Brendan Jackman wrote: > This is the bare minimum to illustrate what KUnit code would look like > that covers the page allocator. > > Even this trivial test illustrates a couple of nice things that are > possible when testing via KUnit > > 1. We can directly assert that the correct zone was used. > (Although note due to the simplistic setup, you can have any zone you > like as long as it's ZONE_NORMAL). > > 2. We can assert that a page got freed. It's probably pretty unlikely > that we'd have a bug that actually causes a page to get leaked by the > allocator, but it serves as a good example of the kind of assertions > we can make by judicously peeking at allocator internals. > > Signed-off-by: Brendan Jackman > --- > mm/page_alloc_test.c | 139 ++++++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 138 insertions(+), 1 deletion(-) > > diff --git a/mm/page_alloc_test.c b/mm/page_alloc_test.c > index c6bcfcaf61b57ca35ad1b5fc48fd07d0402843bc..0c4effb151f4cd31ec6a696615a9b6ae4964b332 100644 > --- a/mm/page_alloc_test.c > +++ b/mm/page_alloc_test.c > @@ -26,6 +26,139 @@ > } \ > }) > > +#define EXPECT_WITHIN_ZONE(test, page, zone) ({ \ ultranit: the '\' above is unaligned with the ones below. > + unsigned long pfn = page_to_pfn(page); \ > + unsigned long start_pfn = zone->zone_start_pfn; \ > + unsigned long end_pfn = start_pfn + zone->spanned_pages; \ > + \ > + KUNIT_EXPECT_TRUE_MSG(test, \ > + pfn >= start_pfn && pfn < end_pfn, \ > + "Wanted PFN 0x%lx - 0x%lx, got 0x%lx", \ > + start_pfn, end_pfn, pfn); \ > + KUNIT_EXPECT_PTR_EQ_MSG(test, page_zone(page), zone, \ > + "Wanted %px (%s), got %px (%s)", \ > + zone, zone->name, page_zone(page), page_zone(page)->name); \ > +}) > + > +static void action_nodemask_free(void *ctx) > +{ > + NODEMASK_FREE(ctx); > +} > + > +/* > + * Call __alloc_pages_noprof with a nodemask containing only the nid. > + * > + * Never returns NULL. > + */ > +static inline struct page *alloc_pages_force_nid(struct kunit *test, > + gfp_t gfp, int order, int nid) > +{ > + NODEMASK_ALLOC(nodemask_t, nodemask, GFP_KERNEL); For the sake of the test can't we just put the nodemask on the stack? > + struct page *page; > + > + KUNIT_ASSERT_NOT_NULL(test, nodemask); > + kunit_add_action(test, action_nodemask_free, &nodemask); Why aren't we just freeing the nodemask after using it, before we make any assertions? > + nodes_clear(*nodemask); > + node_set(nid, *nodemask); > + > + page = __alloc_pages_noprof(GFP_KERNEL, 0, nid, nodemask); > + KUNIT_ASSERT_NOT_NULL(test, page); > + return page; > +} > + > +static inline bool page_on_buddy_list(struct page *want_page, struct list_head *head) > +{ > + struct page *found_page; nit: This is just an iterator, we can probably just call it 'page'. 'found' is confusing for me because we didn't really search for it. > + > + list_for_each_entry(found_page, head, buddy_list) { > + if (found_page == want_page) > + return true; > + } > + > + return false; > +} > + > +/* Test case parameters that are independent of alloc order. */ > +static const struct { > + gfp_t gfp_flags; > + enum zone_type want_zone; > +} alloc_fresh_gfps[] = { > + /* > + * The way we currently set up the isolated node, everything ends up in > + * ZONE_NORMAL. > + */ > + { .gfp_flags = GFP_KERNEL, .want_zone = ZONE_NORMAL }, > + { .gfp_flags = GFP_ATOMIC, .want_zone = ZONE_NORMAL }, > + { .gfp_flags = GFP_USER, .want_zone = ZONE_NORMAL }, > + { .gfp_flags = GFP_DMA32, .want_zone = ZONE_NORMAL }, > +}; > + > +struct alloc_fresh_test_case { > + int order; > + int gfp_idx; > +}; > + > +/* Generate test cases as the cross product of orders and alloc_fresh_gfps. */ > +static const void *alloc_fresh_gen_params(const void *prev, char *desc) > +{ > + /* Buffer to avoid allocations. */ > + static struct alloc_fresh_test_case tc; > + > + if (!prev) { > + /* First call */ > + tc.order = 0; > + tc.gfp_idx = 0; > + return &tc; > + } We need to set 'tc' here to whatever 'prev' is pointing at, right? > + > + tc.gfp_idx++; > + if (tc.gfp_idx >= ARRAY_SIZE(alloc_fresh_gfps)) { > + tc.gfp_idx = 0; > + tc.order++; > + } > + if (tc.order > MAX_PAGE_ORDER) > + /* Finished. */ > + return NULL; > + > + snprintf(desc, KUNIT_PARAM_DESC_SIZE, "order %d %pGg\n", > + tc.order, &alloc_fresh_gfps[tc.gfp_idx].gfp_flags); > + return &tc; > +} > + > +/* Smoke test: allocate from a node where everything is in a pristine state. */ > +static void test_alloc_fresh(struct kunit *test) > +{ > + const struct alloc_fresh_test_case *tc = test->param_value; > + gfp_t gfp_flags = alloc_fresh_gfps[tc->gfp_idx].gfp_flags; > + enum zone_type want_zone_type = alloc_fresh_gfps[tc->gfp_idx].want_zone; > + struct zone *want_zone = &NODE_DATA(isolated_node)->node_zones[want_zone_type]; > + struct list_head *buddy_list; > + struct per_cpu_pages *pcp; > + struct page *page, *merged_page; > + int cpu; > + > + page = alloc_pages_force_nid(test, gfp_flags, tc->order, isolated_node); > + > + EXPECT_WITHIN_ZONE(test, page, want_zone); > + > + cpu = get_cpu(); > + __free_pages(page, 0); > + pcp = per_cpu_ptr(want_zone->per_cpu_pageset, cpu); > + put_cpu(); > + > + /* > + * Should end up back in the free area when drained. Because everything > + * is free, it should get buddy-merged up to the maximum order. > + */ > + drain_zone_pages(want_zone, pcp); > + KUNIT_EXPECT_TRUE(test, PageBuddy(page)); > + KUNIT_EXPECT_EQ(test, buddy_order(page), MAX_PAGE_ORDER); > + KUNIT_EXPECT_TRUE(test, list_empty(&pcp->lists[MIGRATE_UNMOVABLE])); > + merged_page = pfn_to_page(round_down(page_to_pfn(page), 1 << MAX_PAGE_ORDER)); > + buddy_list = &want_zone->free_area[MAX_PAGE_ORDER].free_list[MIGRATE_UNMOVABLE]; > + KUNIT_EXPECT_TRUE(test, page_on_buddy_list(merged_page, buddy_list)); > +} > + > static void action_drain_pages_all(void *unused) > { > int cpu; > @@ -144,7 +277,11 @@ static void depopulate_isolated_node(struct kunit_suite *suite) > WARN_ON(add_memory(0, start, size, MMOP_ONLINE)); > WARN_ON(walk_memory_blocks(start, size, NULL, memory_block_online_cb)); > } > -static struct kunit_case test_cases[] = { {} }; > + > +static struct kunit_case test_cases[] = { > + KUNIT_CASE_PARAM(test_alloc_fresh, alloc_fresh_gen_params), > + {} > +}; > > struct kunit_suite page_alloc_test_suite = { > .name = "page_alloc", > > -- > 2.48.1.601.g30ceb7b040-goog >