[kvm-unit-tests PATCH] lib/alloc_page: Revert to 'unsigned long' for @size params

[kvm-unit-tests PATCH] lib/alloc_page: Revert to 'unsigned long' for @size params

[Sean Christopherson]

Revert to using 'unsigned long' instead of 'size_t' for free_pages() and
get_order().  The recent change to size_t for free_pages() breaks i386
with -Werror as the assert_msg() formats expect unsigned longs, whereas
size_t is an 'unsigned int' on i386 (though both longs and ints are 4
bytes).

Message formatting aside, unsigned long is the correct choice given the
current code base as alloc_pages() and free_pages_by_order() explicitly
expect, work on, and/or assert on the size being an unsigned long.

Fixes: 73f4b202beb39 ("lib/alloc_page: change some parameter types")
Cc: Claudio Imbrenda <imbrenda@linux.ibm.com>
Cc: Andrew Jones <drjones@redhat.com>
Cc: Jim Mattson <jmattson@google.com>
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
 lib/alloc_page.c | 2 +-
 lib/alloc_page.h | 2 +-
 lib/bitops.h     | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/alloc_page.c b/lib/alloc_page.c
index fa3c527..aa98981 100644
--- a/lib/alloc_page.c
+++ b/lib/alloc_page.c
@@ -21,7 +21,7 @@ bool page_alloc_initialized(void)
 	return freelist != 0;
 }
 
-void free_pages(void *mem, size_t size)
+void free_pages(void *mem, unsigned long size)
 {
 	void *old_freelist;
 	void *end;
diff --git a/lib/alloc_page.h b/lib/alloc_page.h
index 88540d1..80eded7 100644
--- a/lib/alloc_page.h
+++ b/lib/alloc_page.h
@@ -13,7 +13,7 @@ void page_alloc_ops_enable(void);
 void *alloc_page(void);
 void *alloc_pages(unsigned int order);
 void free_page(void *page);
-void free_pages(void *mem, size_t size);
+void free_pages(void *mem, unsigned long size);
 void free_pages_by_order(void *mem, unsigned int order);
 
 #endif
diff --git a/lib/bitops.h b/lib/bitops.h
index 308aa86..dd015e8 100644
--- a/lib/bitops.h
+++ b/lib/bitops.h
@@ -79,7 +79,7 @@ static inline bool is_power_of_2(unsigned long n)
 	return n && !(n & (n - 1));
 }
 
-static inline unsigned int get_order(size_t size)
+static inline unsigned int get_order(unsigned long size)
 {
 	return size ? fls(size) + !is_power_of_2(size) : 0;
 }
-- 
2.26.0

Re: [kvm-unit-tests PATCH] lib/alloc_page: Revert to 'unsigned long' for @size params

[Andrew Jones]

On Mon, Jul 13, 2020 at 09:20:46PM -0700, Sean Christopherson wrote:
> Revert to using 'unsigned long' instead of 'size_t' for free_pages() and
> get_order().  The recent change to size_t for free_pages() breaks i386
> with -Werror as the assert_msg() formats expect unsigned longs, whereas
> size_t is an 'unsigned int' on i386 (though both longs and ints are 4
> bytes).
> 
> Message formatting aside, unsigned long is the correct choice given the
> current code base as alloc_pages() and free_pages_by_order() explicitly
> expect, work on, and/or assert on the size being an unsigned long.
> 
> Fixes: 73f4b202beb39 ("lib/alloc_page: change some parameter types")
> Cc: Claudio Imbrenda <imbrenda@linux.ibm.com>
> Cc: Andrew Jones <drjones@redhat.com>
> Cc: Jim Mattson <jmattson@google.com>
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> ---
>  lib/alloc_page.c | 2 +-
>  lib/alloc_page.h | 2 +-
>  lib/bitops.h     | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
>

Fixes compilation on arm32.

Reviewed-by: Andrew Jones <drjones@redhat.com>

Re: [kvm-unit-tests PATCH] lib/alloc_page: Revert to 'unsigned long' for @size params

[Thomas Huth]

On 14/07/2020 06.20, Sean Christopherson wrote:
> Revert to using 'unsigned long' instead of 'size_t' for free_pages() and
> get_order().  The recent change to size_t for free_pages() breaks i386
> with -Werror as the assert_msg() formats expect unsigned longs, whereas
> size_t is an 'unsigned int' on i386 (though both longs and ints are 4
> bytes).
> 
> Message formatting aside, unsigned long is the correct choice given the
> current code base as alloc_pages() and free_pages_by_order() explicitly
> expect, work on, and/or assert on the size being an unsigned long.
> 
> Fixes: 73f4b202beb39 ("lib/alloc_page: change some parameter types")
> Cc: Claudio Imbrenda <imbrenda@linux.ibm.com>
> Cc: Andrew Jones <drjones@redhat.com>
> Cc: Jim Mattson <jmattson@google.com>
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> ---
[...]
> diff --git a/lib/bitops.h b/lib/bitops.h
> index 308aa86..dd015e8 100644
> --- a/lib/bitops.h
> +++ b/lib/bitops.h
> @@ -79,7 +79,7 @@ static inline bool is_power_of_2(unsigned long n)
>  	return n && !(n & (n - 1));
>  }
>  
> -static inline unsigned int get_order(size_t size)
> +static inline unsigned int get_order(unsigned long size)
>  {
>  	return size ? fls(size) + !is_power_of_2(size) : 0;
>  }
> 

get_order() already used size_t when it was introduced in commit
f22e527df02ffaba ... is it necessary to switch it to unsigned long now?

Apart from that, this patch fixes the compilation problems, indeed, I
just checked it in the travis-CI.

Tested-by: Thomas Huth <thuth@redhat.com>

Re: [kvm-unit-tests PATCH] lib/alloc_page: Revert to 'unsigned long' for @size params

[Claudio Imbrenda]

On Tue, 14 Jul 2020 09:12:52 +0200
Thomas Huth <thuth@redhat.com> wrote:

> On 14/07/2020 06.20, Sean Christopherson wrote:
> > Revert to using 'unsigned long' instead of 'size_t' for
> > free_pages() and get_order().  The recent change to size_t for
> > free_pages() breaks i386 with -Werror as the assert_msg() formats
> > expect unsigned longs, whereas size_t is an 'unsigned int' on i386
> > (though both longs and ints are 4 bytes).
> > 
> > Message formatting aside, unsigned long is the correct choice given
> > the current code base as alloc_pages() and free_pages_by_order()
> > explicitly expect, work on, and/or assert on the size being an
> > unsigned long.
> > 
> > Fixes: 73f4b202beb39 ("lib/alloc_page: change some parameter types")
> > Cc: Claudio Imbrenda <imbrenda@linux.ibm.com>
> > Cc: Andrew Jones <drjones@redhat.com>
> > Cc: Jim Mattson <jmattson@google.com>
> > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> > ---  
> [...]
> > diff --git a/lib/bitops.h b/lib/bitops.h
> > index 308aa86..dd015e8 100644
> > --- a/lib/bitops.h
> > +++ b/lib/bitops.h
> > @@ -79,7 +79,7 @@ static inline bool is_power_of_2(unsigned long n)
> >  	return n && !(n & (n - 1));
> >  }
> >  
> > -static inline unsigned int get_order(size_t size)
> > +static inline unsigned int get_order(unsigned long size)
> >  {
> >  	return size ? fls(size) + !is_power_of_2(size) : 0;
> >  }
> >   
> 
> get_order() already used size_t when it was introduced in commit
> f22e527df02ffaba ... is it necessary to switch it to unsigned long
> now?
> 
> Apart from that, this patch fixes the compilation problems, indeed, I
> just checked it in the travis-CI.
> 
> Tested-by: Thomas Huth <thuth@redhat.com>

Yeah I don't think there is any reason to change get_order...