Date:   Fri, 24 Feb 2023 16:52:26 +0200
From:   Mike Rapoport <rppt@kernel.org>
To:     Claudio Migliorelli <claudio.migliorelli@mail.polimi.it>
Cc:     remckee0@gmail.com, david@redhat.com, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] Add tests for memblock_alloc_node()
Message-ID: <Y/jPKvcIpqi1zVgC@kernel.org>
References: <59d4745b-7b2-bf6-7b8-f6571d78d336@mail.polimi.it>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <59d4745b-7b2-bf6-7b8-f6571d78d336@mail.polimi.it>
Precedence: bulk

Hi Claudio,

On Sun, Feb 19, 2023 at 05:28:46PM +0100, Claudio Migliorelli wrote:
> This test is aimed at verifying the memblock_alloc_node() to work as
> expected, so setting the correct NUMA node for the new allocated
> region. The memblock_alloc_node() is called directly without using any
> stub. The core check is between the requested NUMA node and the `nid`
> field inside the memblock_region structure. These two are supposed to
> be equal for the test to succeed.
> 
> Signed-off-by: Claudio Migliorelli <claudio.migliorelli@mail.polimi.it>
> ---
> Changelog:
> ----------
> v2:
> 	- Use the memblock_alloc_node() directly without mimicking it
> 
>  tools/testing/memblock/tests/alloc_nid_api.c | 40 ++++++++++++++++++++
>  1 file changed, 40 insertions(+)

When I tried to apply your patch I've got these errors:

<stdin>:188: trailing whitespace.
 
error: patch failed: tools/testing/memblock/tests/alloc_nid_api.c:2494
error: tools/testing/memblock/tests/alloc_nid_api.c: patch does not apply

> diff --git a/tools/testing/memblock/tests/alloc_nid_api.c b/tools/testing/memblock/tests/alloc_nid_api.c
> index 49ef68cccd6f..975a5317abf3 100644
> --- a/tools/testing/memblock/tests/alloc_nid_api.c
> +++ b/tools/testing/memblock/tests/alloc_nid_api.c
> @@ -2494,6 +2494,35 @@ static int alloc_nid_numa_split_all_reserved_generic_check(void)
>  	return 0;
>  }
> 
> +/*
> + * A simple test that tries to allocate a memory region through the
> + * memblock_alloc_node() on a NUMA node with id `nid`. Expected to have the
> + * correct NUMA node set for the new region.
> + */
> +static int alloc_node_on_correct_nid(void)
> +{
> +	int nid_req = 2;
> +	void *allocated_ptr = NULL;
> +#ifdef CONFIG_NUMA
> +	struct memblock_region *req_node = &memblock.memory.regions[nid_req];
> +#endif
> +	phys_addr_t size = SZ_512;
> +
> +	PREFIX_PUSH();
> +	setup_numa_memblock(node_fractions);
> + +	allocated_ptr = memblock_alloc_node(size, SMP_CACHE_BYTES, nid_req);
> +
> +	ASSERT_NE(allocated_ptr, NULL);
> +#ifdef CONFIG_NUMA
> +	ASSERT_EQ(nid_req, req_node->nid);
> +#endif
> +
> +	test_pass_pop();
> +
> +	return 0;
> +}
> +
>  /* Test case wrappers for NUMA tests */
>  static int alloc_nid_numa_simple_check(void)
>  {
> @@ -2632,6 +2661,15 @@ static int alloc_nid_numa_split_all_reserved_check(void)
>  	return 0;
>  }
> 
> +static int alloc_node_numa_on_correct_nid(void)
> +{
> +	test_print("\tRunning %s...\n", __func__);
> +	run_top_down(alloc_node_on_correct_nid);
> +	run_bottom_up(alloc_node_on_correct_nid);
> +
> +	return 0;
> +}
> +
>  int __memblock_alloc_nid_numa_checks(void)
>  {
>  	test_print("Running %s NUMA tests...\n",
> @@ -2652,6 +2690,8 @@ int __memblock_alloc_nid_numa_checks(void)
>  	alloc_nid_numa_reserved_full_merge_check();
>  	alloc_nid_numa_split_all_reserved_check();
> 
> +	alloc_node_numa_on_correct_nid();
> +
>  	return 0;
>  }
> 
> -- 
> 2.38.3

-- 
Sincerely yours,
Mike.