* 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-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 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
2013-06-21 14:47 ` Seth Jennings
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 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