RE: [PATCH 1/3] zram: force disksize setting before using zram

2013-01-18 Thread Dan Magenheimer
> From: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org]
> Sent: Thursday, January 17, 2013 5:42 PM
> To: Minchan Kim
> Cc: linux...@kvack.org; linux-kernel@vger.kernel.org; Nitin Gupta; Seth 
> Jennings; Dan Magenheimer;
> Konrad Rzeszutek Wilk; Jerome Marchand; Pekka Enberg
> Subject: Re: [PATCH 1/3] zram: force disksize setting before using zram
> 
> On Thu, Jan 17, 2013 at 11:12:47AM +0900, Minchan Kim wrote:
> > Now zram document syas "set disksize is optional"
> > but partly it's wrong. When you try to use zram firstly after
> > booting, you must set disksize, otherwise zram can't work because
> > zram gendisk's size is 0. But once you do it, you can use zram freely
> > after reset because reset doesn't reset to zero paradoxically.
> > So in this time, disksize setting is optional.:(
> > It's inconsitent for user behavior and not straightforward.
> >
> > This patch forces always setting disksize firstly before using zram.
> > Yes. It changes current behavior so someone could complain when
> > he upgrades zram. Apparently it could be a problem if zram is mainline
> > but it still lives in staging so behavior could be changed for right
> > way to go. Let them excuse.
> 
> I don't know about changing this behavior.  I need some acks from some
> of the other zram developers before I can take this, or any of the other
> patches in this series.

I'm not officially a zram developer, but I have used it and I
am knowledgeable about in-kernel compression and know the specific
problem being fixed here.  Unless/until compression is much
more tightly integrated into MM policies and "z*" can manage
space more dynamically, Minchan's patch seems to be a good way
to go, especially since zram has found a solid niche in the
embedded (no swap disk) community.  So FWIW:

Acked-by: Dan Magenheimer 
--
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 1/3] zram: force disksize setting before using zram

2013-01-17 Thread Greg Kroah-Hartman
On Thu, Jan 17, 2013 at 11:12:47AM +0900, Minchan Kim wrote:
> Now zram document syas "set disksize is optional"
> but partly it's wrong. When you try to use zram firstly after
> booting, you must set disksize, otherwise zram can't work because
> zram gendisk's size is 0. But once you do it, you can use zram freely
> after reset because reset doesn't reset to zero paradoxically.
> So in this time, disksize setting is optional.:(
> It's inconsitent for user behavior and not straightforward.
> 
> This patch forces always setting disksize firstly before using zram.
> Yes. It changes current behavior so someone could complain when
> he upgrades zram. Apparently it could be a problem if zram is mainline
> but it still lives in staging so behavior could be changed for right
> way to go. Let them excuse.

I don't know about changing this behavior.  I need some acks from some
of the other zram developers before I can take this, or any of the other
patches in this series.

thanks,

greg k-h
--
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 1/3] zram: force disksize setting before using zram

2013-01-16 Thread Minchan Kim
Now zram document syas "set disksize is optional"
but partly it's wrong. When you try to use zram firstly after
booting, you must set disksize, otherwise zram can't work because
zram gendisk's size is 0. But once you do it, you can use zram freely
after reset because reset doesn't reset to zero paradoxically.
So in this time, disksize setting is optional.:(
It's inconsitent for user behavior and not straightforward.

This patch forces always setting disksize firstly before using zram.
Yes. It changes current behavior so someone could complain when
he upgrades zram. Apparently it could be a problem if zram is mainline
but it still lives in staging so behavior could be changed for right
way to go. Let them excuse.

Signed-off-by: Minchan Kim 
---
 drivers/staging/zram/zram.txt |   27 +-
 drivers/staging/zram/zram_drv.c   |   55 +
 drivers/staging/zram/zram_drv.h   |5 +---
 drivers/staging/zram/zram_sysfs.c |6 +---
 4 files changed, 35 insertions(+), 58 deletions(-)

diff --git a/drivers/staging/zram/zram.txt b/drivers/staging/zram/zram.txt
index 5f75d29..765d790 100644
--- a/drivers/staging/zram/zram.txt
+++ b/drivers/staging/zram/zram.txt
@@ -23,17 +23,17 @@ Following shows a typical sequence of steps for using zram.
This creates 4 devices: /dev/zram{0,1,2,3}
(num_devices parameter is optional. Default: 1)
 
-2) Set Disksize (Optional):
-   Set disk size by writing the value to sysfs node 'disksize'
-   (in bytes). If disksize is not given, default value of 25%
-   of RAM is used.
-
-   # Initialize /dev/zram0 with 50MB disksize
-   echo $((50*1024*1024)) > /sys/block/zram0/disksize
-
-   NOTE: disksize cannot be changed if the disk contains any
-   data. So, for such a disk, you need to issue 'reset' (see below)
-   before you can change its disksize.
+2) Set Disksize
+Set disk size by writing the value to sysfs node 'disksize'.
+The value can be either in bytes or you can use mem suffixes.
+Examples:
+# Initialize /dev/zram0 with 50MB disksize
+echo $((50*1024*1024)) > /sys/block/zram0/disksize
+
+# Using mem suffixes
+echo 256K > /sys/block/zram0/disksize
+echo 512M > /sys/block/zram0/disksize
+echo 1G > /sys/block/zram0/disksize
 
 3) Activate:
mkswap /dev/zram0
@@ -65,8 +65,9 @@ Following shows a typical sequence of steps for using zram.
echo 1 > /sys/block/zram0/reset
echo 1 > /sys/block/zram1/reset
 
-   (This frees all the memory allocated for the given device).
-
+   This frees all the memory allocated for the given device and
+   resets the disksize to zero. You must set the disksize again
+   before reusing the device.
 
 Please report any problems at:
  - Mailing list: linux-mm-cc at laptop dot org
diff --git a/drivers/staging/zram/zram_drv.c b/drivers/staging/zram/zram_drv.c
index f2a73bd..e583a80 100644
--- a/drivers/staging/zram/zram_drv.c
+++ b/drivers/staging/zram/zram_drv.c
@@ -104,35 +104,6 @@ static int page_zero_filled(void *ptr)
return 1;
 }
 
-static void zram_set_disksize(struct zram *zram, size_t totalram_bytes)
-{
-   if (!zram->disksize) {
-   pr_info(
-   "disk size not provided. You can use disksize_kb module "
-   "param to specify size.\nUsing default: (%u%% of RAM).\n",
-   default_disksize_perc_ram
-   );
-   zram->disksize = default_disksize_perc_ram *
-   (totalram_bytes / 100);
-   }
-
-   if (zram->disksize > 2 * (totalram_bytes)) {
-   pr_info(
-   "There is little point creating a zram of greater than "
-   "twice the size of memory since we expect a 2:1 compression "
-   "ratio. Note that zram uses about 0.1%% of the size of "
-   "the disk when not in use so a huge zram is "
-   "wasteful.\n"
-   "\tMemory Size: %zu kB\n"
-   "\tSize you selected: %llu kB\n"
-   "Continuing anyway ...\n",
-   totalram_bytes >> 10, zram->disksize
-   );
-   }
-
-   zram->disksize &= PAGE_MASK;
-}
-
 static void zram_free_page(struct zram *zram, size_t index)
 {
unsigned long handle = zram->table[index].handle;
@@ -506,6 +477,9 @@ void __zram_reset_device(struct zram *zram)
 {
size_t index;
 
+   if (!zram->init_done)
+   goto out;
+
zram->init_done = 0;
 
/* Free various per-device buffers */
@@ -532,8 +506,9 @@ void __zram_reset_device(struct zram *zram)
 
/* Reset stats */
memset(&zram->stats, 0, sizeof(zram->stats));
-
+out:
zram->disksize = 0;
+   set_capacity(zram->disk, 0);
 }
 
 void zram_reset_device(struct zram *zram)
@@ -549,13 +524,24 @@ int zram_init_device(struct zram *zram)
size_t n

[PATCH 1/3] zram: force disksize setting before using zram

2012-11-27 Thread Minchan Kim
Now zram document syas "set disksize is optional"
but partly it's wrong. When you try to use zram firstly after
booting, you must set disksize, otherwise zram can't work because
zram gendisk's size is 0. But once you do it, you can use zram freely
after reset because reset doesn't reset to zero paradoxically.
So in this time, disksize setting is optional.:(
It's inconsitent for user behavior and not straightforward.

This patch forces always setting disksize firstly before using zram.
Yes. It changes current behavior so someone could complain when
he upgrades zram. Apparently it could be a problem if zram is mainline
but it still lives in staging so behavior could be changed for right
way to go. Let them excuse.

Signed-off-by: Minchan Kim 
---
 drivers/staging/zram/zram.txt |   27 +-
 drivers/staging/zram/zram_drv.c   |   55 +
 drivers/staging/zram/zram_drv.h   |5 +---
 drivers/staging/zram/zram_sysfs.c |6 +---
 4 files changed, 35 insertions(+), 58 deletions(-)

diff --git a/drivers/staging/zram/zram.txt b/drivers/staging/zram/zram.txt
index 5f75d29..765d790 100644
--- a/drivers/staging/zram/zram.txt
+++ b/drivers/staging/zram/zram.txt
@@ -23,17 +23,17 @@ Following shows a typical sequence of steps for using zram.
This creates 4 devices: /dev/zram{0,1,2,3}
(num_devices parameter is optional. Default: 1)
 
-2) Set Disksize (Optional):
-   Set disk size by writing the value to sysfs node 'disksize'
-   (in bytes). If disksize is not given, default value of 25%
-   of RAM is used.
-
-   # Initialize /dev/zram0 with 50MB disksize
-   echo $((50*1024*1024)) > /sys/block/zram0/disksize
-
-   NOTE: disksize cannot be changed if the disk contains any
-   data. So, for such a disk, you need to issue 'reset' (see below)
-   before you can change its disksize.
+2) Set Disksize
+Set disk size by writing the value to sysfs node 'disksize'.
+The value can be either in bytes or you can use mem suffixes.
+Examples:
+# Initialize /dev/zram0 with 50MB disksize
+echo $((50*1024*1024)) > /sys/block/zram0/disksize
+
+# Using mem suffixes
+echo 256K > /sys/block/zram0/disksize
+echo 512M > /sys/block/zram0/disksize
+echo 1G > /sys/block/zram0/disksize
 
 3) Activate:
mkswap /dev/zram0
@@ -65,8 +65,9 @@ Following shows a typical sequence of steps for using zram.
echo 1 > /sys/block/zram0/reset
echo 1 > /sys/block/zram1/reset
 
-   (This frees all the memory allocated for the given device).
-
+   This frees all the memory allocated for the given device and
+   resets the disksize to zero. You must set the disksize again
+   before reusing the device.
 
 Please report any problems at:
  - Mailing list: linux-mm-cc at laptop dot org
diff --git a/drivers/staging/zram/zram_drv.c b/drivers/staging/zram/zram_drv.c
index fb4a7c9..b036dcc 100644
--- a/drivers/staging/zram/zram_drv.c
+++ b/drivers/staging/zram/zram_drv.c
@@ -104,35 +104,6 @@ static int page_zero_filled(void *ptr)
return 1;
 }
 
-static void zram_set_disksize(struct zram *zram, size_t totalram_bytes)
-{
-   if (!zram->disksize) {
-   pr_info(
-   "disk size not provided. You can use disksize_kb module "
-   "param to specify size.\nUsing default: (%u%% of RAM).\n",
-   default_disksize_perc_ram
-   );
-   zram->disksize = default_disksize_perc_ram *
-   (totalram_bytes / 100);
-   }
-
-   if (zram->disksize > 2 * (totalram_bytes)) {
-   pr_info(
-   "There is little point creating a zram of greater than "
-   "twice the size of memory since we expect a 2:1 compression "
-   "ratio. Note that zram uses about 0.1%% of the size of "
-   "the disk when not in use so a huge zram is "
-   "wasteful.\n"
-   "\tMemory Size: %zu kB\n"
-   "\tSize you selected: %llu kB\n"
-   "Continuing anyway ...\n",
-   totalram_bytes >> 10, zram->disksize
-   );
-   }
-
-   zram->disksize &= PAGE_MASK;
-}
-
 static void zram_free_page(struct zram *zram, size_t index)
 {
unsigned long handle = zram->table[index].handle;
@@ -497,6 +468,9 @@ void __zram_reset_device(struct zram *zram)
 {
size_t index;
 
+   if (!zram->init_done)
+   goto out;
+
zram->init_done = 0;
 
/* Free various per-device buffers */
@@ -523,8 +497,9 @@ void __zram_reset_device(struct zram *zram)
 
/* Reset stats */
memset(&zram->stats, 0, sizeof(zram->stats));
-
+out:
zram->disksize = 0;
+   set_capacity(zram->disk, 0);
 }
 
 void zram_reset_device(struct zram *zram)
@@ -540,13 +515,24 @@ int zram_init_device(struct zram *zram)
size_t n