Re: [PATCH 1/2] zsmalloc: zsmalloc: use unsigned long instead ofvoid *

From: Nitin Gupta
Date: Wed Jun 06 2012 - 00:00:27 EST

Next message: Andrew Morton: "Re: [PATCH v2] remove no longer use of pdflush interface"
Previous message: Steven Rostedt: "[RFC][PATCH 08/12] kprobes: Inverse taking of module_mutex with kprobe_mutex"
In reply to: Dan Magenheimer: "RE: [PATCH 2/2] zram: clean up handle"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On 06/05/2012 12:23 AM, Minchan Kim wrote:

> We should use unsigned long as handle instead of void * to avoid any
> confusion. Without this, users may just treat zs_malloc return value as
> a pointer and try to deference it.
>
> This patch passed compile test(zram, zcache and ramster) and zram is
> tested on qemu.
>
> changelog
> * from v1
> - change zcache's zv_create return value
> - baesd on next-20120604
>
> Cc: Nitin Gupta <ngupta@xxxxxxxxxx>
> Cc: Dan Magenheimer <dan.magenheimer@xxxxxxxxxx>
> Acked-by: Seth Jennings <sjenning@xxxxxxxxxxxxxxxxxx>
> Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> Signed-off-by: Minchan Kim <minchan@xxxxxxxxxx>
> ---
> drivers/staging/zcache/zcache-main.c | 12 ++++++------
> drivers/staging/zram/zram_drv.c | 16 ++++++++--------
> drivers/staging/zram/zram_drv.h | 2 +-
> drivers/staging/zsmalloc/zsmalloc-main.c | 24 ++++++++++++------------
> drivers/staging/zsmalloc/zsmalloc.h | 8 ++++----
> 5 files changed, 31 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/staging/zcache/zcache-main.c b/drivers/staging/zcache/zcache-main.c
> index 2734dac..d0141fbc 100644
> --- a/drivers/staging/zcache/zcache-main.c
> +++ b/drivers/staging/zcache/zcache-main.c
> @@ -693,14 +693,14 @@ static unsigned int zv_max_mean_zsize = (PAGE_SIZE / 8) * 5;
> static atomic_t zv_curr_dist_counts[NCHUNKS];
> static atomic_t zv_cumul_dist_counts[NCHUNKS];
>
> -static struct zv_hdr *zv_create(struct zs_pool *pool, uint32_t pool_id,
> +static unsigned long zv_create(struct zs_pool *pool, uint32_t pool_id,
> struct tmem_oid *oid, uint32_t index,
> void *cdata, unsigned clen)
> {
> struct zv_hdr *zv;
> u32 size = clen + sizeof(struct zv_hdr);
> int chunks = (size + (CHUNK_SIZE - 1)) >> CHUNK_SHIFT;
> - void *handle = NULL;
> + unsigned long handle = 0;
>
> BUG_ON(!irqs_disabled());
> BUG_ON(chunks >= NCHUNKS);
> @@ -721,7 +721,7 @@ out:
> return handle;
> }
>
> -static void zv_free(struct zs_pool *pool, void *handle)
> +static void zv_free(struct zs_pool *pool, unsigned long handle)
> {
> unsigned long flags;
> struct zv_hdr *zv;
> @@ -743,7 +743,7 @@ static void zv_free(struct zs_pool *pool, void *handle)
> local_irq_restore(flags);
> }
>
> -static void zv_decompress(struct page *page, void *handle)
> +static void zv_decompress(struct page *page, unsigned long handle)
> {
> unsigned int clen = PAGE_SIZE;
> char *to_va;
> @@ -1247,7 +1247,7 @@ static int zcache_pampd_get_data(char *data, size_t *bufsize, bool raw,
> int ret = 0;
>
> BUG_ON(is_ephemeral(pool));
> - zv_decompress((struct page *)(data), pampd);
> + zv_decompress((struct page *)(data), (unsigned long)pampd);
> return ret;
> }
>
> @@ -1282,7 +1282,7 @@ static void zcache_pampd_free(void *pampd, struct tmem_pool *pool,
> atomic_dec(&zcache_curr_eph_pampd_count);
> BUG_ON(atomic_read(&zcache_curr_eph_pampd_count) < 0);
> } else {
> - zv_free(cli->zspool, pampd);
> + zv_free(cli->zspool, (unsigned long)pampd);
> atomic_dec(&zcache_curr_pers_pampd_count);
> BUG_ON(atomic_read(&zcache_curr_pers_pampd_count) < 0);
> }
> diff --git a/drivers/staging/zram/zram_drv.c b/drivers/staging/zram/zram_drv.c
> index 685d612..abd69d1 100644
> --- a/drivers/staging/zram/zram_drv.c
> +++ b/drivers/staging/zram/zram_drv.c
> @@ -135,7 +135,7 @@ static void zram_set_disksize(struct zram *zram, size_t totalram_bytes)
>
> static void zram_free_page(struct zram *zram, size_t index)
> {
> - void *handle = zram->table[index].handle;
> + unsigned long handle = zram->table[index].handle;
>
> if (unlikely(!handle)) {
> /*
> @@ -150,7 +150,7 @@ static void zram_free_page(struct zram *zram, size_t index)
> }
>
> if (unlikely(zram_test_flag(zram, index, ZRAM_UNCOMPRESSED))) {
> - __free_page(handle);
> + __free_page((struct page *)handle);
> zram_clear_flag(zram, index, ZRAM_UNCOMPRESSED);
> zram_stat_dec(&zram->stats.pages_expand);
> goto out;
> @@ -166,7 +166,7 @@ out:
> zram->table[index].size);
> zram_stat_dec(&zram->stats.pages_stored);
>
> - zram->table[index].handle = NULL;
> + zram->table[index].handle = 0;
> zram->table[index].size = 0;
> }
>
> @@ -189,7 +189,7 @@ static void handle_uncompressed_page(struct zram *zram, struct bio_vec *bvec,
> unsigned char *user_mem, *cmem;
>
> user_mem = kmap_atomic(page);
> - cmem = kmap_atomic(zram->table[index].handle);
> + cmem = kmap_atomic((struct page *)zram->table[index].handle);
>
> memcpy(user_mem + bvec->bv_offset, cmem + offset, bvec->bv_len);
> kunmap_atomic(cmem);
> @@ -317,7 +317,7 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
> int ret;
> u32 store_offset;
> size_t clen;
> - void *handle;
> + unsigned long handle;
> struct zobj_header *zheader;
> struct page *page, *page_store;
> unsigned char *user_mem, *cmem, *src, *uncmem = NULL;
> @@ -399,7 +399,7 @@ static int zram_bvec_write(struct zram *zram, struct bio_vec *bvec, u32 index,
> store_offset = 0;
> zram_set_flag(zram, index, ZRAM_UNCOMPRESSED);
> zram_stat_inc(&zram->stats.pages_expand);
> - handle = page_store;
> + handle = (unsigned long)page_store;
> src = kmap_atomic(page);
> cmem = kmap_atomic(page_store);
> goto memstore;
> @@ -592,12 +592,12 @@ void __zram_reset_device(struct zram *zram)
>
> /* Free all pages that are still in this zram device */
> for (index = 0; index < zram->disksize >> PAGE_SHIFT; index++) {
> - void *handle = zram->table[index].handle;
> + unsigned long handle = zram->table[index].handle;
> if (!handle)
> continue;
>
> if (unlikely(zram_test_flag(zram, index, ZRAM_UNCOMPRESSED)))
> - __free_page(handle);
> + __free_page((struct page *)handle);
> else
> zs_free(zram->mem_pool, handle);
> }
> diff --git a/drivers/staging/zram/zram_drv.h b/drivers/staging/zram/zram_drv.h
> index fbe8ac9..7a7e256 100644
> --- a/drivers/staging/zram/zram_drv.h
> +++ b/drivers/staging/zram/zram_drv.h
> @@ -81,7 +81,7 @@ enum zram_pageflags {
>
> /* Allocated for each disk page */
> struct table {
> - void *handle;
> + unsigned long handle;
> u16 size; /* object size (excluding header) */
> u8 count; /* object ref count (not yet used) */
> u8 flags;
> diff --git a/drivers/staging/zsmalloc/zsmalloc-main.c b/drivers/staging/zsmalloc/zsmalloc-main.c
> index 4496737..fcbe83d 100644
> --- a/drivers/staging/zsmalloc/zsmalloc-main.c
> +++ b/drivers/staging/zsmalloc/zsmalloc-main.c
> @@ -247,10 +247,10 @@ static void *obj_location_to_handle(struct page *page, unsigned long obj_idx)
> }
>
> /* Decode <page, obj_idx> pair from the given object handle */
> -static void obj_handle_to_location(void *handle, struct page **page,
> +static void obj_handle_to_location(unsigned long handle, struct page **page,
> unsigned long *obj_idx)
> {
> - unsigned long hval = (unsigned long)handle;
> + unsigned long hval = handle;
>
> *page = pfn_to_page(hval >> OBJ_INDEX_BITS);
> *obj_idx = hval & OBJ_INDEX_MASK;

hval looks redundant now.

Rest of the changes look good to me. Thanks!

Nitin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Next message: Andrew Morton: "Re: [PATCH v2] remove no longer use of pdflush interface"
Previous message: Steven Rostedt: "[RFC][PATCH 08/12] kprobes: Inverse taking of module_mutex with kprobe_mutex"
In reply to: Dan Magenheimer: "RE: [PATCH 2/2] zram: clean up handle"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]