From: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
To: Minchan Kim <minchan@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
linux-kernel@vger.kernel.org,
Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
kernel-team@lge.com
Subject: Re: [PATCH 4/5] zram: remove zram_meta structure
Date: Tue, 4 Apr 2017 11:31:15 +0900 [thread overview]
Message-ID: <20170404023115.GC475@jagdpanzerIV.localdomain> (raw)
In-Reply-To: <1491196653-7388-5-git-send-email-minchan@kernel.org>
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
next prev parent reply other threads:[~2017-04-04 2:31 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-03 5:17 [PATCH 0/5] zram clean up Minchan Kim
2017-04-03 5:17 ` [PATCH 1/5] zram: handle multiple pages attached bio's bvec Minchan Kim
2017-04-03 22:45 ` Andrew Morton
2017-04-03 23:13 ` Minchan Kim
2017-04-04 4:55 ` Sergey Senozhatsky
2017-04-03 5:17 ` [PATCH 2/5] zram: partial IO refactoring Minchan Kim
2017-04-03 5:52 ` Mika Penttilä
2017-04-03 6:12 ` Minchan Kim
2017-04-03 6:57 ` Mika Penttilä
2017-04-04 2:17 ` Sergey Senozhatsky
2017-04-04 4:50 ` Minchan Kim
2017-04-03 5:17 ` [PATCH 3/5] zram: use zram_slot_lock instead of raw bit_spin_lock op Minchan Kim
2017-04-03 6:08 ` Sergey Senozhatsky
2017-04-03 6:34 ` Minchan Kim
2017-04-03 8:06 ` Sergey Senozhatsky
2017-04-04 2:18 ` Sergey Senozhatsky
2017-04-04 4:50 ` Minchan Kim
2017-04-03 5:17 ` [PATCH 4/5] zram: remove zram_meta structure Minchan Kim
2017-04-04 2:31 ` Sergey Senozhatsky [this message]
2017-04-04 4:52 ` Minchan Kim
2017-04-04 5:40 ` Minchan Kim
2017-04-04 5:54 ` Sergey Senozhatsky
2017-04-03 5:17 ` [PATCH 5/5] zram: introduce zram data accessor Minchan Kim
2017-04-04 4:40 ` Sergey Senozhatsky
2017-04-11 5:38 ` [PATCH 0/5] zram clean up Minchan Kim
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170404023115.GC475@jagdpanzerIV.localdomain \
--to=sergey.senozhatsky.work@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=kernel-team@lge.com \
--cc=linux-kernel@vger.kernel.org \
--cc=minchan@kernel.org \
--cc=sergey.senozhatsky@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).