Hi, Zhou > -----Original Message----- > From: "Zhou, Wenjian/???" [mailto:zhouwj-fnst at cn.fujitsu.com] > Sent: Tuesday, February 23, 2016 5:30 PM > To: Usui Minoru(?? ?) <min-usui at ti.jp.nec.com>; kexec at lists.infradead.org > Subject: Re: [PATCH v2] Improve the performance of --num-threads -d 31 > > Hi > > On 02/23/2016 04:00 PM, Minoru Usui wrote: > > Hi, > > > >> -----Original Message----- > >> From: "Zhou, Wenjian/???" [mailto:zhouwj-fnst at cn.fujitsu.com] > >> Sent: Tuesday, February 23, 2016 12:45 PM > >> To: Usui Minoru(?? ?) <min-usui at ti.jp.nec.com>; kexec at lists.infradead.org > >> Subject: Re: [PATCH v2] Improve the performance of --num-threads -d 31 > >> > >> Hello Usui, > >> > >> On 02/23/2016 09:32 AM, Minoru Usui wrote: > >>> Hello, Zhou > >>> > >>> Thank you for reflecting my comments. > >>> I have other comments. > >>> > >>> See below. > >>> > >>>> -----Original Message----- > >>>> From: kexec [mailto:kexec-bounces at lists.infradead.org] On Behalf Of Zhou Wenjian > >>>> Sent: Wednesday, February 17, 2016 4:06 PM > >>>> To: kexec at lists.infradead.org > >>>> Subject: [PATCH v2] Improve the performance of --num-threads -d 31 > >>>> > >>>> v2: > >>>> 1. address Minoru Usui's comments about magic number and error message > >>>> 2. fix a bug in cyclic mode caused by optimizing > >>>> > >>>> v1: > >>>> 1. change page_flag.ready's value to enum > >>>> 2. change the patch description > >>>> 3. cleanup some codes > >>>> 4. fix a bug in cyclic mode > >>>> > >>>> multi-threads implementation will introduce extra cost when handling > >>>> each page. The origin implementation will also do the extra work for > >>>> filtered pages. So there is a big performance degradation in > >>>> --num-threads -d 31. > >>>> The new implementation won't do the extra work for filtered pages any > >>>> more. So the performance of -d 31 is close to that of serial processing. > >>>> > >>>> The new implementation is just like the following: > >>>> * The basic idea is producer producing page and consumer writing page. > >>>> * Each producer have a page_flag_buf list which is used for storing > >>>> page's description. > >>>> * The size of page_flag_buf is little so it won't take too much memory. > >>>> * And all producers will share a page_data_buf array which is > >>>> used for storing page's compressed data. > >>>> * The main thread is the consumer. It will find the next pfn and write > >>>> it into file. > >>>> * The next pfn is smallest pfn in all page_flag_buf. > >>>> > >>>> Signed-off-by: Zhou Wenjian <zhouwj-fnst at cn.fujitsu.com> > >>>> --- > >>>> makedumpfile.c | 266 ++++++++++++++++++++++++++++++++++++++------------------- > >>>> makedumpfile.h | 31 ++++--- > >>>> 2 files changed, 200 insertions(+), 97 deletions(-) > >>>> > >>>> diff --git a/makedumpfile.c b/makedumpfile.c > >>>> index fa0b779..31329b5 100644 > >>>> --- a/makedumpfile.c > >>>> +++ b/makedumpfile.c > >>>> @@ -3483,7 +3483,8 @@ initial_for_parallel() > >>>> unsigned long page_data_buf_size; > >>>> unsigned long limit_size; > >>>> int page_data_num; > >>>> - int i; > >>>> + struct page_flag *current; > >>>> + int i, j; > >>>> > >>>> len_buf_out = calculate_len_buf_out(info->page_size); > >>>> > >>>> @@ -3562,8 +3563,10 @@ initial_for_parallel() > >>>> - MAP_REGION * info->num_threads) * 0.6; > >>>> > >>>> page_data_num = limit_size / page_data_buf_size; > >>>> + info->num_buffers = 3 * info->num_threads; > >>>> > >>>> - info->num_buffers = MIN(NUM_BUFFERS, page_data_num); > >>>> + info->num_buffers = MAX(info->num_buffers, NUM_BUFFERS); > >>>> + info->num_buffers = MIN(info->num_buffers, page_data_num); > >>>> > >>>> DEBUG_MSG("Number of struct page_data for produce/consume: %d\n", > >>>> info->num_buffers); > >>>> @@ -3588,6 +3591,36 @@ initial_for_parallel() > >>>> } > >>>> > >>>> /* > >>>> + * initial page_flag for each thread > >>>> + */ > >>>> + if ((info->page_flag_buf = malloc(sizeof(void *) * info->num_threads)) > >>>> + == NULL) { > >>>> + MSG("Can't allocate memory for page_flag_buf. %s\n", > >>>> + strerror(errno)); > >>>> + return FALSE; > >>>> + } > >>>> + memset(info->page_flag_buf, 0, sizeof(void *) * info->num_threads); > >>>> + > >>>> + for (i = 0; i < info->num_threads; i++) { > >>>> + if ((info->page_flag_buf[i] = calloc(1, sizeof(struct page_flag))) == NULL) { > >>>> + MSG("Can't allocate memory for page_flag. %s\n", > >>>> + strerror(errno)); > >>>> + return FALSE; > >>>> + } > >>>> + current = info->page_flag_buf[i]; > >>>> + > >>>> + for (j = 1; j < NUM_BUFFERS; j++) { > >>>> + if ((current->next = calloc(1, sizeof(struct page_flag))) == NULL) { > >>>> + MSG("Can't allocate memory for page_flag. %s\n", > >>>> + strerror(errno)); > >>>> + return FALSE; > >>>> + } > >>>> + current = current->next; > >>>> + } > >>>> + current->next = info->page_flag_buf[i]; > >>>> + } > >>>> + > >>>> + /* > >>>> * initial fd_memory for threads > >>>> */ > >>>> for (i = 0; i < info->num_threads; i++) { > >>>> @@ -3612,7 +3645,8 @@ initial_for_parallel() > >>>> void > >>>> free_for_parallel() > >>>> { > >>>> - int i; > >>>> + int i, j; > >>>> + struct page_flag *current; > >>>> > >>>> if (info->threads != NULL) { > >>>> for (i = 0; i < info->num_threads; i++) { > >>>> @@ -3655,6 +3689,19 @@ free_for_parallel() > >>>> free(info->page_data_buf); > >>>> } > >>>> > >>>> + if (info->page_flag_buf != NULL) { > >>>> + for (i = 0; i < info->num_threads; i++) { > >>>> + for (j = 0; j < NUM_BUFFERS; j++) { > >>>> + if (info->page_flag_buf[i] != NULL) { > >>>> + current = info->page_flag_buf[i]; > >>>> + info->page_flag_buf[i] = current->next; > >>>> + free(current); > >>>> + } > >>>> + } > >>>> + } > >>>> + free(info->page_flag_buf); > >>>> + } > >>>> + > >>>> if (info->parallel_info == NULL) > >>>> return; > >>>> > >>>> @@ -7076,10 +7123,10 @@ kdump_thread_function_cyclic(void *arg) { > >>>> void *retval = PTHREAD_FAIL; > >>>> struct thread_args *kdump_thread_args = (struct thread_args *)arg; > >>>> struct page_data *page_data_buf = kdump_thread_args->page_data_buf; > >>>> + struct page_flag *page_flag_buf = kdump_thread_args->page_flag_buf; > >>>> struct cycle *cycle = kdump_thread_args->cycle; > >>>> - int page_data_num = kdump_thread_args->page_data_num; > >>>> - mdf_pfn_t pfn; > >>>> - int index; > >>>> + mdf_pfn_t pfn = cycle->start_pfn; > >>>> + int index = kdump_thread_args->thread_num; > >>>> int buf_ready; > >>>> int dumpable; > >>>> int fd_memory = 0; > >>>> @@ -7125,47 +7172,46 @@ kdump_thread_function_cyclic(void *arg) { > >>>> kdump_thread_args->thread_num); > >>>> } > >>>> > >>>> - while (1) { > >>>> - /* get next pfn */ > >>>> - pthread_mutex_lock(&info->current_pfn_mutex); > >>>> - pfn = info->current_pfn; > >>>> - info->current_pfn++; > >>>> - pthread_mutex_unlock(&info->current_pfn_mutex); > >>>> + /* > >>>> + * filtered page won't take anything > >>>> + * unfiltered zero page will only take a page_flag_buf > >>>> + * unfiltered non-zero page will take a page_flag_buf and a page_data_buf > >>>> + */ > >>>> + while (pfn < kdump_thread_args->end_pfn) { > >>>> + buf_ready = FALSE; > >>>> > >>>> - if (pfn >= kdump_thread_args->end_pfn) > >>>> - break; > >>>> + while (page_data_buf[index].used != FALSE || > >>>> + pthread_mutex_trylock(&page_data_buf[index].mutex) != 0) > >>>> + index = (index + 1) % info->num_buffers; > >>>> - index = -1; > >>>> - buf_ready = FALSE; > >>>> + page_data_buf[index].used = TRUE; > >>>> > >>>> while (buf_ready == FALSE) { > >>>> pthread_testcancel(); > >>>> - > >>>> - index = pfn % page_data_num; > >>>> - > >>>> - if (pfn - info->consumed_pfn > info->num_buffers) > >>>> - continue; > >>>> - > >>>> - if (page_data_buf[index].ready != 0) > >>>> + if (page_flag_buf->ready == FLAG_READY) > >>>> continue; > >>>> > >>>> - pthread_mutex_lock(&page_data_buf[index].mutex); > >>>> - > >>>> - if (page_data_buf[index].ready != 0) > >>>> - goto unlock; > >>>> + /* get next pfn */ > >>>> + pthread_mutex_lock(&info->current_pfn_mutex); > >>>> + pfn = info->current_pfn; > >>>> + info->current_pfn++; > >>>> + page_flag_buf->ready = FLAG_FILLING; > >>>> + pthread_mutex_unlock(&info->current_pfn_mutex); > >>>> > >>>> - buf_ready = TRUE; > >>>> + page_flag_buf->pfn = pfn; > >>>> > >>>> - page_data_buf[index].pfn = pfn; > >>>> - page_data_buf[index].ready = 1; > >>>> + if (pfn >= kdump_thread_args->end_pfn) { > >>>> + page_data_buf[index].used = FALSE; > >>>> + page_flag_buf->ready = FLAG_READY; > >>>> + break; > >>>> + } > >>>> > >>>> dumpable = is_dumpable( > >>>> info->fd_bitmap ? &bitmap_parallel : info->bitmap2, > >>>> pfn, > >>>> cycle); > >>>> - page_data_buf[index].dumpable = dumpable; > >>>> if (!dumpable) > >>>> - goto unlock; > >>>> + continue; > >>>> > >>>> if (!read_pfn_parallel(fd_memory, pfn, buf, > >>>> &bitmap_memory_parallel, > >>>> @@ -7178,11 +7224,11 @@ kdump_thread_function_cyclic(void *arg) { > >>>> > >>>> if ((info->dump_level & DL_EXCLUDE_ZERO) > >>>> && is_zero_page(buf, info->page_size)) { > >>>> - page_data_buf[index].zero = TRUE; > >>>> - goto unlock; > >>>> + page_flag_buf->zero = TRUE; > >>>> + goto next; > >>>> } > >>>> > >>>> - page_data_buf[index].zero = FALSE; > >>>> + page_flag_buf->zero = FALSE; > >>>> > >>>> /* > >>>> * Compress the page data. > >>>> @@ -7232,12 +7278,16 @@ kdump_thread_function_cyclic(void *arg) { > >>>> page_data_buf[index].size = info->page_size; > >>>> memcpy(page_data_buf[index].buf, buf, info->page_size); > >>>> } > >>>> -unlock: > >>>> - pthread_mutex_unlock(&page_data_buf[index].mutex); > >>>> + page_flag_buf->index = index; > >>>> + buf_ready = TRUE; > >>>> +next: > >>>> + page_flag_buf->ready = FLAG_READY; > >>>> + page_flag_buf = page_flag_buf->next; > >>>> > >>>> } > >>>> - } > >>>> > >>>> + pthread_mutex_unlock(&page_data_buf[index].mutex); > >>>> + } > >>>> retval = NULL; > >>>> > >>>> fail: > >>>> @@ -7265,14 +7315,15 @@ write_kdump_pages_parallel_cyclic(struct cache_data *cd_header, > >>>> struct page_desc pd; > >>>> struct timeval tv_start; > >>>> struct timeval last, new; > >>>> - unsigned long long consuming_pfn; > >>>> pthread_t **threads = NULL; > >>>> struct thread_args *kdump_thread_args = NULL; > >>>> void *thread_result; > >>>> - int page_data_num; > >>>> + int page_buf_num; > >>>> struct page_data *page_data_buf = NULL; > >>>> int i; > >>>> int index; > >>>> + int end_count, consuming, check_count; > >>>> + mdf_pfn_t current_pfn, temp_pfn; > >>>> > >>>> if (info->flag_elf_dumpfile) > >>>> return FALSE; > >>>> @@ -7319,16 +7370,11 @@ write_kdump_pages_parallel_cyclic(struct cache_data *cd_header, > >>>> threads = info->threads; > >>>> kdump_thread_args = info->kdump_thread_args; > >>>> > >>>> - page_data_num = info->num_buffers; > >>>> + page_buf_num = info->num_buffers; > >>>> page_data_buf = info->page_data_buf; > >>>> > >>>> - for (i = 0; i < page_data_num; i++) { > >>>> - /* > >>>> - * producer will use pfn in page_data_buf to decide the > >>>> - * consumed pfn > >>>> - */ > >>>> - page_data_buf[i].pfn = start_pfn - 1; > >>>> - page_data_buf[i].ready = 0; > >>>> + for (i = 0; i < page_buf_num; i++) { > >>>> + page_data_buf[i].used = FALSE; > >>>> res = pthread_mutex_init(&page_data_buf[i].mutex, NULL); > >>>> if (res != 0) { > >>>> ERRMSG("Can't initialize mutex of page_data_buf. %s\n", > >>>> @@ -7342,8 +7388,9 @@ write_kdump_pages_parallel_cyclic(struct cache_data *cd_header, > >>>> kdump_thread_args[i].len_buf_out = len_buf_out; > >>>> kdump_thread_args[i].start_pfn = start_pfn; > >>>> kdump_thread_args[i].end_pfn = end_pfn; > >>>> - kdump_thread_args[i].page_data_num = page_data_num; > >>>> + kdump_thread_args[i].page_buf_num = page_buf_num; > >>>> kdump_thread_args[i].page_data_buf = page_data_buf; > >>>> + kdump_thread_args[i].page_flag_buf = info->page_flag_buf[i]; > >>>> kdump_thread_args[i].cycle = cycle; > >>>> > >>>> res = pthread_create(threads[i], NULL, > >>>> @@ -7356,55 +7403,101 @@ write_kdump_pages_parallel_cyclic(struct cache_data *cd_header, > >>>> } > >>>> } > >>>> > >>>> - consuming_pfn = start_pfn; > >>>> - index = -1; > >>>> + end_count = 0; > >>>> + while (1) { > >>>> + consuming = 0; > >>>> + check_count = 0; > >>>> > >>>> - gettimeofday(&last, NULL); > >>>> + /* > >>>> + * The basic idea is producer producing page and consumer writing page. > >>>> + * Each producer have a page_flag_buf list which is used for storing page's description. > >>>> + * The size of page_flag_buf is little so it won't take too much memory. > >>>> + * And all producers will share a page_data_buf array which is used for storing page's compressed data. > >>>> + * The main thread is the consumer. It will find the next pfn and write it into file. > >>>> + * The next pfn is smallest pfn in all page_flag_buf. > >>>> + */ > >>>> + gettimeofday(&last, NULL); > >>>> + while (1) { > >>>> + current_pfn = end_pfn; > >>>> > >>>> - while (consuming_pfn < end_pfn) { > >>>> - index = consuming_pfn % page_data_num; > >>>> + /* > >>>> + * page_flag_buf is in circular linked list. > >>>> + * The array info->page_flag_buf[] records the current page_flag_buf in each thread's > >>>> + * page_flag_buf list. > >>>> + * consuming is used for recording in which thread the pfn is the smallest. > >>>> + * current_pfn is used for recording the value of pfn when checking the pfn. > >>>> + */ > >>>> + for (i = 0; i < info->num_threads; i++) { > >>>> + if (info->page_flag_buf[i]->ready == FLAG_UNUSED) > >>>> + continue; > >>>> + temp_pfn = info->page_flag_buf[i]->pfn; > >>>> > >>>> - gettimeofday(&new, NULL); > >>>> - if (new.tv_sec - last.tv_sec > WAIT_TIME) { > >>>> - ERRMSG("Can't get data of pfn %llx.\n", consuming_pfn); > >>>> - goto out; > >>>> - } > >>>> + /* > >>>> + * count how many threads have reached the end. > >>>> + */ > >>>> + if (temp_pfn >= end_pfn) { > >>>> + /* > >>>> + * prevent setting FLAG_UNUSED being optimized. > >>>> + */ > >>>> + MSG("-"); > >>> > >>> Why does this line need? > >>> Please clarify why FLAG_UNUSED is optimized. > >>> > >> > >> I tried but still can't figure out the reason or get better solution. > >> I will be so happy if you can help me. > >> > >> In gdb, I run it by step. > >> The code "info->page_flag_buf[i]->ready = FLAG_UNUSED;" can never be reached. > >> If a breakpoint is set here, the code will never be skipped. > > > > On my environment, RHEL7.2, gdb stops at this line even if MSG("-") is deleted. > > See below. > > > > === > > [root at ark crash]# gdb makedumpfile-code-3d32567394bac85ee3247780bfa41072b0d9bfe9/makedumpfile > > GNU gdb (GDB) Red Hat Enterprise Linux 7.6.1-80.el7 > > Copyright (C) 2013 Free Software Foundation, Inc. > > License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> > > This is free software: you are free to change and redistribute it. > > There is NO WARRANTY, to the extent permitted by law. Type "show copying" > > and "show warranty" for details. > > This GDB was configured as "x86_64-redhat-linux-gnu". > > For bug reporting instructions, please see: > > <http://www.gnu.org/software/gdb/bugs/>... > > Reading symbols from /work/crash/makedumpfile-code-3d32567394bac85ee3247780bfa41072b0d9bfe9/makedumpfile...done. > > (gdb) set args -l -d0 --num-threads=2 ./vmcore ./vmcore.new.2 > > (gdb) l 7780 > > 7775 if (info->page_flag_buf[i]->ready == FLAG_UNUSED) > > 7776 continue; > > 7777 temp_pfn = info->page_flag_buf[i]->pfn; > > 7778 > > 7779 /* > > 7780 * count how many threads have reached the end. > > 7781 */ > > 7782 if (temp_pfn >= end_pfn) { > > 7783 info->page_flag_buf[i]->ready = FLAG_UNUSED; > > 7784 > > (gdb) b 7783 > > Breakpoint 1 at 0x433778: file makedumpfile.c, line 7783. > > (gdb) > > (gdb) run > > Starting program: /work/crash/makedumpfile-code-3d32567394bac85ee3247780bfa41072b0d9bfe9/makedumpfile -l -d0 > --num-threads=2 ./vmcore ./vmcore.new.2 > > [Thread debugging using libthread_db enabled] > > Using host libthread_db library "/lib64/libthread_db.so.1". > > The kernel version is not supported. > > The makedumpfile operation may be incomplete. > > Checking for memory holes : [100.0 %] |[New Thread 0x7ffff6530700 (LWP 32123)] > > [New Thread 0x7ffff5d2f700 (LWP 32124)] > > Copying data : [ 98.8 %] | > > Breakpoint 1, write_kdump_pages_parallel_cyclic (cd_header=<optimized out>, > > cd_page=<optimized out>, pd_zero=<optimized out>, offset_data=<optimized out>, > > cycle=<optimized out>) at makedumpfile.c:7783 > > 7783 info->page_flag_buf[i]->ready = FLAG_UNUSED; > > Missing separate debuginfos, use: debuginfo-install bzip2-libs-1.0.6-13.el7.x86_64 elfutils-libelf-0.163-3.el7.x86_64 > elfutils-libs-0.163-3.el7.x86_64 glibc-2.17-105.el7.x86_64 lzo-2.06-8.el7.x86_64 xz-libs-5.1.2-12alpha.el7.x86_64 > zlib-1.2.7-15.el7.x86_64 > > (gdb) bt > > #0 write_kdump_pages_parallel_cyclic (cd_header=<optimized out>, cd_page=<optimized out>, > > pd_zero=<optimized out>, offset_data=<optimized out>, cycle=<optimized out>) > > at makedumpfile.c:7783 > > #1 0x0000000000439a15 in write_kdump_pages_and_bitmap_cyclic ( > > cd_header=cd_header at entry=0x7fffffffe2d0, cd_page=cd_page at entry=0x7fffffffe300) > > at makedumpfile.c:8501 > > #2 0x000000000043a918 in writeout_dumpfile () at makedumpfile.c:9497 > > #3 0x000000000044181c in create_dumpfile () at makedumpfile.c:9724 > > #4 0x00000000004086df in main (argc=<optimized out>, argv=0x7fffffffe468) > > at makedumpfile.c:11159 > > (gdb) > > === > > > > Yes, it won't be skipped every time without MSG("-"). > But it did occur sometime. > And unluckily, MSG("-") can't do much help either. > > I guess it is the same problem that minfei meets. > > I'm doing some further investigation. > > >>>> > >>>> - /* > >>>> - * check pfn first without mutex locked to reduce the time > >>>> - * trying to lock the mutex > >>>> - */ > >>>> - if (page_data_buf[index].pfn != consuming_pfn) > >>>> - continue; > >>>> + info->page_flag_buf[i]->ready = FLAG_UNUSED; > >>>> > >>>> - if (pthread_mutex_trylock(&page_data_buf[index].mutex) != 0) > >>>> - continue; > >>>> + info->current_pfn = end_pfn; > >>> > >>> This part doesn't get info->current_pfn_mutex. > >>> It becomes bigger than end_pfn at the end of producer thread in following case. > >>> > >>> === > >>> producer Consumer > >>> --------------------------------------------------------- > >>> pthread_mutex_lock() > >>> pfn = info->current_pfn > >>> info->current_pfn = end_pfn > >>> info->current_pfn++ > >>> -> end_pfn + 1 > >>> pthread_mutex_unlock() > >>> === > >>> > >>> But I know, if this race is happened, processing other producer thread and consumer thread works well > >>> in current cycle. > >>> Because each thread checks whether info->current_pfn >= end_pfn. > >>> > >>> On the other hand, info->current_pfn is initialized to cycle->start_pfn before pthread_create() > >>> in write_kdump_pages_parallel_cyclic(). > >>> This means it does not cause a problem in next cycle, too. > >>> > >>> In other words, my point is following. > >>> > >>> a) When info->current_pfn changes, you have to get info->current_pfn_mutex. > >>> b) If you allow info->current_pfn is bigger than end_pfn at the end of each cycle, > >>> "info->current_pfn = end_pfn;" is unnecessary. > >>> > >>> Honestly, I don't like approach b). > >> > >> You're right. Some thing I thought is wrong. > >> But I don't like lock if I have other choice. > >> I will set info->current_pfn before returning. > >> How about it? > > > > If you mean you set info->current_pfn by producer side, > > this race will occur between producer A and producer B. > > > > I think you can't avoid getting mutex lock, if you will change info->current_pfn. > > > > I mean setting it by consumer. I'm sorry, I can't imagine your proposal. What do you change? Please show me the code. --- Thanks Minoru Usui > -- > Thanks > Zhou > > > Thanks, > > Minoru Usui > > > > > >> -- > >> Thanks > >> Zhou > >> > >>> > >>> Thanks, > >>> Minoru Usui > >>> > >>>> + end_count++; > >>>> + continue; > >>>> + } > >>>> + > >>>> + if (current_pfn < temp_pfn) > >>>> + continue; > >>>> > >>>> - /* check whether the found one is ready to be consumed */ > >>>> - if (page_data_buf[index].pfn != consuming_pfn || > >>>> - page_data_buf[index].ready != 1) { > >>>> - goto unlock; > >>>> + check_count++; > >>>> + consuming = i; > >>>> + current_pfn = temp_pfn; > >>>> + } > >>>> + > >>>> + /* > >>>> + * If all the threads have reached the end, we will finish writing. > >>>> + */ > >>>> + if (end_count >= info->num_threads) > >>>> + goto finish; > >>>> + > >>>> + /* > >>>> + * Since it has the probabilty that there is no page_flag_buf being ready, > >>>> + * we should recheck if it happens. > >>>> + */ > >>>> + if (check_count == 0) > >>>> + continue; > >>>> + > >>>> + /* > >>>> + * If the page_flag_buf is not ready, the pfn recorded may be changed. > >>>> + * So we should recheck. > >>>> + */ > >>>> + if (info->page_flag_buf[consuming]->ready != FLAG_READY) { > >>>> + gettimeofday(&new, NULL); > >>>> + if (new.tv_sec - last.tv_sec > WAIT_TIME) { > >>>> + ERRMSG("Can't get data of pfn.\n"); > >>>> + goto out; > >>>> + } > >>>> + continue; > >>>> + } > >>>> + > >>>> + if (current_pfn == info->page_flag_buf[consuming]->pfn) > >>>> + break; > >>>> } > >>>> > >>>> if ((num_dumped % per) == 0) > >>>> print_progress(PROGRESS_COPY, num_dumped, info->num_dumpable); > >>>> > >>>> - /* next pfn is found, refresh last here */ > >>>> - last = new; > >>>> - consuming_pfn++; > >>>> - info->consumed_pfn++; > >>>> - page_data_buf[index].ready = 0; > >>>> - > >>>> - if (page_data_buf[index].dumpable == FALSE) > >>>> - goto unlock; > >>>> - > >>>> num_dumped++; > >>>> > >>>> - if (page_data_buf[index].zero == TRUE) { > >>>> + > >>>> + if (info->page_flag_buf[consuming]->zero == TRUE) { > >>>> if (!write_cache(cd_header, pd_zero, sizeof(page_desc_t))) > >>>> goto out; > >>>> pfn_zero++; > >>>> } else { > >>>> + index = info->page_flag_buf[consuming]->index; > >>>> pd.flags = page_data_buf[index].flags; > >>>> pd.size = page_data_buf[index].size; > >>>> pd.page_flags = 0; > >>>> @@ -7420,12 +7513,12 @@ write_kdump_pages_parallel_cyclic(struct cache_data *cd_header, > >>>> */ > >>>> if (!write_cache(cd_page, page_data_buf[index].buf, pd.size)) > >>>> goto out; > >>>> - > >>>> + page_data_buf[index].used = FALSE; > >>>> } > >>>> -unlock: > >>>> - pthread_mutex_unlock(&page_data_buf[index].mutex); > >>>> + info->page_flag_buf[consuming]->ready = FLAG_UNUSED; > >>>> + info->page_flag_buf[consuming] = info->page_flag_buf[consuming]->next; > >>>> } > >>>> - > >>>> +finish: > >>>> ret = TRUE; > >>>> /* > >>>> * print [100 %] > >>>> @@ -7464,7 +7557,7 @@ out: > >>>> } > >>>> > >>>> if (page_data_buf != NULL) { > >>>> - for (i = 0; i < page_data_num; i++) { > >>>> + for (i = 0; i < page_buf_num; i++) { > >>>> pthread_mutex_destroy(&page_data_buf[i].mutex); > >>>> } > >>>> } > >>>> @@ -7564,6 +7657,7 @@ write_kdump_pages_cyclic(struct cache_data *cd_header, struct cache_data *cd_pag > >>>> num_dumped++; > >>>> if (!read_pfn(pfn, buf)) > >>>> goto out; > >>>> + > >>>> filter_data_buffer(buf, pfn_to_paddr(pfn), info->page_size); > >>>> > >>>> /* > >>>> diff --git a/makedumpfile.h b/makedumpfile.h > >>>> index e0b5bbf..8a9a5b2 100644 > >>>> --- a/makedumpfile.h > >>>> +++ b/makedumpfile.h > >>>> @@ -977,7 +977,7 @@ typedef unsigned long long int ulonglong; > >>>> #define PAGE_DATA_NUM (50) > >>>> #define WAIT_TIME (60 * 10) > >>>> #define PTHREAD_FAIL ((void *)-2) > >>>> -#define NUM_BUFFERS (50) > >>>> +#define NUM_BUFFERS (20) > >>>> > >>>> struct mmap_cache { > >>>> char *mmap_buf; > >>>> @@ -985,28 +985,36 @@ struct mmap_cache { > >>>> off_t mmap_end_offset; > >>>> }; > >>>> > >>>> +enum { > >>>> + FLAG_UNUSED, > >>>> + FLAG_READY, > >>>> + FLAG_FILLING > >>>> +}; > >>>> +struct page_flag { > >>>> + mdf_pfn_t pfn; > >>>> + char zero; > >>>> + char ready; > >>>> + short index; > >>>> + struct page_flag *next; > >>>> +}; > >>>> + > >>>> struct page_data > >>>> { > >>>> - mdf_pfn_t pfn; > >>>> - int dumpable; > >>>> - int zero; > >>>> - unsigned int flags; > >>>> + pthread_mutex_t mutex; > >>>> long size; > >>>> unsigned char *buf; > >>>> - pthread_mutex_t mutex; > >>>> - /* > >>>> - * whether the page_data is ready to be consumed > >>>> - */ > >>>> - int ready; > >>>> + int flags; > >>>> + int used; > >>>> }; > >>>> > >>>> struct thread_args { > >>>> int thread_num; > >>>> unsigned long len_buf_out; > >>>> mdf_pfn_t start_pfn, end_pfn; > >>>> - int page_data_num; > >>>> + int page_buf_num; > >>>> struct cycle *cycle; > >>>> struct page_data *page_data_buf; > >>>> + struct page_flag *page_flag_buf; > >>>> }; > >>>> > >>>> /* > >>>> @@ -1295,6 +1303,7 @@ struct DumpInfo { > >>>> pthread_t **threads; > >>>> struct thread_args *kdump_thread_args; > >>>> struct page_data *page_data_buf; > >>>> + struct page_flag **page_flag_buf; > >>>> pthread_rwlock_t usemmap_rwlock; > >>>> mdf_pfn_t current_pfn; > >>>> pthread_mutex_t current_pfn_mutex; > >>>> -- > >>>> 1.8.3.1 > >>>> > >>>> > >>>> > >>>> > >>>> _______________________________________________ > >>>> kexec mailing list > >>>> kexec at lists.infradead.org > >>>> http://lists.infradead.org/mailman/listinfo/kexec > >> > >> > > > > > > >