Re: [PATCH v2 2/2] zram: reduce metadata overhead
On Thu, Nov 29, 2012 at 10:54:49PM -0800, Nitin Gupta wrote: > Changelog v2 vs v1: > - Use is_zero_page() instead of direct handle comparison > - Use 1 as invalid handle value instead of -1 since handle > is unsigned and thus -1 may refer to a valid object. While 1 > is guaranteed to be invalid since can never > refer to (end of) a valid object. Ho, Hmm, another coupling between zram and zsmalloc. The zram knows internal of zsmalloc very well. Sigh. I don't like it really. Nonetheless, if you really want it, please put "#define ZS_INVALID_HANDLE 1" in zsmalloc.h and use it. But the concern about my suggestion is that user can imagine it's equal to 0 so they might try to use it instead of 0. Maybe we need more clear name. Off-topic: Anyway, my assumption about user's mistake is only vaild in case of general allocator but zsmalloc already wasn't general allocator by following as. zs_map_object zs_get_objsize ZS_INVALID_HANDLE Now I'm sure we shouldn't put it in under /lib. :( > - Remove references to 'table' in comments and messages since > we just have a plain array of handles now. > > For every allocated object, zram maintains the the handle, size, > flags and count fields. Of these, only the handle is required > since zsmalloc now provides the object size given the handle. > The flags field was needed only to mark a given page as zero-filled. > Instead of this field, we now use an invalid value (-1) to mark such ZS_INAVLID_HANDLE or something. > pages. Lastly, the count field was unused, so was simply removed. > > Signed-off-by: Nitin Gupta > Reviewed-by: Jerome Marchand > --- > drivers/staging/zram/zram_drv.c | 97 > --- > drivers/staging/zram/zram_drv.h | 20 ++-- > 2 files changed, 43 insertions(+), 74 deletions(-) > Otherwise, looks good to me. -- Kind regards, Minchan Kim -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/2] zram: reduce metadata overhead
On Thu, Nov 29, 2012 at 10:54:49PM -0800, Nitin Gupta wrote: Changelog v2 vs v1: - Use is_zero_page() instead of direct handle comparison - Use 1 as invalid handle value instead of -1 since handle is unsigned and thus -1 may refer to a valid object. While 1 is guaranteed to be invalid since pfn:0,offset:1 can never refer to (end of) a valid object. Ho, Hmm, another coupling between zram and zsmalloc. The zram knows internal of zsmalloc very well. Sigh. I don't like it really. Nonetheless, if you really want it, please put #define ZS_INVALID_HANDLE 1 in zsmalloc.h and use it. But the concern about my suggestion is that user can imagine it's equal to 0 so they might try to use it instead of 0. Maybe we need more clear name. Off-topic: Anyway, my assumption about user's mistake is only vaild in case of general allocator but zsmalloc already wasn't general allocator by following as. zs_map_object zs_get_objsize ZS_INVALID_HANDLE Now I'm sure we shouldn't put it in under /lib. :( - Remove references to 'table' in comments and messages since we just have a plain array of handles now. For every allocated object, zram maintains the the handle, size, flags and count fields. Of these, only the handle is required since zsmalloc now provides the object size given the handle. The flags field was needed only to mark a given page as zero-filled. Instead of this field, we now use an invalid value (-1) to mark such ZS_INAVLID_HANDLE or something. pages. Lastly, the count field was unused, so was simply removed. Signed-off-by: Nitin Gupta ngu...@vflare.org Reviewed-by: Jerome Marchand jmarc...@redhat.com --- drivers/staging/zram/zram_drv.c | 97 --- drivers/staging/zram/zram_drv.h | 20 ++-- 2 files changed, 43 insertions(+), 74 deletions(-) Otherwise, looks good to me. -- Kind regards, Minchan Kim -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 2/2] zram: reduce metadata overhead
Changelog v2 vs v1:
- Use is_zero_page() instead of direct handle comparison
- Use 1 as invalid handle value instead of -1 since handle
is unsigned and thus -1 may refer to a valid object. While 1
is guaranteed to be invalid since can never
refer to (end of) a valid object.
- Remove references to 'table' in comments and messages since
we just have a plain array of handles now.
For every allocated object, zram maintains the the handle, size,
flags and count fields. Of these, only the handle is required
since zsmalloc now provides the object size given the handle.
The flags field was needed only to mark a given page as zero-filled.
Instead of this field, we now use an invalid value (-1) to mark such
pages. Lastly, the count field was unused, so was simply removed.
Signed-off-by: Nitin Gupta
Reviewed-by: Jerome Marchand
---
drivers/staging/zram/zram_drv.c | 97 ---
drivers/staging/zram/zram_drv.h | 20 ++--
2 files changed, 43 insertions(+), 74 deletions(-)
diff --git a/drivers/staging/zram/zram_drv.c b/drivers/staging/zram/zram_drv.c
index f2a73bd..e6c9bec 100644
--- a/drivers/staging/zram/zram_drv.c
+++ b/drivers/staging/zram/zram_drv.c
@@ -71,24 +71,6 @@ static void zram_stat64_inc(struct zram *zram, u64 *v)
zram_stat64_add(zram, v, 1);
}
-static int zram_test_flag(struct zram *zram, u32 index,
- enum zram_pageflags flag)
-{
- return zram->table[index].flags & BIT(flag);
-}
-
-static void zram_set_flag(struct zram *zram, u32 index,
- enum zram_pageflags flag)
-{
- zram->table[index].flags |= BIT(flag);
-}
-
-static void zram_clear_flag(struct zram *zram, u32 index,
- enum zram_pageflags flag)
-{
- zram->table[index].flags &= ~BIT(flag);
-}
-
static int page_zero_filled(void *ptr)
{
unsigned int pos;
@@ -104,6 +86,11 @@ static int page_zero_filled(void *ptr)
return 1;
}
+static inline int is_zero_page(unsigned long handle)
+{
+ return handle == zero_page_handle;
+}
+
static void zram_set_disksize(struct zram *zram, size_t totalram_bytes)
{
if (!zram->disksize) {
@@ -135,21 +122,20 @@ static void zram_set_disksize(struct zram *zram, size_t
totalram_bytes)
static void zram_free_page(struct zram *zram, size_t index)
{
- unsigned long handle = zram->table[index].handle;
- u16 size = zram->table[index].size;
+ unsigned long handle = zram->handle[index];
+ size_t size;
- if (unlikely(!handle)) {
- /*
-* No memory is allocated for zero filled pages.
-* Simply clear zero page flag.
-*/
- if (zram_test_flag(zram, index, ZRAM_ZERO)) {
- zram_clear_flag(zram, index, ZRAM_ZERO);
- zram_stat_dec(>stats.pages_zero);
- }
+ if (unlikely(!handle))
+ return;
+
+ if (is_zero_page(handle)) {
+ /* No memory is allocated for zero filled pages */
+ zram->handle[index] = 0;
+ zram_stat_dec(>stats.pages_zero);
return;
}
+ size = zs_get_object_size(zram->mem_pool, handle);
if (unlikely(size > max_zpage_size))
zram_stat_dec(>stats.bad_compress);
@@ -158,12 +144,10 @@ static void zram_free_page(struct zram *zram, size_t
index)
if (size <= PAGE_SIZE / 2)
zram_stat_dec(>stats.good_compress);
- zram_stat64_sub(zram, >stats.compr_size,
- zram->table[index].size);
+ zram_stat64_sub(zram, >stats.compr_size, size);
zram_stat_dec(>stats.pages_stored);
- zram->table[index].handle = 0;
- zram->table[index].size = 0;
+ zram->handle[index] = 0;
}
static void handle_zero_page(struct bio_vec *bvec)
@@ -188,19 +172,20 @@ static int zram_decompress_page(struct zram *zram, char
*mem, u32 index)
int ret = LZO_E_OK;
size_t clen = PAGE_SIZE;
unsigned char *cmem;
- unsigned long handle = zram->table[index].handle;
+ unsigned long handle = zram->handle[index];
+ size_t objsize;
- if (!handle || zram_test_flag(zram, index, ZRAM_ZERO)) {
+ if (!handle || is_zero_page(handle)) {
memset(mem, 0, PAGE_SIZE);
return 0;
}
+ objsize = zs_get_object_size(zram->mem_pool, handle);
cmem = zs_map_object(zram->mem_pool, handle, ZS_MM_RO);
- if (zram->table[index].size == PAGE_SIZE)
+ if (objsize == PAGE_SIZE)
memcpy(mem, cmem, PAGE_SIZE);
else
- ret = lzo1x_decompress_safe(cmem, zram->table[index].size,
- mem, );
+ ret = lzo1x_decompress_safe(cmem, objsize, mem, );
zs_unmap_object(zram->mem_pool, handle);
/* Should NEVER happen. Return bio error if it does. */
@@
[PATCH v2 2/2] zram: reduce metadata overhead
Changelog v2 vs v1:
- Use is_zero_page() instead of direct handle comparison
- Use 1 as invalid handle value instead of -1 since handle
is unsigned and thus -1 may refer to a valid object. While 1
is guaranteed to be invalid since pfn:0,offset:1 can never
refer to (end of) a valid object.
- Remove references to 'table' in comments and messages since
we just have a plain array of handles now.
For every allocated object, zram maintains the the handle, size,
flags and count fields. Of these, only the handle is required
since zsmalloc now provides the object size given the handle.
The flags field was needed only to mark a given page as zero-filled.
Instead of this field, we now use an invalid value (-1) to mark such
pages. Lastly, the count field was unused, so was simply removed.
Signed-off-by: Nitin Gupta ngu...@vflare.org
Reviewed-by: Jerome Marchand jmarc...@redhat.com
---
drivers/staging/zram/zram_drv.c | 97 ---
drivers/staging/zram/zram_drv.h | 20 ++--
2 files changed, 43 insertions(+), 74 deletions(-)
diff --git a/drivers/staging/zram/zram_drv.c b/drivers/staging/zram/zram_drv.c
index f2a73bd..e6c9bec 100644
--- a/drivers/staging/zram/zram_drv.c
+++ b/drivers/staging/zram/zram_drv.c
@@ -71,24 +71,6 @@ static void zram_stat64_inc(struct zram *zram, u64 *v)
zram_stat64_add(zram, v, 1);
}
-static int zram_test_flag(struct zram *zram, u32 index,
- enum zram_pageflags flag)
-{
- return zram-table[index].flags BIT(flag);
-}
-
-static void zram_set_flag(struct zram *zram, u32 index,
- enum zram_pageflags flag)
-{
- zram-table[index].flags |= BIT(flag);
-}
-
-static void zram_clear_flag(struct zram *zram, u32 index,
- enum zram_pageflags flag)
-{
- zram-table[index].flags = ~BIT(flag);
-}
-
static int page_zero_filled(void *ptr)
{
unsigned int pos;
@@ -104,6 +86,11 @@ static int page_zero_filled(void *ptr)
return 1;
}
+static inline int is_zero_page(unsigned long handle)
+{
+ return handle == zero_page_handle;
+}
+
static void zram_set_disksize(struct zram *zram, size_t totalram_bytes)
{
if (!zram-disksize) {
@@ -135,21 +122,20 @@ static void zram_set_disksize(struct zram *zram, size_t
totalram_bytes)
static void zram_free_page(struct zram *zram, size_t index)
{
- unsigned long handle = zram-table[index].handle;
- u16 size = zram-table[index].size;
+ unsigned long handle = zram-handle[index];
+ size_t size;
- if (unlikely(!handle)) {
- /*
-* No memory is allocated for zero filled pages.
-* Simply clear zero page flag.
-*/
- if (zram_test_flag(zram, index, ZRAM_ZERO)) {
- zram_clear_flag(zram, index, ZRAM_ZERO);
- zram_stat_dec(zram-stats.pages_zero);
- }
+ if (unlikely(!handle))
+ return;
+
+ if (is_zero_page(handle)) {
+ /* No memory is allocated for zero filled pages */
+ zram-handle[index] = 0;
+ zram_stat_dec(zram-stats.pages_zero);
return;
}
+ size = zs_get_object_size(zram-mem_pool, handle);
if (unlikely(size max_zpage_size))
zram_stat_dec(zram-stats.bad_compress);
@@ -158,12 +144,10 @@ static void zram_free_page(struct zram *zram, size_t
index)
if (size = PAGE_SIZE / 2)
zram_stat_dec(zram-stats.good_compress);
- zram_stat64_sub(zram, zram-stats.compr_size,
- zram-table[index].size);
+ zram_stat64_sub(zram, zram-stats.compr_size, size);
zram_stat_dec(zram-stats.pages_stored);
- zram-table[index].handle = 0;
- zram-table[index].size = 0;
+ zram-handle[index] = 0;
}
static void handle_zero_page(struct bio_vec *bvec)
@@ -188,19 +172,20 @@ static int zram_decompress_page(struct zram *zram, char
*mem, u32 index)
int ret = LZO_E_OK;
size_t clen = PAGE_SIZE;
unsigned char *cmem;
- unsigned long handle = zram-table[index].handle;
+ unsigned long handle = zram-handle[index];
+ size_t objsize;
- if (!handle || zram_test_flag(zram, index, ZRAM_ZERO)) {
+ if (!handle || is_zero_page(handle)) {
memset(mem, 0, PAGE_SIZE);
return 0;
}
+ objsize = zs_get_object_size(zram-mem_pool, handle);
cmem = zs_map_object(zram-mem_pool, handle, ZS_MM_RO);
- if (zram-table[index].size == PAGE_SIZE)
+ if (objsize == PAGE_SIZE)
memcpy(mem, cmem, PAGE_SIZE);
else
- ret = lzo1x_decompress_safe(cmem, zram-table[index].size,
- mem, clen);
+ ret = lzo1x_decompress_safe(cmem, objsize, mem, clen);
zs_unmap_object(zram-mem_pool, handle);
/*
[PATCH v2 2/2] zram: reduce metadata overhead
Changelog v2 vs v1:
- Use is_zero_page() instead of direct handle comparison
- Use 1 as invalid handle value instead of -1 since handle
is unsigned and thus -1 may refer to a valid object. While 1
is guaranteed to be invalid since can never
refer to (end of) a valid object.
- Remove references to 'table' in comments and messages since
we just have a plain array of handles now.
For every allocated object, zram maintains the the handle, size,
flags and count fields. Of these, only the handle is required
since zsmalloc now provides the object size given the handle.
The flags field was needed only to mark a given page as zero-filled.
Instead of this field, we now use an invalid value (-1) to mark such
pages. Lastly, the count field was unused, so was simply removed.
Signed-off-by: Nitin Gupta
Reviewed-by: Jerome Marchand
---
drivers/staging/zram/zram_drv.c | 97 ---
drivers/staging/zram/zram_drv.h | 20 ++--
2 files changed, 43 insertions(+), 74 deletions(-)
diff --git a/drivers/staging/zram/zram_drv.c b/drivers/staging/zram/zram_drv.c
index f2a73bd..e6c9bec 100644
--- a/drivers/staging/zram/zram_drv.c
+++ b/drivers/staging/zram/zram_drv.c
@@ -71,24 +71,6 @@ static void zram_stat64_inc(struct zram *zram, u64 *v)
zram_stat64_add(zram, v, 1);
}
-static int zram_test_flag(struct zram *zram, u32 index,
- enum zram_pageflags flag)
-{
- return zram->table[index].flags & BIT(flag);
-}
-
-static void zram_set_flag(struct zram *zram, u32 index,
- enum zram_pageflags flag)
-{
- zram->table[index].flags |= BIT(flag);
-}
-
-static void zram_clear_flag(struct zram *zram, u32 index,
- enum zram_pageflags flag)
-{
- zram->table[index].flags &= ~BIT(flag);
-}
-
static int page_zero_filled(void *ptr)
{
unsigned int pos;
@@ -104,6 +86,11 @@ static int page_zero_filled(void *ptr)
return 1;
}
+static inline int is_zero_page(unsigned long handle)
+{
+ return handle == zero_page_handle;
+}
+
static void zram_set_disksize(struct zram *zram, size_t totalram_bytes)
{
if (!zram->disksize) {
@@ -135,21 +122,20 @@ static void zram_set_disksize(struct zram *zram, size_t
totalram_bytes)
static void zram_free_page(struct zram *zram, size_t index)
{
- unsigned long handle = zram->table[index].handle;
- u16 size = zram->table[index].size;
+ unsigned long handle = zram->handle[index];
+ size_t size;
- if (unlikely(!handle)) {
- /*
-* No memory is allocated for zero filled pages.
-* Simply clear zero page flag.
-*/
- if (zram_test_flag(zram, index, ZRAM_ZERO)) {
- zram_clear_flag(zram, index, ZRAM_ZERO);
- zram_stat_dec(>stats.pages_zero);
- }
+ if (unlikely(!handle))
+ return;
+
+ if (is_zero_page(handle)) {
+ /* No memory is allocated for zero filled pages */
+ zram->handle[index] = 0;
+ zram_stat_dec(>stats.pages_zero);
return;
}
+ size = zs_get_object_size(zram->mem_pool, handle);
if (unlikely(size > max_zpage_size))
zram_stat_dec(>stats.bad_compress);
@@ -158,12 +144,10 @@ static void zram_free_page(struct zram *zram, size_t
index)
if (size <= PAGE_SIZE / 2)
zram_stat_dec(>stats.good_compress);
- zram_stat64_sub(zram, >stats.compr_size,
- zram->table[index].size);
+ zram_stat64_sub(zram, >stats.compr_size, size);
zram_stat_dec(>stats.pages_stored);
- zram->table[index].handle = 0;
- zram->table[index].size = 0;
+ zram->handle[index] = 0;
}
static void handle_zero_page(struct bio_vec *bvec)
@@ -188,19 +172,20 @@ static int zram_decompress_page(struct zram *zram, char
*mem, u32 index)
int ret = LZO_E_OK;
size_t clen = PAGE_SIZE;
unsigned char *cmem;
- unsigned long handle = zram->table[index].handle;
+ unsigned long handle = zram->handle[index];
+ size_t objsize;
- if (!handle || zram_test_flag(zram, index, ZRAM_ZERO)) {
+ if (!handle || is_zero_page(handle)) {
memset(mem, 0, PAGE_SIZE);
return 0;
}
+ objsize = zs_get_object_size(zram->mem_pool, handle);
cmem = zs_map_object(zram->mem_pool, handle, ZS_MM_RO);
- if (zram->table[index].size == PAGE_SIZE)
+ if (objsize == PAGE_SIZE)
memcpy(mem, cmem, PAGE_SIZE);
else
- ret = lzo1x_decompress_safe(cmem, zram->table[index].size,
- mem, );
+ ret = lzo1x_decompress_safe(cmem, objsize, mem, );
zs_unmap_object(zram->mem_pool, handle);
/* Should NEVER happen. Return bio error if it does. */
@@
[PATCH v2 2/2] zram: reduce metadata overhead
Changelog v2 vs v1:
- Use is_zero_page() instead of direct handle comparison
- Use 1 as invalid handle value instead of -1 since handle
is unsigned and thus -1 may refer to a valid object. While 1
is guaranteed to be invalid since pfn:0,offset:1 can never
refer to (end of) a valid object.
- Remove references to 'table' in comments and messages since
we just have a plain array of handles now.
For every allocated object, zram maintains the the handle, size,
flags and count fields. Of these, only the handle is required
since zsmalloc now provides the object size given the handle.
The flags field was needed only to mark a given page as zero-filled.
Instead of this field, we now use an invalid value (-1) to mark such
pages. Lastly, the count field was unused, so was simply removed.
Signed-off-by: Nitin Gupta ngu...@vflare.org
Reviewed-by: Jerome Marchand jmarc...@redhat.com
---
drivers/staging/zram/zram_drv.c | 97 ---
drivers/staging/zram/zram_drv.h | 20 ++--
2 files changed, 43 insertions(+), 74 deletions(-)
diff --git a/drivers/staging/zram/zram_drv.c b/drivers/staging/zram/zram_drv.c
index f2a73bd..e6c9bec 100644
--- a/drivers/staging/zram/zram_drv.c
+++ b/drivers/staging/zram/zram_drv.c
@@ -71,24 +71,6 @@ static void zram_stat64_inc(struct zram *zram, u64 *v)
zram_stat64_add(zram, v, 1);
}
-static int zram_test_flag(struct zram *zram, u32 index,
- enum zram_pageflags flag)
-{
- return zram-table[index].flags BIT(flag);
-}
-
-static void zram_set_flag(struct zram *zram, u32 index,
- enum zram_pageflags flag)
-{
- zram-table[index].flags |= BIT(flag);
-}
-
-static void zram_clear_flag(struct zram *zram, u32 index,
- enum zram_pageflags flag)
-{
- zram-table[index].flags = ~BIT(flag);
-}
-
static int page_zero_filled(void *ptr)
{
unsigned int pos;
@@ -104,6 +86,11 @@ static int page_zero_filled(void *ptr)
return 1;
}
+static inline int is_zero_page(unsigned long handle)
+{
+ return handle == zero_page_handle;
+}
+
static void zram_set_disksize(struct zram *zram, size_t totalram_bytes)
{
if (!zram-disksize) {
@@ -135,21 +122,20 @@ static void zram_set_disksize(struct zram *zram, size_t
totalram_bytes)
static void zram_free_page(struct zram *zram, size_t index)
{
- unsigned long handle = zram-table[index].handle;
- u16 size = zram-table[index].size;
+ unsigned long handle = zram-handle[index];
+ size_t size;
- if (unlikely(!handle)) {
- /*
-* No memory is allocated for zero filled pages.
-* Simply clear zero page flag.
-*/
- if (zram_test_flag(zram, index, ZRAM_ZERO)) {
- zram_clear_flag(zram, index, ZRAM_ZERO);
- zram_stat_dec(zram-stats.pages_zero);
- }
+ if (unlikely(!handle))
+ return;
+
+ if (is_zero_page(handle)) {
+ /* No memory is allocated for zero filled pages */
+ zram-handle[index] = 0;
+ zram_stat_dec(zram-stats.pages_zero);
return;
}
+ size = zs_get_object_size(zram-mem_pool, handle);
if (unlikely(size max_zpage_size))
zram_stat_dec(zram-stats.bad_compress);
@@ -158,12 +144,10 @@ static void zram_free_page(struct zram *zram, size_t
index)
if (size = PAGE_SIZE / 2)
zram_stat_dec(zram-stats.good_compress);
- zram_stat64_sub(zram, zram-stats.compr_size,
- zram-table[index].size);
+ zram_stat64_sub(zram, zram-stats.compr_size, size);
zram_stat_dec(zram-stats.pages_stored);
- zram-table[index].handle = 0;
- zram-table[index].size = 0;
+ zram-handle[index] = 0;
}
static void handle_zero_page(struct bio_vec *bvec)
@@ -188,19 +172,20 @@ static int zram_decompress_page(struct zram *zram, char
*mem, u32 index)
int ret = LZO_E_OK;
size_t clen = PAGE_SIZE;
unsigned char *cmem;
- unsigned long handle = zram-table[index].handle;
+ unsigned long handle = zram-handle[index];
+ size_t objsize;
- if (!handle || zram_test_flag(zram, index, ZRAM_ZERO)) {
+ if (!handle || is_zero_page(handle)) {
memset(mem, 0, PAGE_SIZE);
return 0;
}
+ objsize = zs_get_object_size(zram-mem_pool, handle);
cmem = zs_map_object(zram-mem_pool, handle, ZS_MM_RO);
- if (zram-table[index].size == PAGE_SIZE)
+ if (objsize == PAGE_SIZE)
memcpy(mem, cmem, PAGE_SIZE);
else
- ret = lzo1x_decompress_safe(cmem, zram-table[index].size,
- mem, clen);
+ ret = lzo1x_decompress_safe(cmem, objsize, mem, clen);
zs_unmap_object(zram-mem_pool, handle);
/*
