| Message ID | 20200714042046.13419-1-sean.j.christopherson@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [kvm-unit-tests] lib/alloc_page: Revert to 'unsigned long' for @size params | expand |
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>
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>
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...
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; }
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(-)