Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752641AbdDDCbR (ORCPT ); Mon, 3 Apr 2017 22:31:17 -0400 Received: from mail-pg0-f65.google.com ([74.125.83.65]:36009 "EHLO mail-pg0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752595AbdDDCbQ (ORCPT ); Mon, 3 Apr 2017 22:31:16 -0400 Date: Tue, 4 Apr 2017 11:31:15 +0900 From: Sergey Senozhatsky To: Minchan Kim Cc: Andrew Morton , linux-kernel@vger.kernel.org, Sergey Senozhatsky , kernel-team@lge.com Subject: Re: [PATCH 4/5] zram: remove zram_meta structure Message-ID: <20170404023115.GC475@jagdpanzerIV.localdomain> References: <1491196653-7388-1-git-send-email-minchan@kernel.org> <1491196653-7388-5-git-send-email-minchan@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1491196653-7388-5-git-send-email-minchan@kernel.org> User-Agent: Mutt/1.8.0 (2017-02-23) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3229 Lines: 143 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