All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rebecca Mckeever <remckee0@gmail.com>
To: "Huang, Shaoqin" <shaoqin.huang@intel.com>
Cc: Mike Rapoport <rppt@kernel.org>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	David Hildenbrand <david@redhat.com>
Subject: Re: [PATCH 1/8] memblock tests: update tests to check if memblock_alloc zeroed memory
Date: Mon, 15 Aug 2022 21:15:22 -0500	[thread overview]
Message-ID: <20220816021522.GA3590@sophie> (raw)
In-Reply-To: <5dde66a7-2236-a890-aaeb-a45876057bec@intel.com>

On Mon, Aug 15, 2022 at 04:50:51PM +0800, Huang, Shaoqin wrote:
> Hi, Rebecca.
> 
> On 8/14/2022 1:53 PM, Rebecca Mckeever wrote:
> > Add an assert in memblock_alloc() tests where allocation is expected to
> > occur. The assert checks whether the entire chunk of allocated memory is
> > cleared.
> > 
> > The current memblock_alloc() tests do not check whether the allocated
> > memory was zeroed. memblock_alloc() should zero the allocated memory since
> > it is a wrapper for memblock_alloc_try_nid().
> > 
> > Signed-off-by: Rebecca Mckeever <remckee0@gmail.com>
> > ---
> >   tools/testing/memblock/tests/alloc_api.c | 24 ++++++++++++++++++++++++
> >   tools/testing/memblock/tests/common.c    |  7 +++++++
> >   tools/testing/memblock/tests/common.h    | 13 +++++++++++++
> >   3 files changed, 44 insertions(+)
> > 
> > diff --git a/tools/testing/memblock/tests/alloc_api.c b/tools/testing/memblock/tests/alloc_api.c
> > index a14f38eb8a89..71c89cb9b2a8 100644
> > --- a/tools/testing/memblock/tests/alloc_api.c
> > +++ b/tools/testing/memblock/tests/alloc_api.c
> > @@ -22,6 +22,8 @@ static int alloc_top_down_simple_check(void)
> >   	allocated_ptr = memblock_alloc(size, SMP_CACHE_BYTES);
> >   	ASSERT_NE(allocated_ptr, NULL);
> > +	ASSERT_MEM_EQ((char *)allocated_ptr, 0, size);
> > +
> >   	ASSERT_EQ(rgn->size, size);
> >   	ASSERT_EQ(rgn->base, expected_start);
> > @@ -80,6 +82,8 @@ static int alloc_top_down_disjoint_check(void)
> >   	allocated_ptr = memblock_alloc(r2_size, alignment);
> >   	ASSERT_NE(allocated_ptr, NULL);
> > +	ASSERT_MEM_EQ((char *)allocated_ptr, 0, r2_size);
> > +
> >   	ASSERT_EQ(rgn1->size, r1.size);
> >   	ASSERT_EQ(rgn1->base, r1.base);
> > @@ -125,6 +129,8 @@ static int alloc_top_down_before_check(void)
> >   	allocated_ptr = memblock_alloc(r2_size, SMP_CACHE_BYTES);
> >   	ASSERT_NE(allocated_ptr, NULL);
> > +	ASSERT_MEM_EQ((char *)allocated_ptr, 0, r2_size);
> > +
> >   	ASSERT_EQ(rgn->size, total_size);
> >   	ASSERT_EQ(rgn->base, memblock_end_of_DRAM() - total_size);
> > @@ -173,6 +179,8 @@ static int alloc_top_down_after_check(void)
> >   	allocated_ptr = memblock_alloc(r2_size, SMP_CACHE_BYTES);
> >   	ASSERT_NE(allocated_ptr, NULL);
> > +	ASSERT_MEM_EQ((char *)allocated_ptr, 0, r2_size);
> > +
> >   	ASSERT_EQ(rgn->size, total_size);
> >   	ASSERT_EQ(rgn->base, r1.base - r2_size);
> > @@ -223,6 +231,8 @@ static int alloc_top_down_second_fit_check(void)
> >   	allocated_ptr = memblock_alloc(r3_size, SMP_CACHE_BYTES);
> >   	ASSERT_NE(allocated_ptr, NULL);
> > +	ASSERT_MEM_EQ((char *)allocated_ptr, 0, r3_size);
> > +
> >   	ASSERT_EQ(rgn->size, r2.size + r3_size);
> >   	ASSERT_EQ(rgn->base, r2.base - r3_size);
> > @@ -277,6 +287,8 @@ static int alloc_in_between_generic_check(void)
> >   	allocated_ptr = memblock_alloc(r3_size, SMP_CACHE_BYTES);
> >   	ASSERT_NE(allocated_ptr, NULL);
> > +	ASSERT_MEM_EQ((char *)allocated_ptr, 0, r3_size);
> > +
> >   	ASSERT_EQ(rgn->size, total_size);
> >   	ASSERT_EQ(rgn->base, r1.base - r2.size - r3_size);
> > @@ -418,6 +430,8 @@ static int alloc_limited_space_generic_check(void)
> >   	allocated_ptr = memblock_alloc(available_size, SMP_CACHE_BYTES);
> >   	ASSERT_NE(allocated_ptr, NULL);
> > +	ASSERT_MEM_EQ((char *)allocated_ptr, 0, available_size);
> > +
> >   	ASSERT_EQ(rgn->size, MEM_SIZE);
> >   	ASSERT_EQ(rgn->base, memblock_start_of_DRAM());
> > @@ -442,6 +456,7 @@ static int alloc_no_memory_generic_check(void)
> >   	PREFIX_PUSH();
> >   	reset_memblock_regions();
> > +	fill_memblock();
> 
> Maybe we don't need this line, it has no effect at here. Anyway, Others
> Looks Good to me.
> 
Do you mean that it's not necessary since the test doesn't run
verify_mem_content()? 

> Reviewed-by: Shaoqin Huang <shaoqin.huang@intel.com>
> 
> >   	allocated_ptr = memblock_alloc(SZ_1K, SMP_CACHE_BYTES);
> > @@ -472,6 +487,8 @@ static int alloc_bottom_up_simple_check(void)
> >   	allocated_ptr = memblock_alloc(SZ_2, SMP_CACHE_BYTES);
> >   	ASSERT_NE(allocated_ptr, NULL);
> > +	ASSERT_MEM_EQ((char *)allocated_ptr, 0, SZ_2);
> > +
> >   	ASSERT_EQ(rgn->size, SZ_2);
> >   	ASSERT_EQ(rgn->base, memblock_start_of_DRAM());
> > @@ -528,6 +545,7 @@ static int alloc_bottom_up_disjoint_check(void)
> >   	allocated_ptr = memblock_alloc(r2_size, alignment);
> >   	ASSERT_NE(allocated_ptr, NULL);
> > +	ASSERT_MEM_EQ((char *)allocated_ptr, 0, r2_size);
> >   	ASSERT_EQ(rgn1->size, r1.size);
> >   	ASSERT_EQ(rgn1->base, r1.base);
> > @@ -571,6 +589,8 @@ static int alloc_bottom_up_before_check(void)
> >   	allocated_ptr = memblock_alloc(r1_size, SMP_CACHE_BYTES);
> >   	ASSERT_NE(allocated_ptr, NULL);
> > +	ASSERT_MEM_EQ((char *)allocated_ptr, 0, r1_size);
> > +
> >   	ASSERT_EQ(rgn->size, total_size);
> >   	ASSERT_EQ(rgn->base, memblock_start_of_DRAM());
> > @@ -618,6 +638,8 @@ static int alloc_bottom_up_after_check(void)
> >   	allocated_ptr = memblock_alloc(r2_size, SMP_CACHE_BYTES);
> >   	ASSERT_NE(allocated_ptr, NULL);
> > +	ASSERT_MEM_EQ((char *)allocated_ptr, 0, r2_size);
> > +
> >   	ASSERT_EQ(rgn->size, total_size);
> >   	ASSERT_EQ(rgn->base, r1.base);
> > @@ -669,6 +691,8 @@ static int alloc_bottom_up_second_fit_check(void)
> >   	allocated_ptr = memblock_alloc(r3_size, SMP_CACHE_BYTES);
> >   	ASSERT_NE(allocated_ptr, NULL);
> > +	ASSERT_MEM_EQ((char *)allocated_ptr, 0, r3_size);
> > +
> >   	ASSERT_EQ(rgn->size, r2.size + r3_size);
> >   	ASSERT_EQ(rgn->base, r2.base);
> > diff --git a/tools/testing/memblock/tests/common.c b/tools/testing/memblock/tests/common.c
> > index 76a8ad818f3a..0ca26fe12c38 100644
> > --- a/tools/testing/memblock/tests/common.c
> > +++ b/tools/testing/memblock/tests/common.c
> > @@ -60,16 +60,23 @@ void reset_memblock_attributes(void)
> >   	memblock.current_limit	= MEMBLOCK_ALLOC_ANYWHERE;
> >   }
> > +void fill_memblock(void)
> > +{
> > +	memset(memory_block.base, 1, MEM_SIZE);
> > +}
> > +
> >   void setup_memblock(void)
> >   {
> >   	reset_memblock_regions();
> >   	memblock_add((phys_addr_t)memory_block.base, MEM_SIZE);
> > +	fill_memblock();
> >   }
> >   void dummy_physical_memory_init(void)
> >   {
> >   	memory_block.base = malloc(MEM_SIZE);
> >   	assert(memory_block.base);
> > +	fill_memblock();
> >   }
> >   void dummy_physical_memory_cleanup(void)
> > diff --git a/tools/testing/memblock/tests/common.h b/tools/testing/memblock/tests/common.h
> > index d396e5423a8e..7a16a7dc8f2c 100644
> > --- a/tools/testing/memblock/tests/common.h
> > +++ b/tools/testing/memblock/tests/common.h
> > @@ -51,6 +51,18 @@
> >   	assert((_expected) < (_seen)); \
> >   } while (0)
> > +/**
> > + * ASSERT_MEM_EQ():
> > + * Check that the first @_size bytes of @_seen are all equal to @_expected.
> > + * If false, print failed test message (if running with --verbose) and then
> > + * assert.
> > + */
> > +#define ASSERT_MEM_EQ(_seen, _expected, _size) do { \
> > +	for (int _i = 0; _i < (_size); _i++) { \
> > +		ASSERT_EQ((_seen)[_i], (_expected)); \
> > +	} \
> > +} while (0)
> > +
> >   #define PREFIX_PUSH() prefix_push(__func__)
> >   /*
> > @@ -70,6 +82,7 @@ struct region {
> >   void reset_memblock_regions(void);
> >   void reset_memblock_attributes(void);
> > +void fill_memblock(void);
> >   void setup_memblock(void);
> >   void dummy_physical_memory_init(void);
> >   void dummy_physical_memory_cleanup(void);


  reply	other threads:[~2022-08-16  2:15 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-14  5:53 [PATCH 0/8] memblock tests: update and extend memblock simulator Rebecca Mckeever
2022-08-14  5:53 ` [PATCH 1/8] memblock tests: update tests to check if memblock_alloc zeroed memory Rebecca Mckeever
2022-08-15  8:50   ` Huang, Shaoqin
2022-08-16  2:15     ` Rebecca Mckeever [this message]
2022-08-16  2:24       ` Huang, Shaoqin
2022-08-14  5:53 ` [PATCH 2/8] memblock tests: update zeroed memory check for memblock_alloc_* tests Rebecca Mckeever
2022-08-15  8:59   ` Huang, Shaoqin
2022-08-14  5:53 ` [PATCH 3/8] memblock tests: add labels to verbose output for generic alloc tests Rebecca Mckeever
2022-08-15  9:15   ` Huang, Shaoqin
2022-08-16  2:39     ` Rebecca Mckeever
2022-08-16  3:08       ` Huang, Shaoqin
2022-08-19  5:03         ` Rebecca Mckeever
2022-08-19  8:39           ` Huang, Shaoqin
2022-08-14  5:53 ` [PATCH 4/8] memblock tests: add additional tests for basic api and memblock_alloc Rebecca Mckeever
2022-08-15 10:19   ` Huang, Shaoqin
2022-08-19  6:10     ` Rebecca Mckeever
2022-08-14  5:53 ` [PATCH 5/8] memblock tests: update alloc_api to test memblock_alloc_raw Rebecca Mckeever
2022-08-14  5:53 ` [PATCH 6/8] memblock tests: update alloc_nid_api to test memblock_alloc_try_nid_raw Rebecca Mckeever
2022-08-16  1:24   ` Huang, Shaoqin
2022-08-19  6:22     ` Rebecca Mckeever
2022-08-14  5:53 ` [PATCH 7/8] memblock tests: add tests for memblock_*bottom_up functions Rebecca Mckeever
2022-08-16  1:46   ` Huang, Shaoqin
2022-08-14  5:54 ` [PATCH 8/8] memblock tests: add tests for memblock_trim_memory Rebecca Mckeever
2022-08-16  1:46   ` Huang, Shaoqin
2022-08-20  8:40     ` Rebecca Mckeever

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=20220816021522.GA3590@sophie \
    --to=remckee0@gmail.com \
    --cc=david@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=rppt@kernel.org \
    --cc=shaoqin.huang@intel.com \
    /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.