Hello Ganesh,
On Sat, Dec 13, 2014 at 09:43:23PM +0800, Ganesh Mahendran wrote:
> Currently functions in zsmalloc.c does not arranged in a readable
> and reasonable sequence. With the more and more functions added,
> we may meet below inconvenience. For example:
>
> Current functions:
> void zs_init()
> {
> }
>
> static void get_maxobj_per_zspage()
> {
> }
>
> Then I want to add a func_1() which is called from zs_init(), and this new added
> function func_1() will used get_maxobj_per_zspage() which is defined below zs_init().
>
> void func_1()
> {
> get_maxobj_per_zspage()
> }
>
> void zs_init()
> {
> func_1()
> }
>
> static void get_maxobj_per_zspage()
> {
> }
>
> This will cause compiling issue. So we must add a declaration:
> static void get_maxobj_per_zspage();
> before func_1() if we do not put get_maxobj_per_zspage() before func_1().
Yes, I suffered from that when I made compaction but was not sure
it's it was obviously wrong.
Stupid question:
What's the problem if we should put function declaration on top of
source code?
>
> In addition, puting module_[init|exit] functions at the bottom of the file
> conforms to our habit.
Normally, we do but without any strong reason, I don't want to rub git-blame
by clean up patches.
In summary, I like this patch but don't like to churn git-blame by clean-up
patchset without strong reason so I need something I am sure.
>
> So, this patch ajusts function sequence as:
> /* helper functions */
> ...
> obj_location_to_handle()
> ...
>
> /* Some exported functions */
> ...
>
> zs_map_object()
> zs_unmap_object()
>
> zs_malloc()
> zs_free()
>
> zs_init()
> zs_exit()
>
> Signed-off-by: Ganesh Mahendran <opensource.ganesh@xxxxxxxxx>
> ---
> mm/zsmalloc.c | 374 ++++++++++++++++++++++++++++-----------------------------
> 1 file changed, 187 insertions(+), 187 deletions(-)
>
> diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
> index 4d0a063..b724039 100644
> --- a/mm/zsmalloc.c
> +++ b/mm/zsmalloc.c
> @@ -884,19 +884,6 @@ static struct notifier_block zs_cpu_nb = {
> .notifier_call = zs_cpu_notifier
> };
>
> -static void zs_unregister_cpu_notifier(void)
> -{
> - int cpu;
> -
> - cpu_notifier_register_begin();
> -
> - for_each_online_cpu(cpu)
> - zs_cpu_notifier(NULL, CPU_DEAD, (void *)(long)cpu);
> - __unregister_cpu_notifier(&zs_cpu_nb);
> -
> - cpu_notifier_register_done();
> -}
> -
> static int zs_register_cpu_notifier(void)
> {
> int cpu, uninitialized_var(ret);
> @@ -914,40 +901,28 @@ static int zs_register_cpu_notifier(void)
> return notifier_to_errno(ret);
> }
>
> -static void init_zs_size_classes(void)
> +static void zs_unregister_cpu_notifier(void)
> {
> - int nr;
> + int cpu;
>
> - nr = (ZS_MAX_ALLOC_SIZE - ZS_MIN_ALLOC_SIZE) / ZS_SIZE_CLASS_DELTA + 1;
> - if ((ZS_MAX_ALLOC_SIZE - ZS_MIN_ALLOC_SIZE) % ZS_SIZE_CLASS_DELTA)
> - nr += 1;
> + cpu_notifier_register_begin();
>
> - zs_size_classes = nr;
> -}
> + for_each_online_cpu(cpu)
> + zs_cpu_notifier(NULL, CPU_DEAD, (void *)(long)cpu);
> + __unregister_cpu_notifier(&zs_cpu_nb);
>
> -static void __exit zs_exit(void)
> -{
> -#ifdef CONFIG_ZPOOL
> - zpool_unregister_driver(&zs_zpool_driver);
> -#endif
> - zs_unregister_cpu_notifier();
> + cpu_notifier_register_done();
> }
>
> -static int __init zs_init(void)
> +static void init_zs_size_classes(void)
> {
> - int ret = zs_register_cpu_notifier();
> -
> - if (ret) {
> - zs_unregister_cpu_notifier();
> - return ret;
> - }
> + int nr;
>
> - init_zs_size_classes();
> + nr = (ZS_MAX_ALLOC_SIZE - ZS_MIN_ALLOC_SIZE) / ZS_SIZE_CLASS_DELTA + 1;
> + if ((ZS_MAX_ALLOC_SIZE - ZS_MIN_ALLOC_SIZE) % ZS_SIZE_CLASS_DELTA)
> + nr += 1;
>
> -#ifdef CONFIG_ZPOOL
> - zpool_register_driver(&zs_zpool_driver);
> -#endif
> - return 0;
> + zs_size_classes = nr;
> }
>
> static unsigned int get_maxobj_per_zspage(int size, int pages_per_zspage)
> @@ -967,113 +942,101 @@ static bool can_merge(struct size_class *prev, int size, int pages_per_zspage)
> return true;
> }
>
> +unsigned long zs_get_total_pages(struct zs_pool *pool)
> +{
> + return atomic_long_read(&pool->pages_allocated);
> +}
> +EXPORT_SYMBOL_GPL(zs_get_total_pages);
> +
> /**
> - * zs_create_pool - Creates an allocation pool to work from.
> - * @flags: allocation flags used to allocate pool metadata
> + * zs_map_object - get address of allocated object from handle.
> + * @pool: pool from which the object was allocated
> + * @handle: handle returned from zs_malloc
> *
> - * This function must be called before anything when using
> - * the zsmalloc allocator.
> + * Before using an object allocated from zs_malloc, it must be mapped using
> + * this function. When done with the object, it must be unmapped using
> + * zs_unmap_object.
> *
> - * On success, a pointer to the newly created pool is returned,
> - * otherwise NULL.
> + * Only one object can be mapped per cpu at a time. There is no protection
> + * against nested mappings.
> + *
> + * This function returns with preemption and page faults disabled.
> */
> -struct zs_pool *zs_create_pool(gfp_t flags)
> +void *zs_map_object(struct zs_pool *pool, unsigned long handle,
> + enum zs_mapmode mm)
> {
> - int i;
> - struct zs_pool *pool;
> - struct size_class *prev_class = NULL;
> + struct page *page;
> + unsigned long obj_idx, off;
>
> - pool = kzalloc(sizeof(*pool), GFP_KERNEL);
> - if (!pool)
> - return NULL;
> + unsigned int class_idx;
> + enum fullness_group fg;
> + struct size_class *class;
> + struct mapping_area *area;
> + struct page *pages[2];
>
> - pool->size_class = kcalloc(zs_size_classes, sizeof(struct size_class *),
> - GFP_KERNEL);
> - if (!pool->size_class) {
> - kfree(pool);
> - return NULL;
> - }
> + BUG_ON(!handle);
>
> /*
> - * Iterate reversly, because, size of size_class that we want to use
> - * for merging should be larger or equal to current size.
> + * Because we use per-cpu mapping areas shared among the
> + * pools/users, we can't allow mapping in interrupt context
> + * because it can corrupt another users mappings.
> */
> - for (i = zs_size_classes - 1; i >= 0; i--) {
> - int size;
> - int pages_per_zspage;
> - struct size_class *class;
> -
> - size = ZS_MIN_ALLOC_SIZE + i * ZS_SIZE_CLASS_DELTA;
> - if (size > ZS_MAX_ALLOC_SIZE)
> - size = ZS_MAX_ALLOC_SIZE;
> - pages_per_zspage = get_pages_per_zspage(size);
> -
> - /*
> - * size_class is used for normal zsmalloc operation such
> - * as alloc/free for that size. Although it is natural that we
> - * have one size_class for each size, there is a chance that we
> - * can get more memory utilization if we use one size_class for
> - * many different sizes whose size_class have same
> - * characteristics. So, we makes size_class point to
> - * previous size_class if possible.
> - */
> - if (prev_class) {
> - if (can_merge(prev_class, size, pages_per_zspage)) {
> - pool->size_class[i] = prev_class;
> - continue;
> - }
> - }
> -
> - class = kzalloc(sizeof(struct size_class), GFP_KERNEL);
> - if (!class)
> - goto err;
> + BUG_ON(in_interrupt());
>
> - class->size = size;
> - class->index = i;
> - class->pages_per_zspage = pages_per_zspage;
> - spin_lock_init(&class->lock);
> - pool->size_class[i] = class;
> + obj_handle_to_location(handle, &page, &obj_idx);
> + get_zspage_mapping(get_first_page(page), &class_idx, &fg);
> + class = pool->size_class[class_idx];
> + off = obj_idx_to_offset(page, obj_idx, class->size);
>
> - prev_class = class;
> + area = &get_cpu_var(zs_map_area);
> + area->vm_mm = mm;
> + if (off + class->size <= PAGE_SIZE) {
> + /* this object is contained entirely within a page */
> + area->vm_addr = kmap_atomic(page);
> + return area->vm_addr + off;
> }
>
> - pool->flags = flags;
> -
> - return pool;
> + /* this object spans two pages */
> + pages[0] = page;
> + pages[1] = get_next_page(page);
> + BUG_ON(!pages[1]);
>
> -err:
> - zs_destroy_pool(pool);
> - return NULL;
> + return __zs_map_object(area, pages, off, class->size);
> }
> -EXPORT_SYMBOL_GPL(zs_create_pool);
> +EXPORT_SYMBOL_GPL(zs_map_object);
>
> -void zs_destroy_pool(struct zs_pool *pool)
> +void zs_unmap_object(struct zs_pool *pool, unsigned long handle)
> {
> - int i;
> + struct page *page;
> + unsigned long obj_idx, off;
>
> - for (i = 0; i < zs_size_classes; i++) {
> - int fg;
> - struct size_class *class = pool->size_class[i];
> + unsigned int class_idx;
> + enum fullness_group fg;
> + struct size_class *class;
> + struct mapping_area *area;
>
> - if (!class)
> - continue;
> + BUG_ON(!handle);
>
> - if (class->index != i)
> - continue;
> + obj_handle_to_location(handle, &page, &obj_idx);
> + get_zspage_mapping(get_first_page(page), &class_idx, &fg);
> + class = pool->size_class[class_idx];
> + off = obj_idx_to_offset(page, obj_idx, class->size);
>
> - for (fg = 0; fg < _ZS_NR_FULLNESS_GROUPS; fg++) {
> - if (class->fullness_list[fg]) {
> - pr_info("Freeing non-empty class with size %db, fullness group %d\n",
> - class->size, fg);
> - }
> - }
> - kfree(class);
> - }
> + area = this_cpu_ptr(&zs_map_area);
> + if (off + class->size <= PAGE_SIZE)
> + kunmap_atomic(area->vm_addr);
> + else {
> + struct page *pages[2];
>
> - kfree(pool->size_class);
> - kfree(pool);
> + pages[0] = page;
> + pages[1] = get_next_page(page);
> + BUG_ON(!pages[1]);
> +
> + __zs_unmap_object(area, pages, off, class->size);
> + }
> + put_cpu_var(zs_map_area);
> }
> -EXPORT_SYMBOL_GPL(zs_destroy_pool);
> +EXPORT_SYMBOL_GPL(zs_unmap_object);
>
> /**
> * zs_malloc - Allocate block of given size from pool.
> @@ -1176,100 +1139,137 @@ void zs_free(struct zs_pool *pool, unsigned long obj)
> EXPORT_SYMBOL_GPL(zs_free);
>
> /**
> - * zs_map_object - get address of allocated object from handle.
> - * @pool: pool from which the object was allocated
> - * @handle: handle returned from zs_malloc
> - *
> - * Before using an object allocated from zs_malloc, it must be mapped using
> - * this function. When done with the object, it must be unmapped using
> - * zs_unmap_object.
> + * zs_create_pool - Creates an allocation pool to work from.
> + * @flags: allocation flags used to allocate pool metadata
> *
> - * Only one object can be mapped per cpu at a time. There is no protection
> - * against nested mappings.
> + * This function must be called before anything when using
> + * the zsmalloc allocator.
> *
> - * This function returns with preemption and page faults disabled.
> + * On success, a pointer to the newly created pool is returned,
> + * otherwise NULL.
> */
> -void *zs_map_object(struct zs_pool *pool, unsigned long handle,
> - enum zs_mapmode mm)
> +struct zs_pool *zs_create_pool(gfp_t flags)
> {
> - struct page *page;
> - unsigned long obj_idx, off;
> + int i;
> + struct zs_pool *pool;
> + struct size_class *prev_class = NULL;
>
> - unsigned int class_idx;
> - enum fullness_group fg;
> - struct size_class *class;
> - struct mapping_area *area;
> - struct page *pages[2];
> + pool = kzalloc(sizeof(*pool), GFP_KERNEL);
> + if (!pool)
> + return NULL;
>
> - BUG_ON(!handle);
> + pool->size_class = kcalloc(zs_size_classes, sizeof(struct size_class *),
> + GFP_KERNEL);
> + if (!pool->size_class) {
> + kfree(pool);
> + return NULL;
> + }
>
> /*
> - * Because we use per-cpu mapping areas shared among the
> - * pools/users, we can't allow mapping in interrupt context
> - * because it can corrupt another users mappings.
> + * Iterate reversly, because, size of size_class that we want to use
> + * for merging should be larger or equal to current size.
> */
> - BUG_ON(in_interrupt());
> + for (i = zs_size_classes - 1; i >= 0; i--) {
> + int size;
> + int pages_per_zspage;
> + struct size_class *class;
>
> - obj_handle_to_location(handle, &page, &obj_idx);
> - get_zspage_mapping(get_first_page(page), &class_idx, &fg);
> - class = pool->size_class[class_idx];
> - off = obj_idx_to_offset(page, obj_idx, class->size);
> + size = ZS_MIN_ALLOC_SIZE + i * ZS_SIZE_CLASS_DELTA;
> + if (size > ZS_MAX_ALLOC_SIZE)
> + size = ZS_MAX_ALLOC_SIZE;
> + pages_per_zspage = get_pages_per_zspage(size);
>
> - area = &get_cpu_var(zs_map_area);
> - area->vm_mm = mm;
> - if (off + class->size <= PAGE_SIZE) {
> - /* this object is contained entirely within a page */
> - area->vm_addr = kmap_atomic(page);
> - return area->vm_addr + off;
> + /*
> + * size_class is used for normal zsmalloc operation such
> + * as alloc/free for that size. Although it is natural that we
> + * have one size_class for each size, there is a chance that we
> + * can get more memory utilization if we use one size_class for
> + * many different sizes whose size_class have same
> + * characteristics. So, we makes size_class point to
> + * previous size_class if possible.
> + */
> + if (prev_class) {
> + if (can_merge(prev_class, size, pages_per_zspage)) {
> + pool->size_class[i] = prev_class;
> + continue;
> + }
> + }
> +
> + class = kzalloc(sizeof(struct size_class), GFP_KERNEL);
> + if (!class)
> + goto err;
> +
> + class->size = size;
> + class->index = i;
> + class->pages_per_zspage = pages_per_zspage;
> + spin_lock_init(&class->lock);
> + pool->size_class[i] = class;
> +
> + prev_class = class;
> }
>
> - /* this object spans two pages */
> - pages[0] = page;
> - pages[1] = get_next_page(page);
> - BUG_ON(!pages[1]);
> + pool->flags = flags;
>
> - return __zs_map_object(area, pages, off, class->size);
> + return pool;
> +
> +err:
> + zs_destroy_pool(pool);
> + return NULL;
> }
> -EXPORT_SYMBOL_GPL(zs_map_object);
> +EXPORT_SYMBOL_GPL(zs_create_pool);
>
> -void zs_unmap_object(struct zs_pool *pool, unsigned long handle)
> +void zs_destroy_pool(struct zs_pool *pool)
> {
> - struct page *page;
> - unsigned long obj_idx, off;
> + int i;
>
> - unsigned int class_idx;
> - enum fullness_group fg;
> - struct size_class *class;
> - struct mapping_area *area;
> + for (i = 0; i < zs_size_classes; i++) {
> + int fg;
> + struct size_class *class = pool->size_class[i];
>
> - BUG_ON(!handle);
> + if (!class)
> + continue;
>
> - obj_handle_to_location(handle, &page, &obj_idx);
> - get_zspage_mapping(get_first_page(page), &class_idx, &fg);
> - class = pool->size_class[class_idx];
> - off = obj_idx_to_offset(page, obj_idx, class->size);
> + if (class->index != i)
> + continue;
>
> - area = this_cpu_ptr(&zs_map_area);
> - if (off + class->size <= PAGE_SIZE)
> - kunmap_atomic(area->vm_addr);
> - else {
> - struct page *pages[2];
> + for (fg = 0; fg < _ZS_NR_FULLNESS_GROUPS; fg++) {
> + if (class->fullness_list[fg]) {
> + pr_info("Freeing non-empty class with size %db, fullness group %d\n",
> + class->size, fg);
> + }
> + }
> + kfree(class);
> + }
>
> - pages[0] = page;
> - pages[1] = get_next_page(page);
> - BUG_ON(!pages[1]);
> + kfree(pool->size_class);
> + kfree(pool);
> +}
> +EXPORT_SYMBOL_GPL(zs_destroy_pool);
>
> - __zs_unmap_object(area, pages, off, class->size);
> +static int __init zs_init(void)
> +{
> + int ret = zs_register_cpu_notifier();
> +
> + if (ret) {
> + zs_unregister_cpu_notifier();
> + return ret;
> }
> - put_cpu_var(zs_map_area);
> +
> + init_zs_size_classes();
> +
> +#ifdef CONFIG_ZPOOL
> + zpool_register_driver(&zs_zpool_driver);
> +#endif
> + return 0;
> }
> -EXPORT_SYMBOL_GPL(zs_unmap_object);
>
> -unsigned long zs_get_total_pages(struct zs_pool *pool)
> +static void __exit zs_exit(void)
> {
> - return atomic_long_read(&pool->pages_allocated);
> +#ifdef CONFIG_ZPOOL
> + zpool_unregister_driver(&zs_zpool_driver);
> +#endif
> + zs_unregister_cpu_notifier();
> }
> -EXPORT_SYMBOL_GPL(zs_get_total_pages);
>
> module_init(zs_init);
> module_exit(zs_exit);
> --
> 1.7.9.5
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>