On Tue, 14 Jul 2020 07:05:34 -0700
Sean Christopherson <sean.j.christopherson@xxxxxxxxx> wrote:
> On Tue, Jul 14, 2020 at 01:41:23PM +0200, Claudio Imbrenda wrote:
> > On Tue, 14 Jul 2020 13:20:16 +0200
> > Thomas Huth <thuth@xxxxxxxxxx> wrote:
> >
> > > On 14/07/2020 13.09, Claudio Imbrenda wrote:
> > > > The assert in lib/alloc_page is hardcoded to long, and size_t is
> > > > just an int on 32 bit architectures.
> > > >
> > > > Adding a cast makes the compiler happy.
> > > >
> > > > Fixes: 73f4b202beb39 ("lib/alloc_page: change some parameter
> > > > types") Signed-off-by: Claudio Imbrenda <imbrenda@xxxxxxxxxxxxx>
> > > > ---
> > > > lib/alloc_page.c | 5 +++--
> > > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/lib/alloc_page.c b/lib/alloc_page.c
> > > > index fa3c527..617b003 100644
> > > > --- a/lib/alloc_page.c
> > > > +++ b/lib/alloc_page.c
> > > > @@ -29,11 +29,12 @@ void free_pages(void *mem, size_t size)
> > > > assert_msg((unsigned long) mem % PAGE_SIZE == 0,
> > > > "mem not page aligned: %p", mem);
> > > >
> > > > - assert_msg(size % PAGE_SIZE == 0, "size not page
> > > > aligned: %#lx", size);
> > > > + assert_msg(size % PAGE_SIZE == 0, "size not page
> > > > aligned: %#lx",
> > > > + (unsigned long)size);
> > > >
> > > > assert_msg(size == 0 || (uintptr_t)mem == -size ||
> > > > (uintptr_t)mem + size > (uintptr_t)mem,
> > > > - "mem + size overflow: %p + %#lx", mem,
> > > > size);
> > > > + "mem + size overflow: %p + %#lx", mem,
> > > > (unsigned long)size);
> > >
> > > Looking at lib/printf.c, it seems like it also supports %z ...
> > > have you tried?
> >
> > no, but in hindsight I should have. It's probably a much cleaner
> > solution. I'll try and respin.
>
> I'm not opposed to using size_t, but if we go that route then the
> entirety of alloc_page.c should be converted to size_t. As is, there
> is code like:
>
> void free_pages_by_order(void *mem, unsigned int order)
> {
> free_pages(mem, 1ul << (order + PAGE_SHIFT));
> }
>
> and
>
> void *alloc_pages(unsigned int order)
> {
> ...
>
> /* Looking for a run of length (1 << order). */
> unsigned long run = 0;
> const unsigned long n = 1ul << order;
> const unsigned long align_mask = (n << PAGE_SHIFT) -
> 1; void *run_start = NULL;
> void *run_prev = NULL;
> unsigned long run_next_pa = 0;
> unsigned long pa;
>
> assert(order < sizeof(unsigned long) * 8);
>
> ...
> }
>
> that very explicitly uses 'unsigned long' for the size.
don't worry, those won't stay there for long :)
once this patch series has stabilized, I'm going to send a more radical
rewrite of the allocators
