[PATCH v2] Add checking num-threads of makedumpfile

[Dave Young]

On 09/08/15 at 03:32pm, "Zhou, Wenjian/周文剑" wrote:
> On 09/08/2015 03:15 PM, Dave Young wrote:
> >On 09/08/15 at 10:21am, "Zhou, Wenjian/周文剑" wrote:
> >>On 08/26/2015 04:26 PM, Dave Young wrote:
> >>>On 08/26/15 at 04:18pm, "Zhou, Wenjian/周文剑" wrote:
> >>>>Hello Dave,
> >>>>
> >>>>On 08/26/2015 03:59 PM, Dave Young wrote:
> >>>>>Hi,
> >>>>>
> >>>>>On 08/25/15 at 04:43pm, Zhou Wenjian wrote:
> >>>>>>A new feature that doing compressing and writing by multi-threads
> >>>>>>has been added in makedumpfile. The thread num is specified by
> >>>>>>"--num-threads NUM". According to its implementation, there will
> >>>>>>be performance degradation if the threads are more than cpus.
> >>>>>>So we should check it.
> >>>>>>
> >>>>>>Signed-off-by: Zhou wenjian <zhouwj-fnst at cn.fujitsu.com>
> >>>>>>---
> >>>>>>  kdumpctl |   15 +++++++++++++++
> >>>>>>  1 files changed, 15 insertions(+), 0 deletions(-)
> >>>>>>
> >>>>>>diff --git a/kdumpctl b/kdumpctl
> >>>>>>index b504734..fd2abe4 100755
> >>>>>>--- a/kdumpctl
> >>>>>>+++ b/kdumpctl
> >>>>>>@@ -259,6 +259,21 @@ check_config()
> >>>>>>  		esac
> >>>>>>  	done < $KDUMP_CONFIG_FILE
> >>>>>>
> >>>>>>+	grep "^core_collector.*makedumpfile" $KDUMP_CONFIG_FILE | grep -q "num-threads"
> >>>>>>+	if [ $? -eq 0 ];then
> >>>>>>+		local nr_cpus=1
> >>>>>>+		local num_threads=0
> >>>>>>+		local core_collector=`grep "^core_collector" $KDUMP_CONFIG_FILE`
> >>>>>>+
> >>>>>>+		num_threads=`echo ${core_collector#*--num-threads} | awk '{print $1}'`
> >>>>>>+		nr_cpus=`echo ${KDUMP_COMMANDLINE_APPEND#*nr_cpus=} | awk '{print $1}'`
> >>>>>>+
> >>>>>>+		if [ $num_threads -ge $nr_cpus ];then
> >>>>>>+			echo "The num_threads:$num_threads(specified in /etc/kdump.conf) should be less than nr_cpus:$nr_cpus(specified in /etc/sysconfig/kdump)"
> >>>>>>+			echo "or makedumpfile may have bad performance!"
> >>>>>
> >>>>>It is better to do this in makedumpfile source code instead of adding
> >>>>>these warnings specific for num-threads here.
> >>>>>
> >>>>
> >>>>I have one more question.
> >>>>Since multi-thread feature is associated with nr_cpus, should we do something here?
> >>>
> >>>Hmm, adding it in makedumpfile will be too late for warning users. But I do not like
> >>>adding these stuff in distribution script, user should know the limitation if he
> >>>choose the thread number. How about moving it to makedumpfile --help to describe
> >>>the problem just around the num_threads option.
> >>>
> >>
> >>Whatever, we should not introduce nr_cpus in makedumpfile --help.
> >>I noticed a discussion between Vivek and Hatayama.
> >>(You can refer to https://lists.fedoraproject.org/pipermail/kexec/2014-March/000687.html)
> >>I think finishing that patch may help.
> >>I will post the patch later.
> >>Please give me some comments on the later patch.
> >
> >I'm fine to add something in kexec kdump howto document, it is better than adding
> >more logic in kdumpctl.
> >
> >Why do you think it is bad to add it to makedumpfile? Maybe makedumpfile manpage?
> >
> 
> I just think it's not good to add the description of setting nr_cpus to makedumpfile.
> 
> As I mentioned in another thread:
> In kexec, we should tell users how to bring up multiple cpus and
> what the benefit is.
> In makedumpfiel, we should tell users how to use multiple threads
> and what may affect the performance.
> 
> What do you think about Minfei's idea that adding an option to turn on/off
> parallel dumping?

I think an option to turn on and off is reasonable, but one more option to
set num_threads is also necessary if someone want to customize it.

Suppose --num-threads is provided it will be used, if there's no --num-threads
and the switch option is on then makedumpfile can check cpu number and select
proper threads number. 

Thanks
Dave