linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 2/2] zswap: update/document boot parameters
@ 2013-06-20  8:29 Bob Liu
  2013-06-20 14:48 ` Seth Jennings
  0 siblings, 1 reply; 6+ messages in thread
From: Bob Liu @ 2013-06-20  8:29 UTC (permalink / raw)
  To: akpm; +Cc: sjenning, linux-mm, konrad.wilk, Bob Liu

The current parameters of zswap are not straightforward.
Changed them to start with zswap* and documented them.

Signed-off-by: Bob Liu <bob.liu@oracle.com>
---
 Documentation/kernel-parameters.txt |    8 ++++++++
 mm/zswap.c                          |   27 +++++++++++++++++++++++----
 2 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 2fe6e76..07642fd 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -3367,6 +3367,14 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			Format:
 			<irq>,<irq_mask>,<io>,<full_duplex>,<do_sound>,<lockup_hack>[,<irq2>[,<irq3>[,<irq4>]]]
 
+	zswap 		Enable compressed cache for swap pages support which
+			is disabled by default.
+	zswapcompressor=
+			Select which compressor to be used by zswap.
+			The default compressor is lzo.
+	zswap_maxpool_percent=
+			Select how may percent of total memory can be used to
+			store comprssed pages. The default percent is 20%.
 ______________________________________________________________________
 
 TODO:
diff --git a/mm/zswap.c b/mm/zswap.c
index 7fe2b1b..8ec1360 100644
--- a/mm/zswap.c
+++ b/mm/zswap.c
@@ -77,17 +77,13 @@ static u64 zswap_duplicate_entry;
 **********************************/
 /* Enable/disable zswap (disabled by default, fixed at boot for now) */
 static bool zswap_enabled __read_mostly;
-module_param_named(enabled, zswap_enabled, bool, 0);
 
 /* Compressor to be used by zswap (fixed at boot for now) */
 #define ZSWAP_COMPRESSOR_DEFAULT "lzo"
 static char *zswap_compressor = ZSWAP_COMPRESSOR_DEFAULT;
-module_param_named(compressor, zswap_compressor, charp, 0);
 
 /* The maximum percentage of memory that the compressed pool can occupy */
 static unsigned int zswap_max_pool_percent = 20;
-module_param_named(max_pool_percent,
-			zswap_max_pool_percent, uint, 0644);
 
 /*********************************
 * compression functions
@@ -914,6 +910,29 @@ static int __init zswap_debugfs_init(void)
 static void __exit zswap_debugfs_exit(void) { }
 #endif
 
+static int __init enable_zswap(char *s)
+{
+	zswap_enabled = true;
+	return 1;
+}
+__setup("zswap", enable_zswap);
+
+static int __init setup_zswap_compressor(char *s)
+{
+	strlcpy(zswap_compressor, s, sizeof(zswap_compressor));
+	zswap_enabled = true;
+	return 1;
+}
+__setup("zswapcompressor=", setup_zswap_compressor);
+
+static int __init setup_zswap_max_pool_percent(char *s)
+{
+	get_option(&s, &zswap_max_pool_percent);
+	zswap_enabled = true;
+	return 1;
+}
+__setup("zswap_maxpool_percent=", setup_zswap_max_pool_percent);
+
 /*********************************
 * module init and exit
 **********************************/
-- 
1.7.10.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] zswap: update/document boot parameters
  2013-06-20  8:29 [PATCH 2/2] zswap: update/document boot parameters Bob Liu
@ 2013-06-20 14:48 ` Seth Jennings
  2013-06-20 23:20   ` Bob Liu
                     ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Seth Jennings @ 2013-06-20 14:48 UTC (permalink / raw)
  To: Bob Liu; +Cc: akpm, linux-mm, konrad.wilk, Bob Liu

On Thu, Jun 20, 2013 at 04:29:09PM +0800, Bob Liu wrote:
> The current parameters of zswap are not straightforward.
> Changed them to start with zswap* and documented them.

Thanks for the patch!

However, I think you might be missing that using module_param(_named) allows
access on the kernel boot line with <modulename>.<moduleparam> syntax.  So
"zswap" already has to be the parameter string as it is the name of the module
to whom the parameters belong.

For example, your patch just changes the boot parameter from
zswap.max_pool_percent to zswap_maxpool_percent.  That doesn't add any clarity
IMO.

Yes, zswap isn't able to be a module right now.  But there is no harm in using
the module framework to provide standardized access to zswap parameters. Plus,
the day might come when zswap can be a module.

As far as documenting them in kernel-parameters.txt, this was mentioned before
and I it was decided to not do that since module parameters are typically not
documented there. However, since zswap is currently not buildable as a module,
I could see where I case could be made for documenting them there.

If you wanted to document them with their <modulename>.<moduleparam> syntax,
I wouldn't be opposed to it.

Seth

> 
> Signed-off-by: Bob Liu <bob.liu@oracle.com>
> ---
>  Documentation/kernel-parameters.txt |    8 ++++++++
>  mm/zswap.c                          |   27 +++++++++++++++++++++++----
>  2 files changed, 31 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index 2fe6e76..07642fd 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -3367,6 +3367,14 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>  			Format:
>  			<irq>,<irq_mask>,<io>,<full_duplex>,<do_sound>,<lockup_hack>[,<irq2>[,<irq3>[,<irq4>]]]
> 
> +	zswap 		Enable compressed cache for swap pages support which
> +			is disabled by default.
> +	zswapcompressor=
> +			Select which compressor to be used by zswap.
> +			The default compressor is lzo.
> +	zswap_maxpool_percent=
> +			Select how may percent of total memory can be used to
> +			store comprssed pages. The default percent is 20%.
>  ______________________________________________________________________
> 
>  TODO:
> diff --git a/mm/zswap.c b/mm/zswap.c
> index 7fe2b1b..8ec1360 100644
> --- a/mm/zswap.c
> +++ b/mm/zswap.c
> @@ -77,17 +77,13 @@ static u64 zswap_duplicate_entry;
>  **********************************/
>  /* Enable/disable zswap (disabled by default, fixed at boot for now) */
>  static bool zswap_enabled __read_mostly;
> -module_param_named(enabled, zswap_enabled, bool, 0);
> 
>  /* Compressor to be used by zswap (fixed at boot for now) */
>  #define ZSWAP_COMPRESSOR_DEFAULT "lzo"
>  static char *zswap_compressor = ZSWAP_COMPRESSOR_DEFAULT;
> -module_param_named(compressor, zswap_compressor, charp, 0);
> 
>  /* The maximum percentage of memory that the compressed pool can occupy */
>  static unsigned int zswap_max_pool_percent = 20;
> -module_param_named(max_pool_percent,
> -			zswap_max_pool_percent, uint, 0644);
> 
>  /*********************************
>  * compression functions
> @@ -914,6 +910,29 @@ static int __init zswap_debugfs_init(void)
>  static void __exit zswap_debugfs_exit(void) { }
>  #endif
> 
> +static int __init enable_zswap(char *s)
> +{
> +	zswap_enabled = true;
> +	return 1;
> +}
> +__setup("zswap", enable_zswap);
> +
> +static int __init setup_zswap_compressor(char *s)
> +{
> +	strlcpy(zswap_compressor, s, sizeof(zswap_compressor));
> +	zswap_enabled = true;
> +	return 1;
> +}
> +__setup("zswapcompressor=", setup_zswap_compressor);
> +
> +static int __init setup_zswap_max_pool_percent(char *s)
> +{
> +	get_option(&s, &zswap_max_pool_percent);
> +	zswap_enabled = true;
> +	return 1;
> +}
> +__setup("zswap_maxpool_percent=", setup_zswap_max_pool_percent);
> +
>  /*********************************
>  * module init and exit
>  **********************************/
> -- 
> 1.7.10.4
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] zswap: update/document boot parameters
  2013-06-20 14:48 ` Seth Jennings
@ 2013-06-20 23:20   ` Bob Liu
  2013-06-20 23:51   ` Wanpeng Li
  2013-06-20 23:51   ` Wanpeng Li
  2 siblings, 0 replies; 6+ messages in thread
From: Bob Liu @ 2013-06-20 23:20 UTC (permalink / raw)
  To: Seth Jennings; +Cc: Bob Liu, akpm, linux-mm, konrad.wilk



On 06/20/2013 10:48 PM, Seth Jennings wrote:
> On Thu, Jun 20, 2013 at 04:29:09PM +0800, Bob Liu wrote:
>> The current parameters of zswap are not straightforward.
>> Changed them to start with zswap* and documented them.
> 
> Thanks for the patch!
> 
> However, I think you might be missing that using module_param(_named) allows
> access on the kernel boot line with <modulename>.<moduleparam> syntax.  So

Oh, yes.
Sorry for the noise, I missed the meaning of module_param.

-- 
Regards,
-Bob

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] zswap: update/document boot parameters
  2013-06-20 14:48 ` Seth Jennings
  2013-06-20 23:20   ` Bob Liu
@ 2013-06-20 23:51   ` Wanpeng Li
  2013-06-21 14:47     ` Seth Jennings
  2013-06-20 23:51   ` Wanpeng Li
  2 siblings, 1 reply; 6+ messages in thread
From: Wanpeng Li @ 2013-06-20 23:51 UTC (permalink / raw)
  To: Seth Jennings; +Cc: akpm, linux-mm, konrad.wilk, Bob Liu

On Thu, Jun 20, 2013 at 09:48:26AM -0500, Seth Jennings wrote:
>On Thu, Jun 20, 2013 at 04:29:09PM +0800, Bob Liu wrote:
>> The current parameters of zswap are not straightforward.
>> Changed them to start with zswap* and documented them.
>
>Thanks for the patch!
>
>However, I think you might be missing that using module_param(_named) allows
>access on the kernel boot line with <modulename>.<moduleparam> syntax.  So
>"zswap" already has to be the parameter string as it is the name of the module
>to whom the parameters belong.
>
>For example, your patch just changes the boot parameter from
>zswap.max_pool_percent to zswap_maxpool_percent.  That doesn't add any clarity
>IMO.
>

Hi Seth,

>Yes, zswap isn't able to be a module right now.  But there is no harm in using
>the module framework to provide standardized access to zswap parameters. Plus,
>the day might come when zswap can be a module.
>

Do you plan to do zswap modulization? Otherwise I am happy to do that.
;-)

Regards,
Wanpeng Li 

>As far as documenting them in kernel-parameters.txt, this was mentioned before
>and I it was decided to not do that since module parameters are typically not
>documented there. However, since zswap is currently not buildable as a module,
>I could see where I case could be made for documenting them there.
>
>If you wanted to document them with their <modulename>.<moduleparam> syntax,
>I wouldn't be opposed to it.
>
>Seth
>
>> 
>> Signed-off-by: Bob Liu <bob.liu@oracle.com>
>> ---
>>  Documentation/kernel-parameters.txt |    8 ++++++++
>>  mm/zswap.c                          |   27 +++++++++++++++++++++++----
>>  2 files changed, 31 insertions(+), 4 deletions(-)
>> 
>> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
>> index 2fe6e76..07642fd 100644
>> --- a/Documentation/kernel-parameters.txt
>> +++ b/Documentation/kernel-parameters.txt
>> @@ -3367,6 +3367,14 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>>  			Format:
>>  			<irq>,<irq_mask>,<io>,<full_duplex>,<do_sound>,<lockup_hack>[,<irq2>[,<irq3>[,<irq4>]]]
>> 
>> +	zswap 		Enable compressed cache for swap pages support which
>> +			is disabled by default.
>> +	zswapcompressor=
>> +			Select which compressor to be used by zswap.
>> +			The default compressor is lzo.
>> +	zswap_maxpool_percent=
>> +			Select how may percent of total memory can be used to
>> +			store comprssed pages. The default percent is 20%.
>>  ______________________________________________________________________
>> 
>>  TODO:
>> diff --git a/mm/zswap.c b/mm/zswap.c
>> index 7fe2b1b..8ec1360 100644
>> --- a/mm/zswap.c
>> +++ b/mm/zswap.c
>> @@ -77,17 +77,13 @@ static u64 zswap_duplicate_entry;
>>  **********************************/
>>  /* Enable/disable zswap (disabled by default, fixed at boot for now) */
>>  static bool zswap_enabled __read_mostly;
>> -module_param_named(enabled, zswap_enabled, bool, 0);
>> 
>>  /* Compressor to be used by zswap (fixed at boot for now) */
>>  #define ZSWAP_COMPRESSOR_DEFAULT "lzo"
>>  static char *zswap_compressor = ZSWAP_COMPRESSOR_DEFAULT;
>> -module_param_named(compressor, zswap_compressor, charp, 0);
>> 
>>  /* The maximum percentage of memory that the compressed pool can occupy */
>>  static unsigned int zswap_max_pool_percent = 20;
>> -module_param_named(max_pool_percent,
>> -			zswap_max_pool_percent, uint, 0644);
>> 
>>  /*********************************
>>  * compression functions
>> @@ -914,6 +910,29 @@ static int __init zswap_debugfs_init(void)
>>  static void __exit zswap_debugfs_exit(void) { }
>>  #endif
>> 
>> +static int __init enable_zswap(char *s)
>> +{
>> +	zswap_enabled = true;
>> +	return 1;
>> +}
>> +__setup("zswap", enable_zswap);
>> +
>> +static int __init setup_zswap_compressor(char *s)
>> +{
>> +	strlcpy(zswap_compressor, s, sizeof(zswap_compressor));
>> +	zswap_enabled = true;
>> +	return 1;
>> +}
>> +__setup("zswapcompressor=", setup_zswap_compressor);
>> +
>> +static int __init setup_zswap_max_pool_percent(char *s)
>> +{
>> +	get_option(&s, &zswap_max_pool_percent);
>> +	zswap_enabled = true;
>> +	return 1;
>> +}
>> +__setup("zswap_maxpool_percent=", setup_zswap_max_pool_percent);
>> +
>>  /*********************************
>>  * module init and exit
>>  **********************************/
>> -- 
>> 1.7.10.4
>> 
>
>--
>To unsubscribe, send a message with 'unsubscribe linux-mm' in
>the body to majordomo@kvack.org.  For more info on Linux MM,
>see: http://www.linux-mm.org/ .
>Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] zswap: update/document boot parameters
  2013-06-20 14:48 ` Seth Jennings
  2013-06-20 23:20   ` Bob Liu
  2013-06-20 23:51   ` Wanpeng Li
@ 2013-06-20 23:51   ` Wanpeng Li
  2 siblings, 0 replies; 6+ messages in thread
From: Wanpeng Li @ 2013-06-20 23:51 UTC (permalink / raw)
  To: Seth Jennings; +Cc: akpm, linux-mm, konrad.wilk, Bob Liu

On Thu, Jun 20, 2013 at 09:48:26AM -0500, Seth Jennings wrote:
>On Thu, Jun 20, 2013 at 04:29:09PM +0800, Bob Liu wrote:
>> The current parameters of zswap are not straightforward.
>> Changed them to start with zswap* and documented them.
>
>Thanks for the patch!
>
>However, I think you might be missing that using module_param(_named) allows
>access on the kernel boot line with <modulename>.<moduleparam> syntax.  So
>"zswap" already has to be the parameter string as it is the name of the module
>to whom the parameters belong.
>
>For example, your patch just changes the boot parameter from
>zswap.max_pool_percent to zswap_maxpool_percent.  That doesn't add any clarity
>IMO.
>

Hi Seth,

>Yes, zswap isn't able to be a module right now.  But there is no harm in using
>the module framework to provide standardized access to zswap parameters. Plus,
>the day might come when zswap can be a module.
>

Do you plan to do zswap modulization? Otherwise I am happy to do that.
;-)

Regards,
Wanpeng Li 

>As far as documenting them in kernel-parameters.txt, this was mentioned before
>and I it was decided to not do that since module parameters are typically not
>documented there. However, since zswap is currently not buildable as a module,
>I could see where I case could be made for documenting them there.
>
>If you wanted to document them with their <modulename>.<moduleparam> syntax,
>I wouldn't be opposed to it.
>
>Seth
>
>> 
>> Signed-off-by: Bob Liu <bob.liu@oracle.com>
>> ---
>>  Documentation/kernel-parameters.txt |    8 ++++++++
>>  mm/zswap.c                          |   27 +++++++++++++++++++++++----
>>  2 files changed, 31 insertions(+), 4 deletions(-)
>> 
>> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
>> index 2fe6e76..07642fd 100644
>> --- a/Documentation/kernel-parameters.txt
>> +++ b/Documentation/kernel-parameters.txt
>> @@ -3367,6 +3367,14 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>>  			Format:
>>  			<irq>,<irq_mask>,<io>,<full_duplex>,<do_sound>,<lockup_hack>[,<irq2>[,<irq3>[,<irq4>]]]
>> 
>> +	zswap 		Enable compressed cache for swap pages support which
>> +			is disabled by default.
>> +	zswapcompressor=
>> +			Select which compressor to be used by zswap.
>> +			The default compressor is lzo.
>> +	zswap_maxpool_percent=
>> +			Select how may percent of total memory can be used to
>> +			store comprssed pages. The default percent is 20%.
>>  ______________________________________________________________________
>> 
>>  TODO:
>> diff --git a/mm/zswap.c b/mm/zswap.c
>> index 7fe2b1b..8ec1360 100644
>> --- a/mm/zswap.c
>> +++ b/mm/zswap.c
>> @@ -77,17 +77,13 @@ static u64 zswap_duplicate_entry;
>>  **********************************/
>>  /* Enable/disable zswap (disabled by default, fixed at boot for now) */
>>  static bool zswap_enabled __read_mostly;
>> -module_param_named(enabled, zswap_enabled, bool, 0);
>> 
>>  /* Compressor to be used by zswap (fixed at boot for now) */
>>  #define ZSWAP_COMPRESSOR_DEFAULT "lzo"
>>  static char *zswap_compressor = ZSWAP_COMPRESSOR_DEFAULT;
>> -module_param_named(compressor, zswap_compressor, charp, 0);
>> 
>>  /* The maximum percentage of memory that the compressed pool can occupy */
>>  static unsigned int zswap_max_pool_percent = 20;
>> -module_param_named(max_pool_percent,
>> -			zswap_max_pool_percent, uint, 0644);
>> 
>>  /*********************************
>>  * compression functions
>> @@ -914,6 +910,29 @@ static int __init zswap_debugfs_init(void)
>>  static void __exit zswap_debugfs_exit(void) { }
>>  #endif
>> 
>> +static int __init enable_zswap(char *s)
>> +{
>> +	zswap_enabled = true;
>> +	return 1;
>> +}
>> +__setup("zswap", enable_zswap);
>> +
>> +static int __init setup_zswap_compressor(char *s)
>> +{
>> +	strlcpy(zswap_compressor, s, sizeof(zswap_compressor));
>> +	zswap_enabled = true;
>> +	return 1;
>> +}
>> +__setup("zswapcompressor=", setup_zswap_compressor);
>> +
>> +static int __init setup_zswap_max_pool_percent(char *s)
>> +{
>> +	get_option(&s, &zswap_max_pool_percent);
>> +	zswap_enabled = true;
>> +	return 1;
>> +}
>> +__setup("zswap_maxpool_percent=", setup_zswap_max_pool_percent);
>> +
>>  /*********************************
>>  * module init and exit
>>  **********************************/
>> -- 
>> 1.7.10.4
>> 
>
>--
>To unsubscribe, send a message with 'unsubscribe linux-mm' in
>the body to majordomo@kvack.org.  For more info on Linux MM,
>see: http://www.linux-mm.org/ .
>Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] zswap: update/document boot parameters
  2013-06-20 23:51   ` Wanpeng Li
@ 2013-06-21 14:47     ` Seth Jennings
  0 siblings, 0 replies; 6+ messages in thread
From: Seth Jennings @ 2013-06-21 14:47 UTC (permalink / raw)
  To: Wanpeng Li; +Cc: akpm, linux-mm, konrad.wilk, Bob Liu

On Fri, Jun 21, 2013 at 07:51:09AM +0800, Wanpeng Li wrote:
> Do you plan to do zswap modulization? Otherwise I am happy to do that.
> ;-)

The question really isn't _can_ it be done.  It really is as simple as
exporting some swap symbols from the kernel.  The question is can it be done
_cleanly_.  I, for one, haven't seen a way to do it cleanly.

I guess there is also the question, "what is the benefit of having it as a
module?" and "could it restrict future functionality?" i.e. tighter integration
with the VMM.

Seth

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-06-21 14:48 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-06-20  8:29 [PATCH 2/2] zswap: update/document boot parameters Bob Liu
2013-06-20 14:48 ` Seth Jennings
2013-06-20 23:20   ` Bob Liu
2013-06-20 23:51   ` Wanpeng Li
2013-06-21 14:47     ` Seth Jennings
2013-06-20 23:51   ` Wanpeng Li

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).