public inbox for kexec@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH] makedumpfile: print spinner in progress information
@ 2013-10-25  1:45 HATAYAMA Daisuke
  2013-10-25  2:57 ` Jingbai Ma
  2013-10-25  4:07 ` Atsushi Kumagai
  0 siblings, 2 replies; 8+ messages in thread
From: HATAYAMA Daisuke @ 2013-10-25  1:45 UTC (permalink / raw)
  To: Atsushi Kumagai; +Cc: kexec@lists.infradead.org

On system with huge memory, percentage in progress information is
updated at very slow interval, because 1 percent on 1 TiB memory is
about 10 GiB, which looks like as if system has freezed. Then,
confused users might get tempted to push a reset button to recover the
system. We want to avoid such situation as much as possible.

To address the issue, this patch adds spinner that rotates in the
order of /, |, \ and - next to the progress indicator in percentage,
which helps users to get aware that system is still active and crash
dump process is still in progress now.

This code is borrowed from diskdump code.

The example is like this:

Copying data                       : [  0 %] /
Copying data                       : [  8 %] |
Copying data                       : [ 11 %] \
Copying data                       : [ 14 %] -
Copying data                       : [ 16 %] /
...
Copying data                       : [ 99 %] /
Copying data                       : [100 %] |

Signed-off-by: HATAYAMA Daisuke <d.hatayama@jp.fujitsu.com>
---
 print_info.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/print_info.c b/print_info.c
index 3527970..879fa4b 100644
--- a/print_info.c
+++ b/print_info.c
@@ -286,6 +286,8 @@ print_progress(const char *msg, unsigned long current, unsigned long end)
 	int progress;
 	time_t tm;
 	static time_t last_time = 0;
+	static unsigned int lapse = 0;
+	const char *spinner = "/|\\-";
 
 	if (current < end) {
 		tm = time(NULL);
@@ -297,13 +299,14 @@ print_progress(const char *msg, unsigned long current, unsigned long end)
 		progress = 100;
 
 	if (flag_ignore_r_char) {
-		PROGRESS_MSG("%-" PROGRESS_MAXLEN "s: [%3d %%]\n",
-			     msg, progress);
+		PROGRESS_MSG("%-" PROGRESS_MAXLEN "s: [%3d %%] %c\n",
+			     msg, progress, spinner[lapse & 3]);
 	} else {
 		PROGRESS_MSG("\r");
-		PROGRESS_MSG("%-" PROGRESS_MAXLEN "s: [%3d %%] ",
-			     msg, progress);
+		PROGRESS_MSG("%-" PROGRESS_MAXLEN "s: [%3d %%] %c",
+			     msg, progress, spinner[lapse & 3]);
 	}
+	lapse++;
 }
 
 void
-- 
1.8.3.1


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

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

* Re: [PATCH] makedumpfile: print spinner in progress information
  2013-10-25  1:45 [PATCH] makedumpfile: print spinner in progress information HATAYAMA Daisuke
@ 2013-10-25  2:57 ` Jingbai Ma
  2013-10-25  3:03   ` Jingbai Ma
  2013-10-25  4:07 ` Atsushi Kumagai
  1 sibling, 1 reply; 8+ messages in thread
From: Jingbai Ma @ 2013-10-25  2:57 UTC (permalink / raw)
  To: HATAYAMA Daisuke; +Cc: kexec@lists.infradead.org, Atsushi Kumagai, jingbai.ma

On 10/25/2013 09:45 AM, HATAYAMA Daisuke wrote:
> On system with huge memory, percentage in progress information is
> updated at very slow interval, because 1 percent on 1 TiB memory is
> about 10 GiB, which looks like as if system has freezed. Then,
> confused users might get tempted to push a reset button to recover the
> system. We want to avoid such situation as much as possible.
>
> To address the issue, this patch adds spinner that rotates in the
> order of /, |, \ and - next to the progress indicator in percentage,
> which helps users to get aware that system is still active and crash
> dump process is still in progress now.
>
> This code is borrowed from diskdump code.
>
> The example is like this:
>
> Copying data                       : [  0 %] /
> Copying data                       : [  8 %] |
> Copying data                       : [ 11 %] \
> Copying data                       : [ 14 %] -
> Copying data                       : [ 16 %] /
> ...
> Copying data                       : [ 99 %] /
> Copying data                       : [100 %] |
>
> Signed-off-by: HATAYAMA Daisuke<d.hatayama@jp.fujitsu.com>
> ---
>   print_info.c | 11 +++++++----
>   1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/print_info.c b/print_info.c
> index 3527970..879fa4b 100644
> --- a/print_info.c
> +++ b/print_info.c
> @@ -286,6 +286,8 @@ print_progress(const char *msg, unsigned long current, unsigned long end)
>   	int progress;
>   	time_t tm;
>   	static time_t last_time = 0;
> +	static unsigned int lapse = 0;
> +	const char *spinner = "/|\\-";

I believe your spinner should be:
const char *spinner = "/|\-/|\-";

>
>   	if (current<  end) {
>   		tm = time(NULL);
> @@ -297,13 +299,14 @@ print_progress(const char *msg, unsigned long current, unsigned long end)
>   		progress = 100;
>
>   	if (flag_ignore_r_char) {
> -		PROGRESS_MSG("%-" PROGRESS_MAXLEN "s: [%3d %%]\n",
> -			     msg, progress);
> +		PROGRESS_MSG("%-" PROGRESS_MAXLEN "s: [%3d %%] %c\n",
> +			     msg, progress, spinner[lapse&  3]);
>   	} else {
>   		PROGRESS_MSG("\r");
> -		PROGRESS_MSG("%-" PROGRESS_MAXLEN "s: [%3d %%] ",
> -			     msg, progress);
> +		PROGRESS_MSG("%-" PROGRESS_MAXLEN "s: [%3d %%] %c",
> +			     msg, progress, spinner[lapse&  3]);
>   	}
> +	lapse++;
>   }
>
>   void


-- 
Thanks,
Jingbai Ma

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

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

* Re: [PATCH] makedumpfile: print spinner in progress information
  2013-10-25  2:57 ` Jingbai Ma
@ 2013-10-25  3:03   ` Jingbai Ma
  2013-10-29  0:57     ` HATAYAMA Daisuke
  0 siblings, 1 reply; 8+ messages in thread
From: Jingbai Ma @ 2013-10-25  3:03 UTC (permalink / raw)
  To: Jingbai Ma; +Cc: kexec@lists.infradead.org, HATAYAMA Daisuke, Atsushi Kumagai

On 10/25/2013 10:57 AM, Jingbai Ma wrote:
> On 10/25/2013 09:45 AM, HATAYAMA Daisuke wrote:
>> On system with huge memory, percentage in progress information is
>> updated at very slow interval, because 1 percent on 1 TiB memory is
>> about 10 GiB, which looks like as if system has freezed. Then,
>> confused users might get tempted to push a reset button to recover the
>> system. We want to avoid such situation as much as possible.
>>
>> To address the issue, this patch adds spinner that rotates in the
>> order of /, |, \ and - next to the progress indicator in percentage,
>> which helps users to get aware that system is still active and crash
>> dump process is still in progress now.
>>
>> This code is borrowed from diskdump code.
>>
>> The example is like this:
>>
>> Copying data : [ 0 %] /
>> Copying data : [ 8 %] |
>> Copying data : [ 11 %] \
>> Copying data : [ 14 %] -
>> Copying data : [ 16 %] /
>> ...
>> Copying data : [ 99 %] /
>> Copying data : [100 %] |
>>
>> Signed-off-by: HATAYAMA Daisuke<d.hatayama@jp.fujitsu.com>
>> ---
>> print_info.c | 11 +++++++----
>> 1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/print_info.c b/print_info.c
>> index 3527970..879fa4b 100644
>> --- a/print_info.c
>> +++ b/print_info.c
>> @@ -286,6 +286,8 @@ print_progress(const char *msg, unsigned long
>> current, unsigned long end)
>> int progress;
>> time_t tm;
>> static time_t last_time = 0;
>> + static unsigned int lapse = 0;
>> + const char *spinner = "/|\\-";
>
> I believe your spinner should be:
> const char *spinner = "/|\-/|\-";
>

Oh, it seems not a full round, this one should be better:
const char *spinner = "/|\-";

>>
>> if (current< end) {
>> tm = time(NULL);
>> @@ -297,13 +299,14 @@ print_progress(const char *msg, unsigned long
>> current, unsigned long end)
>> progress = 100;
>>
>> if (flag_ignore_r_char) {
>> - PROGRESS_MSG("%-" PROGRESS_MAXLEN "s: [%3d %%]\n",
>> - msg, progress);
>> + PROGRESS_MSG("%-" PROGRESS_MAXLEN "s: [%3d %%] %c\n",
>> + msg, progress, spinner[lapse& 3]);

And then, this line should be:
msg, progress, spinner[lapse % 4]);

>> } else {
>> PROGRESS_MSG("\r");
>> - PROGRESS_MSG("%-" PROGRESS_MAXLEN "s: [%3d %%] ",
>> - msg, progress);
>> + PROGRESS_MSG("%-" PROGRESS_MAXLEN "s: [%3d %%] %c",
>> + msg, progress, spinner[lapse& 3]);

This line too:
msg, progress, spinner[lapse % 4]);

>> }
>> + lapse++;
>> }
>>
>> void
>
>


-- 
Thanks,
Jingbai Ma

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

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

* Re: [PATCH] makedumpfile: print spinner in progress information
  2013-10-25  1:45 [PATCH] makedumpfile: print spinner in progress information HATAYAMA Daisuke
  2013-10-25  2:57 ` Jingbai Ma
@ 2013-10-25  4:07 ` Atsushi Kumagai
  2013-10-29  1:26   ` HATAYAMA Daisuke
  1 sibling, 1 reply; 8+ messages in thread
From: Atsushi Kumagai @ 2013-10-25  4:07 UTC (permalink / raw)
  To: d.hatayama@jp.fujitsu.com; +Cc: kexec@lists.infradead.org

Hello HATAYAMA-san,

(2013/10/25 9:55), HATAYAMA Daisuke wrote:
> On system with huge memory, percentage in progress information is
> updated at very slow interval, because 1 percent on 1 TiB memory is
> about 10 GiB, which looks like as if system has freezed. Then,
> confused users might get tempted to push a reset button to recover the
> system. We want to avoid such situation as much as possible.
>
> To address the issue, this patch adds spinner that rotates in the
> order of /, |, \ and - next to the progress indicator in percentage,
> which helps users to get aware that system is still active and crash
> dump process is still in progress now.
>
> This code is borrowed from diskdump code.
>
> The example is like this:
>
> Copying data                       : [  0 %] /
> Copying data                       : [  8 %] |
> Copying data                       : [ 11 %] \
> Copying data                       : [ 14 %] -
> Copying data                       : [ 16 %] /
> ...
> Copying data                       : [ 99 %] /
> Copying data                       : [100 %] |

I like it, but have a comment.

    6109 int
    6110 write_kdump_pages_cyclic(struct cache_data *cd_header, struct cache_data *cd_page,
    6111                          struct page_desc *pd_zero, off_t *offset_data)
    6112 {
    ...
    6156         per = info->num_dumpable / 100;
    ...
    6178         for (pfn = start_pfn; pfn < end_pfn; pfn++) {
    6179
    6180                 if ((num_dumped % per) == 0)
    6181                         print_progress(PROGRESS_COPY, num_dumped, info->num_dumpable);

The interval of calling print_progress() looks still long if
num_dumpable is huge.
So how about fix this, e.g., by changing the interval to time based ?


Thanks
Atsushi Kumagai

> Signed-off-by: HATAYAMA Daisuke <d.hatayama@jp.fujitsu.com>
> ---
>   print_info.c | 11 +++++++----
>   1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/print_info.c b/print_info.c
> index 3527970..879fa4b 100644
> --- a/print_info.c
> +++ b/print_info.c
> @@ -286,6 +286,8 @@ print_progress(const char *msg, unsigned long current, unsigned long end)
>   	int progress;
>   	time_t tm;
>   	static time_t last_time = 0;
> +	static unsigned int lapse = 0;
> +	const char *spinner = "/|\\-";
>
>   	if (current < end) {
>   		tm = time(NULL);
> @@ -297,13 +299,14 @@ print_progress(const char *msg, unsigned long current, unsigned long end)
>   		progress = 100;
>
>   	if (flag_ignore_r_char) {
> -		PROGRESS_MSG("%-" PROGRESS_MAXLEN "s: [%3d %%]\n",
> -			     msg, progress);
> +		PROGRESS_MSG("%-" PROGRESS_MAXLEN "s: [%3d %%] %c\n",
> +			     msg, progress, spinner[lapse & 3]);
>   	} else {
>   		PROGRESS_MSG("\r");
> -		PROGRESS_MSG("%-" PROGRESS_MAXLEN "s: [%3d %%] ",
> -			     msg, progress);
> +		PROGRESS_MSG("%-" PROGRESS_MAXLEN "s: [%3d %%] %c",
> +			     msg, progress, spinner[lapse & 3]);
>   	}
> +	lapse++;
>   }
>
>   void
>


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

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

* Re: [PATCH] makedumpfile: print spinner in progress information
  2013-10-25  3:03   ` Jingbai Ma
@ 2013-10-29  0:57     ` HATAYAMA Daisuke
  0 siblings, 0 replies; 8+ messages in thread
From: HATAYAMA Daisuke @ 2013-10-29  0:57 UTC (permalink / raw)
  To: Jingbai Ma; +Cc: kexec@lists.infradead.org, Atsushi Kumagai

(2013/10/25 12:03), Jingbai Ma wrote:
> On 10/25/2013 10:57 AM, Jingbai Ma wrote:
>> On 10/25/2013 09:45 AM, HATAYAMA Daisuke wrote:
>>> On system with huge memory, percentage in progress information is
>>> updated at very slow interval, because 1 percent on 1 TiB memory is
>>> about 10 GiB, which looks like as if system has freezed. Then,
>>> confused users might get tempted to push a reset button to recover the
>>> system. We want to avoid such situation as much as possible.
>>>
>>> To address the issue, this patch adds spinner that rotates in the
>>> order of /, |, \ and - next to the progress indicator in percentage,
>>> which helps users to get aware that system is still active and crash
>>> dump process is still in progress now.
>>>
>>> This code is borrowed from diskdump code.
>>>
>>> The example is like this:
>>>
>>> Copying data : [ 0 %] /
>>> Copying data : [ 8 %] |
>>> Copying data : [ 11 %] \
>>> Copying data : [ 14 %] -
>>> Copying data : [ 16 %] /
>>> ...
>>> Copying data : [ 99 %] /
>>> Copying data : [100 %] |
>>>
>>> Signed-off-by: HATAYAMA Daisuke<d.hatayama@jp.fujitsu.com>
>>> ---
>>> print_info.c | 11 +++++++----
>>> 1 file changed, 7 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/print_info.c b/print_info.c
>>> index 3527970..879fa4b 100644
>>> --- a/print_info.c
>>> +++ b/print_info.c
>>> @@ -286,6 +286,8 @@ print_progress(const char *msg, unsigned long
>>> current, unsigned long end)
>>> int progress;
>>> time_t tm;
>>> static time_t last_time = 0;
>>> + static unsigned int lapse = 0;
>>> + const char *spinner = "/|\\-";
>>
>> I believe your spinner should be:
>> const char *spinner = "/|\-/|\-";
>>
>
> Oh, it seems not a full round, this one should be better:
> const char *spinner = "/|\-";
>

It's escape sequence.

>>>
>>> if (current< end) {
>>> tm = time(NULL);
>>> @@ -297,13 +299,14 @@ print_progress(const char *msg, unsigned long
>>> current, unsigned long end)
>>> progress = 100;
>>>
>>> if (flag_ignore_r_char) {
>>> - PROGRESS_MSG("%-" PROGRESS_MAXLEN "s: [%3d %%]\n",
>>> - msg, progress);
>>> + PROGRESS_MSG("%-" PROGRESS_MAXLEN "s: [%3d %%] %c\n",
>>> + msg, progress, spinner[lapse& 3]);
>
> And then, this line should be:
> msg, progress, spinner[lapse % 4]);
>

OK, though I think either is fine.

>>> } else {
>>> PROGRESS_MSG("\r");
>>> - PROGRESS_MSG("%-" PROGRESS_MAXLEN "s: [%3d %%] ",
>>> - msg, progress);
>>> + PROGRESS_MSG("%-" PROGRESS_MAXLEN "s: [%3d %%] %c",
>>> + msg, progress, spinner[lapse& 3]);
>
> This line too:
> msg, progress, spinner[lapse % 4]);
>

OK.

>>> }
>>> + lapse++;
>>> }
>>>
>>> void
>>
>>
>
>


-- 
Thanks.
HATAYAMA, Daisuke


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

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

* Re: [PATCH] makedumpfile: print spinner in progress information
  2013-10-25  4:07 ` Atsushi Kumagai
@ 2013-10-29  1:26   ` HATAYAMA Daisuke
  2013-10-29  2:42     ` HATAYAMA Daisuke
  0 siblings, 1 reply; 8+ messages in thread
From: HATAYAMA Daisuke @ 2013-10-29  1:26 UTC (permalink / raw)
  To: Atsushi Kumagai; +Cc: kexec@lists.infradead.org

(2013/10/25 13:07), Atsushi Kumagai wrote:
> Hello HATAYAMA-san,
> 
> (2013/10/25 9:55), HATAYAMA Daisuke wrote:
>> On system with huge memory, percentage in progress information is
>> updated at very slow interval, because 1 percent on 1 TiB memory is
>> about 10 GiB, which looks like as if system has freezed. Then,
>> confused users might get tempted to push a reset button to recover the
>> system. We want to avoid such situation as much as possible.
>>
>> To address the issue, this patch adds spinner that rotates in the
>> order of /, |, \ and - next to the progress indicator in percentage,
>> which helps users to get aware that system is still active and crash
>> dump process is still in progress now.
>>
>> This code is borrowed from diskdump code.
>>
>> The example is like this:
>>
>> Copying data                       : [  0 %] /
>> Copying data                       : [  8 %] |
>> Copying data                       : [ 11 %] \
>> Copying data                       : [ 14 %] -
>> Copying data                       : [ 16 %] /
>> ...
>> Copying data                       : [ 99 %] /
>> Copying data                       : [100 %] |
> 
> I like it, but have a comment.
> 
>      6109 int
>      6110 write_kdump_pages_cyclic(struct cache_data *cd_header, struct cache_data *cd_page,
>      6111                          struct page_desc *pd_zero, off_t *offset_data)
>      6112 {
>      ...
>      6156         per = info->num_dumpable / 100;
>      ...
>      6178         for (pfn = start_pfn; pfn < end_pfn; pfn++) {
>      6179
>      6180                 if ((num_dumped % per) == 0)
>      6181                         print_progress(PROGRESS_COPY, num_dumped, info->num_dumpable);
> 
> The interval of calling print_progress() looks still long if
> num_dumpable is huge.
> So how about fix this, e.g., by changing the interval to time based ?
> 

I wrote simple bench for time-based interval as below, which measures
total time consumed for calling time system call with/without vDSO.
It seems to me that both results are acceptable.
I'll reflect this change in next version.

$ ./bench
total: 21.059131
average: 0.000000
total: 65.558263
average: 0.000000

==bench.c
#include <stdio.h>
#include <stdlib.h>
#include <time.h>

static inline double getdtime(void)
{
        struct timeval tv;
        gettimeofday(&tv, NULL);
        return (double)tv.tv_sec + (double)tv.tv_usec * 1.0e-6;
}

int main(int argc, char **argv)
{
        unsigned long i;
        time_t t;
        double t1, t2, total;
        const int NR_time = 201;
        const unsigned long nr_repeat = (1UL << 40) / 4096;

        for (i = 0; i < nr_repeat; ++i) {
                t1 = getdtime();
                time(&t);
                t2 = getdtime();
                total += t2 - t1;
        }
        printf("total: %lf\n", total);
        printf("average: %lf\n", total / nr_repeat);

        for (i = 0; i < nr_repeat; ++i) {
                t1 = getdtime();
                syscall(NR_time, &t);
                t2 = getdtime();
                total += t2 - t1;
        }
        printf("total: %lf\n", total);
        printf("average: %lf\n", total / nr_repeat);

        return 0;
}
==

-- 
Thanks.
HATAYAMA, Daisuke


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

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

* Re: [PATCH] makedumpfile: print spinner in progress information
  2013-10-29  1:26   ` HATAYAMA Daisuke
@ 2013-10-29  2:42     ` HATAYAMA Daisuke
  2013-10-29  4:50       ` Atsushi Kumagai
  0 siblings, 1 reply; 8+ messages in thread
From: HATAYAMA Daisuke @ 2013-10-29  2:42 UTC (permalink / raw)
  To: Atsushi Kumagai; +Cc: kexec@lists.infradead.org

(2013/10/29 10:26), HATAYAMA Daisuke wrote:
> (2013/10/25 13:07), Atsushi Kumagai wrote:
>> Hello HATAYAMA-san,
>>
>> (2013/10/25 9:55), HATAYAMA Daisuke wrote:
>>> On system with huge memory, percentage in progress information is
>>> updated at very slow interval, because 1 percent on 1 TiB memory is
>>> about 10 GiB, which looks like as if system has freezed. Then,
>>> confused users might get tempted to push a reset button to recover the
>>> system. We want to avoid such situation as much as possible.
>>>
>>> To address the issue, this patch adds spinner that rotates in the
>>> order of /, |, \ and - next to the progress indicator in percentage,
>>> which helps users to get aware that system is still active and crash
>>> dump process is still in progress now.
>>>
>>> This code is borrowed from diskdump code.
>>>
>>> The example is like this:
>>>
>>> Copying data                       : [  0 %] /
>>> Copying data                       : [  8 %] |
>>> Copying data                       : [ 11 %] \
>>> Copying data                       : [ 14 %] -
>>> Copying data                       : [ 16 %] /
>>> ...
>>> Copying data                       : [ 99 %] /
>>> Copying data                       : [100 %] |
>>
>> I like it, but have a comment.
>>
>>       6109 int
>>       6110 write_kdump_pages_cyclic(struct cache_data *cd_header, struct cache_data *cd_page,
>>       6111                          struct page_desc *pd_zero, off_t *offset_data)
>>       6112 {
>>       ...
>>       6156         per = info->num_dumpable / 100;
>>       ...
>>       6178         for (pfn = start_pfn; pfn < end_pfn; pfn++) {
>>       6179
>>       6180                 if ((num_dumped % per) == 0)
>>       6181                         print_progress(PROGRESS_COPY, num_dumped, info->num_dumpable);
>>
>> The interval of calling print_progress() looks still long if
>> num_dumpable is huge.
>> So how about fix this, e.g., by changing the interval to time based ?
>>
>
> I wrote simple bench for time-based interval as below, which measures
> total time consumed for calling time system call with/without vDSO.
> It seems to me that both results are acceptable.
> I'll reflect this change in next version.
>
> $ ./bench
> total: 21.059131
> average: 0.000000
> total: 65.558263
> average: 0.000000
>

This conclusion was wrong. Sorry. For example on our FJ 12 TiB system we collected about 300 GiB
crash dump in about 40 minutes. If removing "if ((num_dumped % per) == 0)" and calling time()
in each loop in print_progress(), total time for invoking time() system call is about 65 * 12
= 780 sec = 13 min. This is about 20 % of a whole crash dump time. Obviously problematic.

Instead, I think it better to increase the number of calling print_progress() like:

   per = info->num_dumpable / 10000

-- 
Thanks.
HATAYAMA, Daisuke


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

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

* Re: [PATCH] makedumpfile: print spinner in progress information
  2013-10-29  2:42     ` HATAYAMA Daisuke
@ 2013-10-29  4:50       ` Atsushi Kumagai
  0 siblings, 0 replies; 8+ messages in thread
From: Atsushi Kumagai @ 2013-10-29  4:50 UTC (permalink / raw)
  To: d.hatayama@jp.fujitsu.com; +Cc: kexec@lists.infradead.org

(2013/10/29 11:43), HATAYAMA Daisuke wrote:
> (2013/10/29 10:26), HATAYAMA Daisuke wrote:
>> (2013/10/25 13:07), Atsushi Kumagai wrote:
>>> Hello HATAYAMA-san,
>>>
>>> (2013/10/25 9:55), HATAYAMA Daisuke wrote:
>>>> On system with huge memory, percentage in progress information is
>>>> updated at very slow interval, because 1 percent on 1 TiB memory is
>>>> about 10 GiB, which looks like as if system has freezed. Then,
>>>> confused users might get tempted to push a reset button to recover the
>>>> system. We want to avoid such situation as much as possible.
>>>>
>>>> To address the issue, this patch adds spinner that rotates in the
>>>> order of /, |, \ and - next to the progress indicator in percentage,
>>>> which helps users to get aware that system is still active and crash
>>>> dump process is still in progress now.
>>>>
>>>> This code is borrowed from diskdump code.
>>>>
>>>> The example is like this:
>>>>
>>>> Copying data                       : [  0 %] /
>>>> Copying data                       : [  8 %] |
>>>> Copying data                       : [ 11 %] \
>>>> Copying data                       : [ 14 %] -
>>>> Copying data                       : [ 16 %] /
>>>> ...
>>>> Copying data                       : [ 99 %] /
>>>> Copying data                       : [100 %] |
>>>
>>> I like it, but have a comment.
>>>
>>>       6109 int
>>>       6110 write_kdump_pages_cyclic(struct cache_data *cd_header, struct cache_data *cd_page,
>>>       6111                          struct page_desc *pd_zero, off_t *offset_data)
>>>       6112 {
>>>       ...
>>>       6156         per = info->num_dumpable / 100;
>>>       ...
>>>       6178         for (pfn = start_pfn; pfn < end_pfn; pfn++) {
>>>       6179
>>>       6180                 if ((num_dumped % per) == 0)
>>>       6181                         print_progress(PROGRESS_COPY, num_dumped, info->num_dumpable);
>>>
>>> The interval of calling print_progress() looks still long if
>>> num_dumpable is huge.
>>> So how about fix this, e.g., by changing the interval to time based ?
>>>
>>
>> I wrote simple bench for time-based interval as below, which measures
>> total time consumed for calling time system call with/without vDSO.
>> It seems to me that both results are acceptable.
>> I'll reflect this change in next version.
>>
>> $ ./bench
>> total: 21.059131
>> average: 0.000000
>> total: 65.558263
>> average: 0.000000
>>
>
> This conclusion was wrong. Sorry. For example on our FJ 12 TiB system we collected about 300 GiB
> crash dump in about 40 minutes. If removing "if ((num_dumped % per) == 0)" and calling time()
> in each loop in print_progress(), total time for invoking time() system call is about 65 * 12
> = 780 sec = 13 min. This is about 20 % of a whole crash dump time. Obviously problematic.
>
> Instead, I think it better to increase the number of calling print_progress() like:
>
>    per = info->num_dumpable / 10000

I agree with you, let's go with this idea.


Thanks
Atsushi Kumagai

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

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

end of thread, other threads:[~2013-10-29  4:54 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-25  1:45 [PATCH] makedumpfile: print spinner in progress information HATAYAMA Daisuke
2013-10-25  2:57 ` Jingbai Ma
2013-10-25  3:03   ` Jingbai Ma
2013-10-29  0:57     ` HATAYAMA Daisuke
2013-10-25  4:07 ` Atsushi Kumagai
2013-10-29  1:26   ` HATAYAMA Daisuke
2013-10-29  2:42     ` HATAYAMA Daisuke
2013-10-29  4:50       ` Atsushi Kumagai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox