On Sat, 3 Oct 2020 10:46:39 +0200 Andrew Jones <drjones@xxxxxxxxxx> wrote: [...] > > + /* the pointer is page-aligned, it was a multi-page > > allocation */ > > + m = GET_METADATA(mem); > > + assert(m->magic == VM_MAGIC); > > + assert(m->npages > 0); > > + /* free all the pages including the metadata page */ > > + ptr = (uintptr_t)mem - PAGE_SIZE; > > + end = ptr + m->npages * PAGE_SIZE; > > + for ( ; ptr < end; ptr += PAGE_SIZE) > > + free_page(phys_to_virt(virt_to_pte_phys(page_root, > > (void *)ptr))); > > + /* free the last one separately to avoid overflow issues */ > > + free_page(phys_to_virt(virt_to_pte_phys(page_root, (void > > *)ptr))); > > I don't get this. How is > > for (p = start; p < end; p += step) > process(p); > process(p) > > different from > > for (p = start; p <= end; p += step) > process(p); there was a reason at some point, I think the code evolved past it and these lines stayed there as is > To avoid overflow issues we should simple ensure start and end are > computed correctly. Also, I'd prefer 'end' point to the actual end, > not the last included page, e.g. start=0x1000, end=0x1fff. Then we > have > > start = get_start(); > assert(PAGE_ALIGN(start) == start); > end = start + nr_pages * PAGE_SIZE - 1; > assert(start < end); > for (p = start; start < end; p += PAGE_SIZE) > process(p); > > Thanks, > drew yeah I'll definitely fix it
