Re: [kvm-unit-tests PATCH v2 4/6] x86: lib/alloc: move heap management to lib

[Andrew Jones <drjones@redhat.com>]

On Thu, Nov 03, 2016 at 02:28:15PM +0100, Laurent Vivier wrote:
> 
> 
> On 02/11/2016 21:52, Andrew Jones wrote:
> > This will allow other arches to use {alloc,free}_page.
> > 
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > ---
> >  lib/alloc.c         | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
> >  lib/alloc.h         | 10 ++++++++++
> >  lib/x86/vm.c        | 48 ++----------------------------------------------
> >  x86/Makefile.common |  1 +
> >  4 files changed, 62 insertions(+), 46 deletions(-)
> > 
> > diff --git a/lib/alloc.c b/lib/alloc.c
> > index 1d990a803825..ce1198e2977f 100644
> > --- a/lib/alloc.c
> > +++ b/lib/alloc.c
> > @@ -5,6 +5,7 @@
> >   */
> >  #include "alloc.h"
> >  #include "asm/spinlock.h"
> > +#include "asm/page.h"
> >  #include "asm/io.h"
> >  
> >  #define MIN(a, b)		((a) < (b) ? (a) : (b))
> > @@ -150,6 +151,54 @@ phys_addr_t phys_zalloc(phys_addr_t size)
> >  	return phys_zalloc_aligned(size, phys_alloc_align_min);
> >  }
> >  
> > +static struct spinlock heap_lock;
> > +static void *heap_free_head;
> > +
> > +void heap_init(void *start, size_t size)
> > +{
> > +	void *p = start;
> 
> why do you introduce "p"? It's not obvious for me...

Just naming. I prefer the input to be named start,
but the iterator to be named p.

> 
> > +
> > +	assert(!((unsigned long)start & ~PAGE_MASK));
> > +
> > +	spin_lock(&heap_lock);
> > +
> > +	heap_free_head = NULL;
> > +
> > +	while (size >= PAGE_SIZE) {
> > +		*(void **)p = heap_free_head;
> > +		heap_free_head = p;
> > +		p += PAGE_SIZE;
> > +		size -= PAGE_SIZE;
> > +	}
> > +
> > +	spin_unlock(&heap_lock);
> > +}
> > +
> > +void *alloc_page(void)
> > +{
> > +	void *p;
> > +
> > +	spin_lock(&heap_lock);
> > +
> > +	if (!heap_free_head)
> > +		return NULL;
> 
> missing unlock propagated from PATCH 1

Yup. Already fixed for v3

Thanks,
drew

> 
> Laurent
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html