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@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" + 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 + 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!" + fi + fi + check_fence_kdump_config || return 1
return 0
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@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.
- if [ $? -eq 0 ];then
local nr_cpus=1local num_threads=0local 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/nullif [ $? -eq 0 ];then
To keep the style, it is better to use "if [ $a -ge $b ]".
Thanks Minfei
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@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=1local num_threads=0local 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/nullif [ $? -eq 0 ];thenTo 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.
BTW, should we add some descriptions to this feature of makedumpfile in some where?
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@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=1local num_threads=0local 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/nullif [ $? -eq 0 ];thenTo 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
On 08/25/2015 02:11 PM, Minfei Huang wrote:
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@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=1local num_threads=0local 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/nullif [ $? -eq 0 ];thenTo 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?
Of course not integer is invalid. But the value of num_threads won't be checked until makedumpfile is executed. If user sets "core_collector makedumpfile --num-threads a" in kdump.conf, it will lead to a fault here.
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.
On 08/25/15 at 02:17pm, "Zhou, Wenjian/周文剑" wrote:
- if [ $? -eq 0 ];then
local nr_cpus=1local num_threads=0local 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/nullif [ $? -eq 0 ];thenTo 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?
Of course not integer is invalid. But the value of num_threads won't be checked until makedumpfile is executed. If user sets "core_collector makedumpfile --num-threads a" in kdump.conf, it will lead to a fault here.
I think it is fine without validating the type of num-threads, since makedumpfile will do this work.
kdump does not check other usages of makedumpfile as well. It is better to wrap a new function to check it in kdumpctl, if you want to try to validate the usage value.
Thanks Minfei
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.
On 08/25/2015 02:57 PM, Minfei Huang wrote:
On 08/25/15 at 02:17pm, "Zhou, Wenjian/周文剑" wrote:
- if [ $? -eq 0 ];then
local nr_cpus=1local num_threads=0local 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/nullif [ $? -eq 0 ];thenTo 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?
Of course not integer is invalid. But the value of num_threads won't be checked until makedumpfile is executed. If user sets "core_collector makedumpfile --num-threads a" in kdump.conf, it will lead to a fault here.
I think it is fine without validating the type of num-threads, since makedumpfile will do this work.
kdump does not check other usages of makedumpfile as well. It is better to wrap a new function to check it in kdumpctl, if you want to try to validate the usage value.
I agree with you that we should not validate the type of num-threads. But using "test" is not to validate the type. It is just for avoiding the validating. If don't avoid it, the fault will occur here. And user may feel strange about the error message.
On 08/25/15 at 03:21pm, "Zhou, Wenjian/周文剑" wrote:
On 08/25/2015 02:57 PM, Minfei Huang wrote:
On 08/25/15 at 02:17pm, "Zhou, Wenjian/周文剑" wrote:
>+ 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?
Of course not integer is invalid. But the value of num_threads won't be checked until makedumpfile is executed. If user sets "core_collector makedumpfile --num-threads a" in kdump.conf, it will lead to a fault here.
I think it is fine without validating the type of num-threads, since makedumpfile will do this work.
kdump does not check other usages of makedumpfile as well. It is better to wrap a new function to check it in kdumpctl, if you want to try to validate the usage value.
I agree with you that we should not validate the type of num-threads. But using "test" is not to validate the type. It is just for avoiding the validating. If don't avoid it, the fault will occur here. And user may feel strange about the error message.
If we can detect the error as early as we can, the rate we dump the vmcore successfully is higher.
Thanks Minfei
On 08/25/2015 03:36 PM, Minfei Huang wrote:
On 08/25/15 at 03:21pm, "Zhou, Wenjian/周文剑" wrote:
On 08/25/2015 02:57 PM, Minfei Huang wrote:
On 08/25/15 at 02:17pm, "Zhou, Wenjian/周文剑" wrote:
>> + 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?
Of course not integer is invalid. But the value of num_threads won't be checked until makedumpfile is executed. If user sets "core_collector makedumpfile --num-threads a" in kdump.conf, it will lead to a fault here.
I think it is fine without validating the type of num-threads, since makedumpfile will do this work.
kdump does not check other usages of makedumpfile as well. It is better to wrap a new function to check it in kdumpctl, if you want to try to validate the usage value.
I agree with you that we should not validate the type of num-threads. But using "test" is not to validate the type. It is just for avoiding the validating. If don't avoid it, the fault will occur here. And user may feel strange about the error message.
If we can detect the error as early as we can, the rate we dump the vmcore successfully is higher.
Do you mean it's also needed to check the usage of makedumpfile in kdumpctl, even makedumpfile will do this?
I think kdump should not do makedumpfile's work. kdump just provides a way for users to use different core collectors. There is no need for kdump to check how the core collector users use.
On 08/25/15 at 04:07pm, "Zhou, Wenjian/周文剑" wrote:
On 08/25/2015 03:36 PM, Minfei Huang wrote:
On 08/25/15 at 03:21pm, "Zhou, Wenjian/周文剑" wrote:
On 08/25/2015 02:57 PM, Minfei Huang wrote:
On 08/25/15 at 02:17pm, "Zhou, Wenjian/周文剑" wrote:
>>>+ 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?
Of course not integer is invalid. But the value of num_threads won't be checked until makedumpfile is executed. If user sets "core_collector makedumpfile --num-threads a" in kdump.conf, it will lead to a fault here.
I think it is fine without validating the type of num-threads, since makedumpfile will do this work.
kdump does not check other usages of makedumpfile as well. It is better to wrap a new function to check it in kdumpctl, if you want to try to validate the usage value.
I agree with you that we should not validate the type of num-threads. But using "test" is not to validate the type. It is just for avoiding the validating. If don't avoid it, the fault will occur here. And user may feel strange about the error message.
If we can detect the error as early as we can, the rate we dump the vmcore successfully is higher.
Do you mean it's also needed to check the usage of makedumpfile in kdumpctl, even makedumpfile will do this?
I think kdump should not do makedumpfile's work. kdump just provides a way for users to use different core collectors. There is no need for kdump to check how the core collector users use.
You are right that kdump does not need to do the work which makdumpfile does.
I mean you can leave the condition to use "if [ $a -ge $b ]". It is fine if kdump raises the error message due to the invalid value.
Thanks Minfei
On 08/25/2015 04:18 PM, Minfei Huang wrote:
On 08/25/15 at 04:07pm, "Zhou, Wenjian/周文剑" wrote:
On 08/25/2015 03:36 PM, Minfei Huang wrote:
On 08/25/15 at 03:21pm, "Zhou, Wenjian/周文剑" wrote:
On 08/25/2015 02:57 PM, Minfei Huang wrote:
On 08/25/15 at 02:17pm, "Zhou, Wenjian/周文剑" wrote:
>>>> + 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? >
Of course not integer is invalid. But the value of num_threads won't be checked until makedumpfile is executed. If user sets "core_collector makedumpfile --num-threads a" in kdump.conf, it will lead to a fault here.
I think it is fine without validating the type of num-threads, since makedumpfile will do this work.
kdump does not check other usages of makedumpfile as well. It is better to wrap a new function to check it in kdumpctl, if you want to try to validate the usage value.
I agree with you that we should not validate the type of num-threads. But using "test" is not to validate the type. It is just for avoiding the validating. If don't avoid it, the fault will occur here. And user may feel strange about the error message.
If we can detect the error as early as we can, the rate we dump the vmcore successfully is higher.
Do you mean it's also needed to check the usage of makedumpfile in kdumpctl, even makedumpfile will do this?
I think kdump should not do makedumpfile's work. kdump just provides a way for users to use different core collectors. There is no need for kdump to check how the core collector users use.
You are right that kdump does not need to do the work which makdumpfile does.
I mean you can leave the condition to use "if [ $a -ge $b ]". It is fine if kdump raises the error message due to the invalid value.
I accept it.
-
Minfei Huang -
Zhou Wenjian