| Message ID | 20201216201200.255172-7-imbrenda@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix and improve the page allocator | expand |
On 12/16/20 12:11 PM, Claudio Imbrenda wrote: > Remove align_min from struct alloc_ops, since it is no longer used. > > Signed-off-by: Claudio Imbrenda<imbrenda@linux.ibm.com> > --- > lib/alloc.h | 1 - > lib/alloc_page.c | 1 - > lib/alloc_phys.c | 9 +++++---- > lib/vmalloc.c | 1 - > 4 files changed, 5 insertions(+), 7 deletions(-) > > diff --git a/lib/alloc.h b/lib/alloc.h > index 9b4b634..db90b01 100644 > --- a/lib/alloc.h > +++ b/lib/alloc.h > @@ -25,7 +25,6 @@ > struct alloc_ops { > void *(*memalign)(size_t alignment, size_t size); > void (*free)(void *ptr); > - size_t align_min; > }; > > extern struct alloc_ops *alloc_ops; > diff --git a/lib/alloc_page.c b/lib/alloc_page.c > index 8d2700d..b1cdf21 100644 > --- a/lib/alloc_page.c > +++ b/lib/alloc_page.c > @@ -385,7 +385,6 @@ void *memalign_pages_area(unsigned int area, size_t alignment, size_t size) > static struct alloc_ops page_alloc_ops = { > .memalign = memalign_pages, > .free = free_pages, > - .align_min = PAGE_SIZE, > }; > > /* > diff --git a/lib/alloc_phys.c b/lib/alloc_phys.c > index 72e20f7..a4d2bf2 100644 > --- a/lib/alloc_phys.c > +++ b/lib/alloc_phys.c > @@ -29,8 +29,8 @@ static phys_addr_t base, top; > static void *early_memalign(size_t alignment, size_t size); > static struct alloc_ops early_alloc_ops = { > .memalign = early_memalign, > - .align_min = DEFAULT_MINIMUM_ALIGNMENT > }; > +static size_t align_min; I don't see any caller of phys_alloc_set_minimum_alignment(). So when you are comparing against this variable in phys_alloc_aligned_safe() below, you are comparing against zero. Is that what you is intended or should 'align_min' be set some default ? > > struct alloc_ops *alloc_ops = &early_alloc_ops; > > @@ -39,8 +39,7 @@ void phys_alloc_show(void) > int i; > > spin_lock(&lock); > - printf("phys_alloc minimum alignment: %#" PRIx64 "\n", > - (u64)early_alloc_ops.align_min); > + printf("phys_alloc minimum alignment: %#" PRIx64 "\n", (u64)align_min); > for (i = 0; i < nr_regions; ++i) > printf("%016" PRIx64 "-%016" PRIx64 " [%s]\n", > (u64)regions[i].base, > @@ -64,7 +63,7 @@ void phys_alloc_set_minimum_alignment(phys_addr_t align) > { > assert(align && !(align & (align - 1))); > spin_lock(&lock); > - early_alloc_ops.align_min = align; > + align_min = align; > spin_unlock(&lock); > } > > @@ -83,6 +82,8 @@ static phys_addr_t phys_alloc_aligned_safe(phys_addr_t size, > top_safe = MIN(top_safe, 1ULL << 32); > > assert(base < top_safe); > + if (align < align_min) > + align = align_min; > > addr = ALIGN(base, align); > size += addr - base; > diff --git a/lib/vmalloc.c b/lib/vmalloc.c > index 7a49adf..e146162 100644 > --- a/lib/vmalloc.c > +++ b/lib/vmalloc.c > @@ -190,7 +190,6 @@ static void vm_free(void *mem) > static struct alloc_ops vmalloc_ops = { > .memalign = vm_memalign, > .free = vm_free, > - .align_min = PAGE_SIZE, > }; > > void __attribute__((__weak__)) find_highmem(void)
On Thu, 24 Dec 2020 10:17:20 -0800 Krish Sadhukhan <krish.sadhukhan@oracle.com> wrote: > On 12/16/20 12:11 PM, Claudio Imbrenda wrote: > > Remove align_min from struct alloc_ops, since it is no longer used. > > > > Signed-off-by: Claudio Imbrenda<imbrenda@linux.ibm.com> > > --- > > lib/alloc.h | 1 - > > lib/alloc_page.c | 1 - > > lib/alloc_phys.c | 9 +++++---- > > lib/vmalloc.c | 1 - > > 4 files changed, 5 insertions(+), 7 deletions(-) > > > > diff --git a/lib/alloc.h b/lib/alloc.h > > index 9b4b634..db90b01 100644 > > --- a/lib/alloc.h > > +++ b/lib/alloc.h > > @@ -25,7 +25,6 @@ > > struct alloc_ops { > > void *(*memalign)(size_t alignment, size_t size); > > void (*free)(void *ptr); > > - size_t align_min; > > }; > > > > extern struct alloc_ops *alloc_ops; > > diff --git a/lib/alloc_page.c b/lib/alloc_page.c > > index 8d2700d..b1cdf21 100644 > > --- a/lib/alloc_page.c > > +++ b/lib/alloc_page.c > > @@ -385,7 +385,6 @@ void *memalign_pages_area(unsigned int area, > > size_t alignment, size_t size) static struct alloc_ops > > page_alloc_ops = { .memalign = memalign_pages, > > .free = free_pages, > > - .align_min = PAGE_SIZE, > > }; > > > > /* > > diff --git a/lib/alloc_phys.c b/lib/alloc_phys.c > > index 72e20f7..a4d2bf2 100644 > > --- a/lib/alloc_phys.c > > +++ b/lib/alloc_phys.c > > @@ -29,8 +29,8 @@ static phys_addr_t base, top; > > static void *early_memalign(size_t alignment, size_t size); > > static struct alloc_ops early_alloc_ops = { > > .memalign = early_memalign, > > - .align_min = DEFAULT_MINIMUM_ALIGNMENT > > }; > > +static size_t align_min; > > > I don't see any caller of phys_alloc_set_minimum_alignment(). So when lib/arm/setup.c:150 lib/powerpc/setup.c:150 > you are comparing against this variable in phys_alloc_aligned_safe() > below, you are comparing against zero. Is that what you is intended > or should 'align_min' be set some default ? if the architecture specific code did not specify anything better, 0 is ok. > > > > struct alloc_ops *alloc_ops = &early_alloc_ops; > > > > @@ -39,8 +39,7 @@ void phys_alloc_show(void) > > int i; > > > > spin_lock(&lock); > > - printf("phys_alloc minimum alignment: %#" PRIx64 "\n", > > - (u64)early_alloc_ops.align_min); > > + printf("phys_alloc minimum alignment: %#" PRIx64 "\n", > > (u64)align_min); for (i = 0; i < nr_regions; ++i) > > printf("%016" PRIx64 "-%016" PRIx64 " [%s]\n", > > (u64)regions[i].base, > > @@ -64,7 +63,7 @@ void phys_alloc_set_minimum_alignment(phys_addr_t > > align) { > > assert(align && !(align & (align - 1))); > > spin_lock(&lock); > > - early_alloc_ops.align_min = align; > > + align_min = align; > > spin_unlock(&lock); > > } > > > > @@ -83,6 +82,8 @@ static phys_addr_t > > phys_alloc_aligned_safe(phys_addr_t size, top_safe = MIN(top_safe, > > 1ULL << 32); > > assert(base < top_safe); > > + if (align < align_min) > > + align = align_min; > > > > addr = ALIGN(base, align); > > size += addr - base; > > diff --git a/lib/vmalloc.c b/lib/vmalloc.c > > index 7a49adf..e146162 100644 > > --- a/lib/vmalloc.c > > +++ b/lib/vmalloc.c > > @@ -190,7 +190,6 @@ static void vm_free(void *mem) > > static struct alloc_ops vmalloc_ops = { > > .memalign = vm_memalign, > > .free = vm_free, > > - .align_min = PAGE_SIZE, > > }; > > > > void __attribute__((__weak__)) find_highmem(void)
On 1/4/21 5:05 AM, Claudio Imbrenda wrote: > On Thu, 24 Dec 2020 10:17:20 -0800 > Krish Sadhukhan <krish.sadhukhan@oracle.com> wrote: > >> On 12/16/20 12:11 PM, Claudio Imbrenda wrote: >>> Remove align_min from struct alloc_ops, since it is no longer used. >>> >>> Signed-off-by: Claudio Imbrenda<imbrenda@linux.ibm.com> >>> --- >>> lib/alloc.h | 1 - >>> lib/alloc_page.c | 1 - >>> lib/alloc_phys.c | 9 +++++---- >>> lib/vmalloc.c | 1 - >>> 4 files changed, 5 insertions(+), 7 deletions(-) >>> >>> diff --git a/lib/alloc.h b/lib/alloc.h >>> index 9b4b634..db90b01 100644 >>> --- a/lib/alloc.h >>> +++ b/lib/alloc.h >>> @@ -25,7 +25,6 @@ >>> struct alloc_ops { >>> void *(*memalign)(size_t alignment, size_t size); >>> void (*free)(void *ptr); >>> - size_t align_min; >>> }; >>> >>> extern struct alloc_ops *alloc_ops; >>> diff --git a/lib/alloc_page.c b/lib/alloc_page.c >>> index 8d2700d..b1cdf21 100644 >>> --- a/lib/alloc_page.c >>> +++ b/lib/alloc_page.c >>> @@ -385,7 +385,6 @@ void *memalign_pages_area(unsigned int area, >>> size_t alignment, size_t size) static struct alloc_ops >>> page_alloc_ops = { .memalign = memalign_pages, >>> .free = free_pages, >>> - .align_min = PAGE_SIZE, >>> }; >>> >>> /* >>> diff --git a/lib/alloc_phys.c b/lib/alloc_phys.c >>> index 72e20f7..a4d2bf2 100644 >>> --- a/lib/alloc_phys.c >>> +++ b/lib/alloc_phys.c >>> @@ -29,8 +29,8 @@ static phys_addr_t base, top; >>> static void *early_memalign(size_t alignment, size_t size); >>> static struct alloc_ops early_alloc_ops = { >>> .memalign = early_memalign, >>> - .align_min = DEFAULT_MINIMUM_ALIGNMENT >>> }; >>> +static size_t align_min; >> >> I don't see any caller of phys_alloc_set_minimum_alignment(). So when > lib/arm/setup.c:150 > lib/powerpc/setup.c:150 > >> you are comparing against this variable in phys_alloc_aligned_safe() >> below, you are comparing against zero. Is that what you is intended >> or should 'align_min' be set some default ? > if the architecture specific code did not specify anything better, 0 is > ok. > >>> >>> struct alloc_ops *alloc_ops = &early_alloc_ops; >>> >>> @@ -39,8 +39,7 @@ void phys_alloc_show(void) >>> int i; >>> >>> spin_lock(&lock); >>> - printf("phys_alloc minimum alignment: %#" PRIx64 "\n", >>> - (u64)early_alloc_ops.align_min); >>> + printf("phys_alloc minimum alignment: %#" PRIx64 "\n", >>> (u64)align_min); for (i = 0; i < nr_regions; ++i) >>> printf("%016" PRIx64 "-%016" PRIx64 " [%s]\n", >>> (u64)regions[i].base, >>> @@ -64,7 +63,7 @@ void phys_alloc_set_minimum_alignment(phys_addr_t >>> align) { >>> assert(align && !(align & (align - 1))); >>> spin_lock(&lock); >>> - early_alloc_ops.align_min = align; >>> + align_min = align; >>> spin_unlock(&lock); >>> } >>> >>> @@ -83,6 +82,8 @@ static phys_addr_t >>> phys_alloc_aligned_safe(phys_addr_t size, top_safe = MIN(top_safe, >>> 1ULL << 32); >>> assert(base < top_safe); >>> + if (align < align_min) >>> + align = align_min; >>> >>> addr = ALIGN(base, align); >>> size += addr - base; >>> diff --git a/lib/vmalloc.c b/lib/vmalloc.c >>> index 7a49adf..e146162 100644 >>> --- a/lib/vmalloc.c >>> +++ b/lib/vmalloc.c >>> @@ -190,7 +190,6 @@ static void vm_free(void *mem) >>> static struct alloc_ops vmalloc_ops = { >>> .memalign = vm_memalign, >>> .free = vm_free, >>> - .align_min = PAGE_SIZE, >>> }; >>> >>> void __attribute__((__weak__)) find_highmem(void) Reviewed-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
diff --git a/lib/alloc.h b/lib/alloc.h index 9b4b634..db90b01 100644 --- a/lib/alloc.h +++ b/lib/alloc.h @@ -25,7 +25,6 @@ struct alloc_ops { void *(*memalign)(size_t alignment, size_t size); void (*free)(void *ptr); - size_t align_min; }; extern struct alloc_ops *alloc_ops; diff --git a/lib/alloc_page.c b/lib/alloc_page.c index 8d2700d..b1cdf21 100644 --- a/lib/alloc_page.c +++ b/lib/alloc_page.c @@ -385,7 +385,6 @@ void *memalign_pages_area(unsigned int area, size_t alignment, size_t size) static struct alloc_ops page_alloc_ops = { .memalign = memalign_pages, .free = free_pages, - .align_min = PAGE_SIZE, }; /* diff --git a/lib/alloc_phys.c b/lib/alloc_phys.c index 72e20f7..a4d2bf2 100644 --- a/lib/alloc_phys.c +++ b/lib/alloc_phys.c @@ -29,8 +29,8 @@ static phys_addr_t base, top; static void *early_memalign(size_t alignment, size_t size); static struct alloc_ops early_alloc_ops = { .memalign = early_memalign, - .align_min = DEFAULT_MINIMUM_ALIGNMENT }; +static size_t align_min; struct alloc_ops *alloc_ops = &early_alloc_ops; @@ -39,8 +39,7 @@ void phys_alloc_show(void) int i; spin_lock(&lock); - printf("phys_alloc minimum alignment: %#" PRIx64 "\n", - (u64)early_alloc_ops.align_min); + printf("phys_alloc minimum alignment: %#" PRIx64 "\n", (u64)align_min); for (i = 0; i < nr_regions; ++i) printf("%016" PRIx64 "-%016" PRIx64 " [%s]\n", (u64)regions[i].base, @@ -64,7 +63,7 @@ void phys_alloc_set_minimum_alignment(phys_addr_t align) { assert(align && !(align & (align - 1))); spin_lock(&lock); - early_alloc_ops.align_min = align; + align_min = align; spin_unlock(&lock); } @@ -83,6 +82,8 @@ static phys_addr_t phys_alloc_aligned_safe(phys_addr_t size, top_safe = MIN(top_safe, 1ULL << 32); assert(base < top_safe); + if (align < align_min) + align = align_min; addr = ALIGN(base, align); size += addr - base; diff --git a/lib/vmalloc.c b/lib/vmalloc.c index 7a49adf..e146162 100644 --- a/lib/vmalloc.c +++ b/lib/vmalloc.c @@ -190,7 +190,6 @@ static void vm_free(void *mem) static struct alloc_ops vmalloc_ops = { .memalign = vm_memalign, .free = vm_free, - .align_min = PAGE_SIZE, }; void __attribute__((__weak__)) find_highmem(void)
Remove align_min from struct alloc_ops, since it is no longer used. Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> --- lib/alloc.h | 1 - lib/alloc_page.c | 1 - lib/alloc_phys.c | 9 +++++---- lib/vmalloc.c | 1 - 4 files changed, 5 insertions(+), 7 deletions(-)