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(a)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?
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.
--
Thanks
Zhou