Hi, Zhou I wrote a typo, sorry. > -----Original Message----- > From: kexec [mailto:kexec-bounces at lists.infradead.org] On Behalf Of Minoru Usui > Sent: Wednesday, March 02, 2016 12:06 PM > To: "Zhou, Wenjian/???" <zhouwj-fnst at cn.fujitsu.com>; kexec at lists.infradead.org > Subject: Re: [PATCH v2] Improve the performance of --num-threads -d 31 > > Hi, Zhou > > > >>>>> === > > >>>>> producer Consumer > > >>>>> --------------------------------------------------------- > > >>>>> pthread_mutex_lock() > > >>>>> pfn = info->current_pfn > > >>>>> info->current_pfn = end_pfn > > >>>>> info->current_pfn++ > > >>>>> -> end_pfn + 1 > > >>>>> pthread_mutex_unlock() > > >>>>> === > > > > How about just changing "info->current_pfn = end_pfn" to "info->current_pfn--" ? > > Just like the first version of the patch. > > If you don't get mutex lock in consumer side, this change is meaningless. > Of course, info->current_pfn may equal to end_pfn at the end of the cycle, > but there is a timing that info->current_pfn is bigger than end_pfn in processing producer thread. > > The root cause is producer increments info->current_pfn everytime, even if info->current_pfn == end_pfn > in following code. > > === > >>>> + /* get next pfn */ > >>>> + pthread_mutex_lock(&info->current_pfn_mutex); > >>>> + pfn = info->current_pfn; > >>>> + info->current_pfn++; # increment everytime > >>>> + 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; # not decrement > >>>> + } > === > > If you don't allow info->current_pfn is bigger than end_pfn, > you don't need to increment info->current_pfn when pfn >= kdump_thread_args->end_pfn like following. > > === > /* get next pfn */ > pthread_mutex_lock(&info->current_pfn_mutex); > pfn = info->current_pfn; > page_flag_buf->pfn = pfn; > if (pfn >= kdump_thread_args->end_pfn) { > page_data_buf[index].used = FALSE; > page_flag_buf->ready = FLAG_READY; > pthread_mutex_unlock(&info->current_pfn_mutex); > break; > } > page_flag_buf->ready = FLAG_FILLING; > info->current_pfn++; > pthread_mutex_unlock(&info->current_pfn_mutex); > === > > If you allow info->current_pfn is bigger than end_pfn, producer doesn't need to change info->current_pfn. ^^^^^^^^ consumer Consumer doesn't need to change info->current_pfn. --- Thanks Minoru Usui