On (04/03/17 14:17), Minchan Kim wrote:
[..]
> -static struct zram_meta *zram_meta_alloc(char *pool_name, u64 disksize)
> +static bool zram_meta_alloc(struct zram *zram, u64 disksize)
>  {
>       size_t num_pages;
> -     struct zram_meta *meta = kmalloc(sizeof(*meta), GFP_KERNEL);
> -
> -     if (!meta)
> -             return NULL;
>  
>       num_pages = disksize >> PAGE_SHIFT;
> -     meta->table = vzalloc(num_pages * sizeof(*meta->table));
> -     if (!meta->table) {
> -             pr_err("Error allocating zram address table\n");
> -             goto out_error;
> -     }
> +     zram->table = vzalloc(num_pages * sizeof(*zram->table));
> +     if (!zram->table)
> +             return false;
>  
> -     meta->mem_pool = zs_create_pool(pool_name);
> -     if (!meta->mem_pool) {
> -             pr_err("Error creating memory pool\n");
> -             goto out_error;
> +     zram->mem_pool = zs_create_pool(zram->disk->disk_name);
> +     if (!zram->mem_pool) {
> +             vfree(zram->table);
> +             return false;
>       }
>  
> -     return meta;
> -
> -out_error:
> -     vfree(meta->table);
> -     kfree(meta);
> -     return NULL;
> +     return true;
>  }

[..]

> @@ -1020,7 +989,6 @@ static ssize_t disksize_store(struct device *dev,
>  {
>       u64 disksize;
>       struct zcomp *comp;
> -     struct zram_meta *meta;
>       struct zram *zram = dev_to_zram(dev);
>       int err;
>  
> @@ -1029,8 +997,7 @@ static ssize_t disksize_store(struct device *dev,
>               return -EINVAL;
>  
>       disksize = PAGE_ALIGN(disksize);
> -     meta = zram_meta_alloc(zram->disk->disk_name, disksize);
> -     if (!meta)
> +     if (!zram_meta_alloc(zram, disksize))
>               return -ENOMEM;
>  
>       comp = zcomp_create(zram->compressor);
> @@ -1048,7 +1015,6 @@ static ssize_t disksize_store(struct device *dev,
>               goto out_destroy_comp;
>       }
>  
> -     zram->meta = meta;
>       zram->comp = comp;
>       zram->disksize = disksize;
>       set_capacity(zram->disk, zram->disksize >> SECTOR_SHIFT);
> @@ -1061,7 +1027,7 @@ static ssize_t disksize_store(struct device *dev,
>       up_write(&zram->init_lock);
>       zcomp_destroy(comp);
>  out_free_meta:
> -     zram_meta_free(meta, disksize);
> +     zram_meta_free(zram, disksize);
>       return err;
>  }

OK, I don't think it's the same.

we used to have

        struct zram_meta *zram_meta_alloc()
        {
                meta->table = vzalloc()
                meta->mem_pool = zs_create_pool();
                return meta;
        }


        disksize_store()
        {
                meta = zram_meta_alloc();
                if (init_done(zram)) {
                        pr_info("Cannot change disksize for initialized 
device\n");
                        goto out_destroy_comp;
                }

                zram->meta = meta;
                ^^^^^^^^^^^^^^^^^^
        }

now we have

        struct zram_meta *zram_meta_alloc()
        {
                zram->table = vzalloc()
                zram->mem_pool = zs_create_pool();
                return true;
        }


        disksize_store()
        {
                zram_meta_alloc();
                ^^^^^^^^^^^^^^^^^
                if (init_done(zram)) {
                        pr_info("Cannot change disksize for initialized 
device\n");
                        goto out_destroy_comp;
                }
        }


by the time we call init_done(zram) on already init device zram->table
and zram->mem_pool are overwritten and lost. right?


[..]
> -struct zram_meta {
> +struct zram {
>       struct zram_table_entry *table;
>       struct zs_pool *mem_pool;
> -};
> -
> -struct zram {
> -     struct zram_meta *meta;
>       struct zcomp *comp;
>       struct gendisk *disk;
>       /* Prevent concurrent execution of device init */


we still have several zram_meta_FOO() left overs in zram_drv.c

        -ss

Reply via email to