| Message ID | 20200703115109.39139-5-imbrenda@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | More lib/alloc cleanup and a minor improvement | expand |
On Fri, Jul 03, 2020 at 01:51:09PM +0200, Claudio Imbrenda wrote: > Allow allocating aligned virtual memory with alignment larger than only > one page. > > Add a check that the backing pages were actually allocated. > > Export the alloc_vpages_aligned function to allow users to allocate > non-backed aligned virtual addresses. > > Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > --- > lib/vmalloc.h | 3 +++ > lib/vmalloc.c | 40 ++++++++++++++++++++++++++++++++-------- > 2 files changed, 35 insertions(+), 8 deletions(-) > > diff --git a/lib/vmalloc.h b/lib/vmalloc.h > index 2b563f4..fa3fa22 100644 > --- a/lib/vmalloc.h > +++ b/lib/vmalloc.h > @@ -5,6 +5,9 @@ > > /* Allocate consecutive virtual pages (without backing) */ > extern void *alloc_vpages(ulong nr); > +/* Allocate consecutive and aligned virtual pages (without backing) */ > +extern void *alloc_vpages_aligned(ulong nr, unsigned int alignment_pages); > + > /* Allocate one virtual page (without backing) */ > extern void *alloc_vpage(void); > /* Set the top of the virtual address space */ > diff --git a/lib/vmalloc.c b/lib/vmalloc.c > index 9237a0f..2c482aa 100644 > --- a/lib/vmalloc.c > +++ b/lib/vmalloc.c > @@ -12,19 +12,28 @@ > #include "alloc.h" > #include "alloc_phys.h" > #include "alloc_page.h" > +#include <bitops.h> > #include "vmalloc.h" > > static struct spinlock lock; > static void *vfree_top = 0; > static void *page_root; > > -void *alloc_vpages(ulong nr) > +/* > + * Allocate a certain number of pages from the virtual address space (without > + * physical backing). > + * > + * nr is the number of pages to allocate > + * alignment_pages is the alignment of the allocation *in pages* > + */ > +static void *alloc_vpages_intern(ulong nr, unsigned int alignment_pages) This helper function isn't necessary. Just introduce alloc_vpages_aligned() and then call alloc_vpages_aligned(nr, 1) from alloc_vpages(). > { > uintptr_t ptr; > > spin_lock(&lock); > ptr = (uintptr_t)vfree_top; > ptr -= PAGE_SIZE * nr; > + ptr &= GENMASK_ULL(63, PAGE_SHIFT + get_order(alignment_pages)); > vfree_top = (void *)ptr; > spin_unlock(&lock); > > @@ -32,6 +41,16 @@ void *alloc_vpages(ulong nr) > return (void *)ptr; > } > > +void *alloc_vpages(ulong nr) > +{ > + return alloc_vpages_intern(nr, 1); > +} > + > +void *alloc_vpages_aligned(ulong nr, unsigned int alignment_pages) > +{ > + return alloc_vpages_intern(nr, alignment_pages); > +} > + > void *alloc_vpage(void) > { > return alloc_vpages(1); > @@ -55,17 +74,22 @@ void *vmap(phys_addr_t phys, size_t size) > return mem; > } > > +/* > + * Allocate virtual memory, with the specified minimum alignment. > + */ > static void *vm_memalign(size_t alignment, size_t size) > { > + phys_addr_t pa; > void *mem, *p; > - size_t pages; > > - assert(alignment <= PAGE_SIZE); > - size = PAGE_ALIGN(size); > - pages = size / PAGE_SIZE; > - mem = p = alloc_vpages(pages); > - while (pages--) { > - phys_addr_t pa = virt_to_phys(alloc_page()); > + assert(is_power_of_2(alignment)); > + > + size = PAGE_ALIGN(size) / PAGE_SIZE; > + alignment = PAGE_ALIGN(alignment) / PAGE_SIZE; > + mem = p = alloc_vpages_intern(size, alignment); > + while (size--) { > + pa = virt_to_phys(alloc_page()); > + assert(pa); > install_page(page_root, pa, p); > p += PAGE_SIZE; > } > -- > 2.26.2 > Otherwise Reviewed-by: Andrew Jones <drjones@redhat.com> Thanks, drew
On Fri, 3 Jul 2020 14:30:01 +0200 Andrew Jones <drjones@redhat.com> wrote: [...] > > -void *alloc_vpages(ulong nr) > > +/* > > + * Allocate a certain number of pages from the virtual address > > space (without > > + * physical backing). > > + * > > + * nr is the number of pages to allocate > > + * alignment_pages is the alignment of the allocation *in pages* > > + */ > > +static void *alloc_vpages_intern(ulong nr, unsigned int > > alignment_pages) > > This helper function isn't necessary. Just introduce > alloc_vpages_aligned() and then call alloc_vpages_aligned(nr, 1) from > alloc_vpages(). the helper will actually be useful in future patches. maybe I should have written that in the patch description. I can respin without helper if you prefer (and introduce it when needed) or simply update the patch description. > > { > > uintptr_t ptr; > > > > spin_lock(&lock); > > ptr = (uintptr_t)vfree_top; > > ptr -= PAGE_SIZE * nr; > > + ptr &= GENMASK_ULL(63, PAGE_SHIFT + > > get_order(alignment_pages)); vfree_top = (void *)ptr; > > spin_unlock(&lock); > > > > @@ -32,6 +41,16 @@ void *alloc_vpages(ulong nr) > > return (void *)ptr; > > } > > > > +void *alloc_vpages(ulong nr) > > +{ > > + return alloc_vpages_intern(nr, 1); > > +} > > + > > +void *alloc_vpages_aligned(ulong nr, unsigned int alignment_pages) > > +{ > > + return alloc_vpages_intern(nr, alignment_pages); > > +} > > + > > void *alloc_vpage(void) > > { > > return alloc_vpages(1); > > @@ -55,17 +74,22 @@ void *vmap(phys_addr_t phys, size_t size) > > return mem; > > } > > > > +/* > > + * Allocate virtual memory, with the specified minimum alignment. > > + */ > > static void *vm_memalign(size_t alignment, size_t size) > > { > > + phys_addr_t pa; > > void *mem, *p; > > - size_t pages; > > > > - assert(alignment <= PAGE_SIZE); > > - size = PAGE_ALIGN(size); > > - pages = size / PAGE_SIZE; > > - mem = p = alloc_vpages(pages); > > - while (pages--) { > > - phys_addr_t pa = virt_to_phys(alloc_page()); > > + assert(is_power_of_2(alignment)); > > + > > + size = PAGE_ALIGN(size) / PAGE_SIZE; > > + alignment = PAGE_ALIGN(alignment) / PAGE_SIZE; > > + mem = p = alloc_vpages_intern(size, alignment); > > + while (size--) { > > + pa = virt_to_phys(alloc_page()); > > + assert(pa); > > install_page(page_root, pa, p); > > p += PAGE_SIZE; > > } > > -- > > 2.26.2 > > > > Otherwise > > Reviewed-by: Andrew Jones <drjones@redhat.com> > > Thanks, > drew >
On 03/07/20 15:49, Claudio Imbrenda wrote: > On Fri, 3 Jul 2020 14:30:01 +0200 > Andrew Jones <drjones@redhat.com> wrote: > > [...] > >>> -void *alloc_vpages(ulong nr) >>> +/* >>> + * Allocate a certain number of pages from the virtual address >>> space (without >>> + * physical backing). >>> + * >>> + * nr is the number of pages to allocate >>> + * alignment_pages is the alignment of the allocation *in pages* >>> + */ >>> +static void *alloc_vpages_intern(ulong nr, unsigned int >>> alignment_pages) >> >> This helper function isn't necessary. Just introduce >> alloc_vpages_aligned() and then call alloc_vpages_aligned(nr, 1) from >> alloc_vpages(). > > the helper will actually be useful in future patches. > > maybe I should have written that in the patch description. > > I can respin without helper if you prefer (and introduce it when > needed) or simply update the patch description. Would it make sense, for your future patches, to keep the helper (but don't abbrev. "internal" :)) and make it get an order instead of the number of pages? Paolo
diff --git a/lib/vmalloc.h b/lib/vmalloc.h index 2b563f4..fa3fa22 100644 --- a/lib/vmalloc.h +++ b/lib/vmalloc.h @@ -5,6 +5,9 @@ /* Allocate consecutive virtual pages (without backing) */ extern void *alloc_vpages(ulong nr); +/* Allocate consecutive and aligned virtual pages (without backing) */ +extern void *alloc_vpages_aligned(ulong nr, unsigned int alignment_pages); + /* Allocate one virtual page (without backing) */ extern void *alloc_vpage(void); /* Set the top of the virtual address space */ diff --git a/lib/vmalloc.c b/lib/vmalloc.c index 9237a0f..2c482aa 100644 --- a/lib/vmalloc.c +++ b/lib/vmalloc.c @@ -12,19 +12,28 @@ #include "alloc.h" #include "alloc_phys.h" #include "alloc_page.h" +#include <bitops.h> #include "vmalloc.h" static struct spinlock lock; static void *vfree_top = 0; static void *page_root; -void *alloc_vpages(ulong nr) +/* + * Allocate a certain number of pages from the virtual address space (without + * physical backing). + * + * nr is the number of pages to allocate + * alignment_pages is the alignment of the allocation *in pages* + */ +static void *alloc_vpages_intern(ulong nr, unsigned int alignment_pages) { uintptr_t ptr; spin_lock(&lock); ptr = (uintptr_t)vfree_top; ptr -= PAGE_SIZE * nr; + ptr &= GENMASK_ULL(63, PAGE_SHIFT + get_order(alignment_pages)); vfree_top = (void *)ptr; spin_unlock(&lock); @@ -32,6 +41,16 @@ void *alloc_vpages(ulong nr) return (void *)ptr; } +void *alloc_vpages(ulong nr) +{ + return alloc_vpages_intern(nr, 1); +} + +void *alloc_vpages_aligned(ulong nr, unsigned int alignment_pages) +{ + return alloc_vpages_intern(nr, alignment_pages); +} + void *alloc_vpage(void) { return alloc_vpages(1); @@ -55,17 +74,22 @@ void *vmap(phys_addr_t phys, size_t size) return mem; } +/* + * Allocate virtual memory, with the specified minimum alignment. + */ static void *vm_memalign(size_t alignment, size_t size) { + phys_addr_t pa; void *mem, *p; - size_t pages; - assert(alignment <= PAGE_SIZE); - size = PAGE_ALIGN(size); - pages = size / PAGE_SIZE; - mem = p = alloc_vpages(pages); - while (pages--) { - phys_addr_t pa = virt_to_phys(alloc_page()); + assert(is_power_of_2(alignment)); + + size = PAGE_ALIGN(size) / PAGE_SIZE; + alignment = PAGE_ALIGN(alignment) / PAGE_SIZE; + mem = p = alloc_vpages_intern(size, alignment); + while (size--) { + pa = virt_to_phys(alloc_page()); + assert(pa); install_page(page_root, pa, p); p += PAGE_SIZE; }
Allow allocating aligned virtual memory with alignment larger than only one page. Add a check that the backing pages were actually allocated. Export the alloc_vpages_aligned function to allow users to allocate non-backed aligned virtual addresses. Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> --- lib/vmalloc.h | 3 +++ lib/vmalloc.c | 40 ++++++++++++++++++++++++++++++++-------- 2 files changed, 35 insertions(+), 8 deletions(-)