All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 3/3] Stop maximizing the bitmap buffer to reduce the risk of OOM.
@ 2014-06-10  4:54 Atsushi Kumagai
  2014-06-16  8:29 ` Baoquan He
  0 siblings, 1 reply; 3+ messages in thread
From: Atsushi Kumagai @ 2014-06-10  4:54 UTC (permalink / raw)
  To: kexec

We tried to maximize the bitmap buffer to get the best performance,
but the performance degradation caused by multi-cycle processing
looks very small according to the benchmark on 2TB memory:

  https://lkml.org/lkml/2013/3/26/914

This result means we don't need to make an effort to maximize the
bitmap buffer, it will just increase the risk of OOM.

This patch sets a small fixed value (4MB) as a safety limit,
it may be safer and enough in most cases.

Signed-off-by: Atsushi Kumagai <kumagai-atsushi@mxc.nes.nec.co.jp>
---
 makedumpfile.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/makedumpfile.c b/makedumpfile.c
index b8f1ec4..c23667b 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -8946,13 +8946,15 @@ out:
 
 
 /*
- * Choose the lesser value of the two below as the size of cyclic buffer.
- *  - the size enough for storing the 1st/2nd bitmap for the whole of vmcore
- *  - 80% of free memory (as safety limit)
+ * Choose the lesser value of the three below as the size of cyclic buffer.
+ *  - the size enough for storing the 1st or 2nd bitmap for the whole of vmcore
+ *  - 4MB as sufficient value
+ *  - 60% of free memory as safety limit
  */
 int
 calculate_cyclic_buffer_size(void) {
 	unsigned long long limit_size, bitmap_size;
+	const unsigned long long maximum_size = 4 * 1024 * 1024;
 
 	if (info->max_mapnr <= 0) {
 		ERRMSG("Invalid max_mapnr(%llu).\n", info->max_mapnr);
@@ -8960,17 +8962,18 @@ calculate_cyclic_buffer_size(void) {
 	}
 
 	/*
-	 *  We should keep the size of cyclic buffer within 80% of free memory
-	 *  for safety.
+	 *  At least, we should keep the size of cyclic buffer within 60% of
+	 *  free memory for safety.
 	 */
-	limit_size = get_free_memory_size() * 0.8;
+	limit_size = get_free_memory_size() * 0.6;
 	bitmap_size = info->max_mapnr / BITPERBYTE;
 
 	/* if --split was specified cyclic buffer allocated per dump file */
 	if (info->num_dumpfile > 1)
 		bitmap_size /= info->num_dumpfile;
 
-	info->bufsize_cyclic = MIN(limit_size, bitmap_size);
+	/* 4MB will be enought for performance according to benchmarks. */
+	info->bufsize_cyclic = MIN(MIN(limit_size, maximum_size), bitmap_size);
 
 	return TRUE;
 }
-- 
1.9.0

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* Re: [PATCH 3/3] Stop maximizing the bitmap buffer to reduce the risk of OOM.
  2014-06-10  4:54 [PATCH 3/3] Stop maximizing the bitmap buffer to reduce the risk of OOM Atsushi Kumagai
@ 2014-06-16  8:29 ` Baoquan He
  2014-06-17  2:32   ` Atsushi Kumagai
  0 siblings, 1 reply; 3+ messages in thread
From: Baoquan He @ 2014-06-16  8:29 UTC (permalink / raw)
  To: Atsushi Kumagai; +Cc: kexec

On 06/10/14 at 04:54am, Atsushi Kumagai wrote:
> We tried to maximize the bitmap buffer to get the best performance,
> but the performance degradation caused by multi-cycle processing
> looks very small according to the benchmark on 2TB memory:
> 
>   https://lkml.org/lkml/2013/3/26/914
> 
> This result means we don't need to make an effort to maximize the
> bitmap buffer, it will just increase the risk of OOM.
> 
> This patch sets a small fixed value (4MB) as a safety limit,
> it may be safer and enough in most cases.
> 
> Signed-off-by: Atsushi Kumagai <kumagai-atsushi@mxc.nes.nec.co.jp>
> ---
>  makedumpfile.c | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/makedumpfile.c b/makedumpfile.c
> index b8f1ec4..c23667b 100644
> --- a/makedumpfile.c
> +++ b/makedumpfile.c
> @@ -8946,13 +8946,15 @@ out:
>  
>  
>  /*
> - * Choose the lesser value of the two below as the size of cyclic buffer.
> - *  - the size enough for storing the 1st/2nd bitmap for the whole of vmcore
> - *  - 80% of free memory (as safety limit)
> + * Choose the lesser value of the three below as the size of cyclic buffer.
                 ~~~~~ typo, it should be less.
> + *  - the size enough for storing the 1st or 2nd bitmap for the whole of vmcore
> + *  - 4MB as sufficient value
> + *  - 60% of free memory as safety limit
>   */
>  int
>  calculate_cyclic_buffer_size(void) {
>  	unsigned long long limit_size, bitmap_size;
> +	const unsigned long long maximum_size = 4 * 1024 * 1024;
>  
>  	if (info->max_mapnr <= 0) {
>  		ERRMSG("Invalid max_mapnr(%llu).\n", info->max_mapnr);
> @@ -8960,17 +8962,18 @@ calculate_cyclic_buffer_size(void) {
>  	}
>  
>  	/*
> -	 *  We should keep the size of cyclic buffer within 80% of free memory
> -	 *  for safety.
> +	 *  At least, we should keep the size of cyclic buffer within 60% of
> +	 *  free memory for safety.
>  	 */
> -	limit_size = get_free_memory_size() * 0.8;
> +	limit_size = get_free_memory_size() * 0.6;
>  	bitmap_size = info->max_mapnr / BITPERBYTE;
>  
>  	/* if --split was specified cyclic buffer allocated per dump file */
>  	if (info->num_dumpfile > 1)
>  		bitmap_size /= info->num_dumpfile;
>  
> -	info->bufsize_cyclic = MIN(limit_size, bitmap_size);
> +	/* 4MB will be enought for performance according to benchmarks. */
                      ~~~~~~~~   
There's a typo, here should be enough.

> +	info->bufsize_cyclic = MIN(MIN(limit_size, maximum_size), bitmap_size);
>  
>  	return TRUE;
>  }
> -- 
> 1.9.0
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* RE: [PATCH 3/3] Stop maximizing the bitmap buffer to reduce the risk of OOM.
  2014-06-16  8:29 ` Baoquan He
@ 2014-06-17  2:32   ` Atsushi Kumagai
  0 siblings, 0 replies; 3+ messages in thread
From: Atsushi Kumagai @ 2014-06-17  2:32 UTC (permalink / raw)
  To: bhe; +Cc: kexec

Hello Baoquan,

Thanks for your careful review.

>On 06/10/14 at 04:54am, Atsushi Kumagai wrote:
>> We tried to maximize the bitmap buffer to get the best performance,
>> but the performance degradation caused by multi-cycle processing
>> looks very small according to the benchmark on 2TB memory:
>>
>>   https://lkml.org/lkml/2013/3/26/914
>>
>> This result means we don't need to make an effort to maximize the
>> bitmap buffer, it will just increase the risk of OOM.
>>
>> This patch sets a small fixed value (4MB) as a safety limit,
>> it may be safer and enough in most cases.
>>
>> Signed-off-by: Atsushi Kumagai <kumagai-atsushi@mxc.nes.nec.co.jp>
>> ---
>>  makedumpfile.c | 17 ++++++++++-------
>>  1 file changed, 10 insertions(+), 7 deletions(-)
>>
>> diff --git a/makedumpfile.c b/makedumpfile.c
>> index b8f1ec4..c23667b 100644
>> --- a/makedumpfile.c
>> +++ b/makedumpfile.c
>> @@ -8946,13 +8946,15 @@ out:
>>
>>
>>  /*
>> - * Choose the lesser value of the two below as the size of cyclic buffer.
>> - *  - the size enough for storing the 1st/2nd bitmap for the whole of vmcore
>> - *  - 80% of free memory (as safety limit)
>> + * Choose the lesser value of the three below as the size of cyclic buffer.
>                 ~~~~~ typo, it should be less.

I'll fix it.

>> + *  - the size enough for storing the 1st or 2nd bitmap for the whole of vmcore
>> + *  - 4MB as sufficient value
>> + *  - 60% of free memory as safety limit
>>   */
>>  int
>>  calculate_cyclic_buffer_size(void) {
>>  	unsigned long long limit_size, bitmap_size;
>> +	const unsigned long long maximum_size = 4 * 1024 * 1024;
>>
>>  	if (info->max_mapnr <= 0) {
>>  		ERRMSG("Invalid max_mapnr(%llu).\n", info->max_mapnr);
>> @@ -8960,17 +8962,18 @@ calculate_cyclic_buffer_size(void) {
>>  	}
>>
>>  	/*
>> -	 *  We should keep the size of cyclic buffer within 80% of free memory
>> -	 *  for safety.
>> +	 *  At least, we should keep the size of cyclic buffer within 60% of
>> +	 *  free memory for safety.
>>  	 */
>> -	limit_size = get_free_memory_size() * 0.8;
>> +	limit_size = get_free_memory_size() * 0.6;
>>  	bitmap_size = info->max_mapnr / BITPERBYTE;
>>
>>  	/* if --split was specified cyclic buffer allocated per dump file */
>>  	if (info->num_dumpfile > 1)
>>  		bitmap_size /= info->num_dumpfile;
>>
>> -	info->bufsize_cyclic = MIN(limit_size, bitmap_size);
>> +	/* 4MB will be enought for performance according to benchmarks. */
>                      ~~~~~~~~
>There's a typo, here should be enough.

I'll fix it.


Thanks
Atsushi Kumagai

>> +	info->bufsize_cyclic = MIN(MIN(limit_size, maximum_size), bitmap_size);
>>
>>  	return TRUE;
>>  }
>> --
>> 1.9.0
>>
>> _______________________________________________
>> kexec mailing list
>> kexec@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/kexec

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

end of thread, other threads:[~2014-06-17  2:35 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-10  4:54 [PATCH 3/3] Stop maximizing the bitmap buffer to reduce the risk of OOM Atsushi Kumagai
2014-06-16  8:29 ` Baoquan He
2014-06-17  2:32   ` Atsushi Kumagai

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.