lkml.org 
[lkml]   [2017]   [Apr]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 4/5] zram: remove zram_meta structure
    On Tue, Apr 04, 2017 at 11:31:15AM +0900, Sergey Senozhatsky wrote:
    > 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?

    Right you are.
    I will fix it!

    >
    >
    > [..]
    > > -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

    \
     
     \ /
      Last update: 2017-04-04 06:53    [W:3.281 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site