* [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