Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751073AbdDDEww (ORCPT ); Tue, 4 Apr 2017 00:52:52 -0400 Received: from LGEAMRELO13.lge.com ([156.147.23.53]:44111 "EHLO lgeamrelo13.lge.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750961AbdDDEwv (ORCPT ); Tue, 4 Apr 2017 00:52:51 -0400 X-Original-SENDERIP: 156.147.1.126 X-Original-MAILFROM: minchan@kernel.org X-Original-SENDERIP: 165.244.249.23 X-Original-MAILFROM: minchan@kernel.org X-Original-SENDERIP: 10.177.223.161 X-Original-MAILFROM: minchan@kernel.org Date: Tue, 4 Apr 2017 13:52:48 +0900 From: Minchan Kim To: Sergey Senozhatsky CC: Andrew Morton , , Sergey Senozhatsky , Subject: Re: [PATCH 4/5] zram: remove zram_meta structure Message-ID: <20170404045248.GC32020@bbox> References: <1491196653-7388-1-git-send-email-minchan@kernel.org> <1491196653-7388-5-git-send-email-minchan@kernel.org> <20170404023115.GC475@jagdpanzerIV.localdomain> MIME-Version: 1.0 In-Reply-To: <20170404023115.GC475@jagdpanzerIV.localdomain> User-Agent: Mutt/1.5.24 (2015-08-30) X-MIMETrack: Itemize by SMTP Server on LGEKRMHUB01/LGE/LG Group(Release 8.5.3FP6|November 21, 2013) at 2017/04/04 13:52:48, Serialize by Router on LGEKRMHUB01/LGE/LG Group(Release 8.5.3FP6|November 21, 2013) at 2017/04/04 13:52:48, Serialize complete at 2017/04/04 13:52:48 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3615 Lines: 148 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