[PATCH] Add checking num-threads of makedumpfile

Minfei Huang mhuang at redhat.com
Tue Aug 25 06:11:37 UTC 2015


On 08/25/15 at 01:49pm, "Zhou, Wenjian/周文剑" wrote:
> Hello Minfei,
> 
> Thanks for your quick reply.
> 
> On 08/25/2015 01:42 PM, Minfei Huang wrote:
> >On 08/25/15 at 12:51pm, 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 |   16 ++++++++++++++++
> >>  1 files changed, 16 insertions(+), 0 deletions(-)
> >>
> >>diff --git a/kdumpctl b/kdumpctl
> >>index b504734..a4b4681 100755
> >>--- a/kdumpctl
> >>+++ b/kdumpctl
> >>@@ -259,6 +259,22 @@ check_config()
> >>  		esac
> >>  	done < $KDUMP_CONFIG_FILE
> >>
> >>+	grep -v "^#" $KDUMP_CONFIG_FILE | grep -q "num-threads"
> >
> >This filter cannot handle the all of the corner cases, like the
> >following.
> >
> >  # grep -v "^#" /etc/kdump.conf  | grep "num-threads"
> >  ext4 /dev/mapper/num-threads
> >
> >So it is better to use the exact regular expression to filter out the
> >string. Maybe you can use $core_collector to filter out the string
> >--num_threads.
> >
> 
> I see.
> 
> >>+	if [ $? -eq 0 ];then
> >>+		local nr_cpus=1
> >>+		local num_threads=0
> >>+		local core_collector=`grep -v "^#" $KDUMP_CONFIG_FILE | grep "^core_collector"`
> >>+
> >>+		num_threads=`echo ${core_collector#*--num-threads} | awk '{print $1}'`
> >>+		nr_cpus=`echo ${KDUMP_COMMANDLINE_APPEND#*nr_cpus=} | awk '{print $1}'`
> >>+
> >>+		test $num_threads -ge $nr_cpus &> /dev/null
> >>+		if [ $? -eq 0 ];then
> >
> >To keep the style, it is better to use "if [ $a -ge $b ]".
> >
> 
> Using "test" is to handle the situation that the $num_threads or $nr_cpus is not an integer.
> 

Is it allowed for $num_threads or $nr_cpus, if the value is not the
integer?

> BTW, should we add some descriptions to this feature of makedumpfile in some where?

IMO, it is appropriate to document it in the makedumpfile manpage, like
the feature --split. For kdump, we only document the default usages for
makedumpfile in kexec-kdump-howto.txt.

Thanks
Minfei


More information about the kexec mailing list