lkml.org 
[lkml]   [2017]   [Feb]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] mm: extend zero pages to same element pages for zram
From
Date


On 2017/2/7 10:54, Minchan Kim wrote:
> On Tue, Feb 07, 2017 at 10:20:57AM +0800, zhouxianrong wrote:
>
> < snip >
>
>>>> 3. the below should be modified.
>>>>
>>>> static inline bool zram_meta_get(struct zram *zram)
>>>> @@ -495,11 +553,17 @@ static void zram_meta_free(struct zram_meta *meta, u64 disksize)
>>>>
>>>> /* Free all pages that are still in this zram device */
>>>> for (index = 0; index < num_pages; index++) {
>>>> - unsigned long handle = meta->table[index].handle;
>>>> + unsigned long handle;
>>>> +
>>>> + bit_spin_lock(ZRAM_ACCESS, &meta->table[index].value);
>>>> + handle = meta->table[index].handle;
>>>>
>>>> - if (!handle)
>>>> + if (!handle || zram_test_flag(meta, index, ZRAM_SAME)) {
>>>> + bit_spin_unlock(ZRAM_ACCESS, &meta->table[index].value);
>>>> continue;
>>>> + }
>>>>
>>>> + bit_spin_unlock(ZRAM_ACCESS, &meta->table[index].value);
>>>> zs_free(meta->mem_pool, handle);
>>>
>>> Could you explain why we need this modification?
>>>
>>>> }
>>>>
>>>> @@ -511,7 +575,7 @@ static void zram_meta_free(struct zram_meta *meta, u64 disksize)
>>>> static struct zram_meta *zram_meta_alloc(char *pool_name, u64 disksize)
>>>> {
>>>> size_t num_pages;
>>>> - struct zram_meta *meta = kmalloc(sizeof(*meta), GFP_KERNEL);
>>>> + struct zram_meta *meta = kzalloc(sizeof(*meta), GFP_KERNEL);
>>>
>>> Ditto
>>>
>>>>
>>>>
>>>
>>> .
>>>
>>
>> because of union of handle and element, i think a non-zero element (other than handle) is prevented from freeing.
>> if zram_meta_get was modified, zram_meta_alloc did so.
>
> Right. Thanks but I don't see why we need the locking in there and modification of
> zram_meta_alloc.
>
> Isn't it enough with this?

i am afraid someone do reset_store, so did lock.

yes, i am wrong, zram_meta_alloc should not be modified here. because meta->table has already cleared

meta->table = vzalloc(num_pages * sizeof(*meta->table));




>
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index c20b05a84f21..a25d34a8af19 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -425,8 +425,11 @@ static void zram_meta_free(struct zram_meta *meta, u64 disksize)
> /* Free all pages that are still in this zram device */
> for (index = 0; index < num_pages; index++) {
> unsigned long handle = meta->table[index].handle;
> -
> - if (!handle)
> + /*
> + * No memory is allocated for same element filled pages.
> + * Simply clear same page flag.
> + */
> + if (!handle || zram_test_flag(meta, index, ZRAM_SAME))
> continue;
>
> zs_free(meta->mem_pool, handle);
>
> .
>

\
 
 \ /
  Last update: 2017-02-07 04:25    [W:0.475 / U:0.028 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site