Re: [kvm-unit-tests PATCH v1 12/12] lib/alloc_page: default flags and zero pages by default

[Claudio Imbrenda <imbrenda@linux.ibm.com>]

On Thu, 24 Dec 2020 10:17:30 -0800
Krish Sadhukhan <krish.sadhukhan@oracle.com> wrote:

> On 12/16/20 12:12 PM, Claudio Imbrenda wrote:
> > The new function page_alloc_set_default_flags can be used to set the
> > default flags for allocations. The passed value will be ORed with
> > the flags argument passed to the allocator at each allocation.
> >
> > The default value for the default flags is FLAG_ZERO, which means
> > that by default all allocated memory is now zeroed, restoring the
> > default behaviour that had been accidentally removed by a previous
> > commit.
> >
> > If needed, a testcase can call page_alloc_set_default_flags(0) in
> > order to get non-zeroed pages from the allocator. For example, if
> > the testcase will need fresh memory, the zero flag should be
> > removed from the default.
> >
> > Fixes: 8131e91a4b61 ("lib/alloc_page: complete rewrite of the page
> > allocator") Reported-by: Nadav Amit<nadav.amit@gmail.com>
> >
> > Signed-off-by: Claudio Imbrenda<imbrenda@linux.ibm.com>
> > ---
> >   lib/alloc_page.h | 3 +++
> >   lib/alloc_page.c | 8 ++++++++
> >   2 files changed, 11 insertions(+)
> >
> > diff --git a/lib/alloc_page.h b/lib/alloc_page.h
> > index 1039814..8b53a58 100644
> > --- a/lib/alloc_page.h
> > +++ b/lib/alloc_page.h
> > @@ -22,6 +22,9 @@
> >   /* Returns true if the page allocator has been initialized */
> >   bool page_alloc_initialized(void);
> >   
> > +/* Sets the default flags for the page allocator, the default is
> > FLAG_ZERO */ +void page_alloc_set_default_flags(unsigned int flags);
> > +
> >   /*
> >    * Initializes a memory area.
> >    * n is the number of the area to initialize
> > diff --git a/lib/alloc_page.c b/lib/alloc_page.c
> > index 4d5722f..08e0d05 100644
> > --- a/lib/alloc_page.c
> > +++ b/lib/alloc_page.c
> > @@ -54,12 +54,19 @@ static struct mem_area areas[MAX_AREAS];
> >   static unsigned int areas_mask;
> >   /* Protects areas and areas mask */
> >   static struct spinlock lock;
> > +/* Default behaviour: zero allocated pages */
> > +static unsigned int default_flags = FLAG_ZERO;
> >   
> >   bool page_alloc_initialized(void)
> >   {
> >   	return areas_mask != 0;
> >   }
> >   
> > +void page_alloc_set_default_flags(unsigned int flags)
> > +{
> > +	default_flags = flags;  
> 
> 
> Who calls this functions ?

nobody yet, since I have just introduced them.

The idea is that a testcase should call this early on to set the default

> Just wondering if default flag should be a static set of flag values 
> which the caller can override based on needs rather than the caller 
> setting the default flag.

hmmmmm

I would only need to reverse the semantics of FLAG_ZERO, but then I can
get rid of this patch

I think I'll do it
 
> > +}
> > +
> >   /*
> >    * Each memory area contains an array of metadata entries at the
> > very
> >    * beginning. The usable memory follows immediately afterwards.
> > @@ -394,6 +401,7 @@ static void *page_memalign_order_flags(u8 ord,
> > u8 al, u32 flags) void *res = NULL;
> >   	int i, area, fresh;
> >   
> > +	flags |= default_flags;
> >   	fresh = !!(flags & FLAG_FRESH);
> >   	spin_lock(&lock);
> >   	area = (flags & AREA_MASK) ? flags & areas_mask :
> > areas_mask;