Re: [LTP] [PATCH] hotplug/cpu_hotplug: add it to run default and cleanup

[gux.fnst <gux...@cn...>]

On 06/12/2014 08:10 PM, ch...@su... wrote:
> Hi!
>> -cpuhotplug01 export CPU_TO_TEST=1; $LTPROOT/testcases/bin/cpu_hotplug/functional/cpuhotplug01.sh
>> -cpuhotplug02 export CPU_TO_TEST=1; $LTPROOT/testcases/bin/cpu_hotplug/functional/cpuhotplug02.sh
>> -cpuhotplug03 export CPU_TO_TEST=1; $LTPROOT/testcases/bin/cpu_hotplug/functional/cpuhotplug03.sh
>> -cpuhotplug04 export CPU_TO_TEST=1; $LTPROOT/testcases/bin/cpu_hotplug/functional/cpuhotplug04.sh
>> -cpuhotplug05 export CPU_TO_TEST=1; $LTPROOT/testcases/bin/cpu_hotplug/functional/cpuhotplug05.sh
>> -cpuhotplug06 export CPU_TO_TEST=1; $LTPROOT/testcases/bin/cpu_hotplug/functional/cpuhotplug06.sh
>> -cpuhotplug07 export CPU_TO_TEST=1; $LTPROOT/testcases/bin/cpu_hotplug/functional/cpuhotplug07.sh
>> +#cpuhotplug01; $LTPROOT/testcases/bin/cpu_hotplug/functional/cpuhotplug01.sh -c 1 -l 1 -n 1 -f 1 -e 6
>> +cpuhotplug02; $LTPROOT/testcases/bin/cpu_hotplug/functional/cpuhotplug02.sh -c 1 -l 1
>> +cpuhotplug03; $LTPROOT/testcases/bin/cpu_hotplug/functional/cpuhotplug03.sh -c 1 -l 1
>> +cpuhotplug04; $LTPROOT/testcases/bin/cpu_hotplug/functional/cpuhotplug04.sh -l 1
>> +cpuhotplug05; $LTPROOT/testcases/bin/cpu_hotplug/functional/cpuhotplug05.sh -c 1 -l 1 -d /tmp
>> +cpuhotplug06; $LTPROOT/testcases/bin/cpu_hotplug/functional/cpuhotplug06.sh -c 1 -l 1
>> +cpuhotplug07; $LTPROOT/testcases/bin/cpu_hotplug/functional/cpuhotplug07.sh -c 1 -l 1 -d /usr/src/linux
>
> Ideally the cpuhotplug*.sh tests should be installed under the
> testcases/bin/ so we can call them without the full path. Or is there
> any good reason why they are not?

Oh, sorry, I have not considered the aspect of install directory, and 
indeed it is better to install this tests under the testcases/bin/.

>
>> diff --git a/testcases/kernel/hotplug/cpu_hotplug/functional/cpuhotplug01.sh b/testcases/kernel/hotplug/cpu_hotplug/functional/cpuhotplug01.sh
>> index 07379dc..acf28a9 100755
>> --- a/testcases/kernel/hotplug/cpu_hotplug/functional/cpuhotplug01.sh
>> +++ b/testcases/kernel/hotplug/cpu_hotplug/functional/cpuhotplug01.sh
>> @@ -5,21 +5,13 @@
>>   # Based on script by Ashok Raj <ash...@in...>
>>   # Modified by Mark D and Bryce, Aug '05.
>>
>> -HOTPLUG01_LOOPS=${HOTPLUG01_LOOPS:-${LOOPS}}
>>   export TCID="cpuhotplug01"
>> -export TST_COUNT=1
>> -export TST_TOTAL=${HOTPLUG01_LOOPS:-1}
>> -
>> -CPU_TO_TEST=${CPU_TO_TEST:-1}
>> -if [ -z "${CPU_TO_TEST}" ]; then
>> -	echo "usage: ${0##*/} <CPU to online>"
>> -	exit 1
>> -fi
>> +export TST_TOTAL=1
>>
>>   # Includes:
>> -LHCS_PATH=${LHCS_PATH:-$LTPROOT/testcases/bin/cpu_hotplug}
>> -. $LHCS_PATH/include/testsuite.fns
>> -. $LHCS_PATH/include/hotplug.fns
>> +. test.sh
>> +. cpu_hotplug/include/testsuite.fns
>> +. cpu_hotplug/include/hotplug.fns
>
> These files should be renamed to have standard .sh extension.

Got it.

>
>>   cat <<EOF
>>   Name:   $TCID
>> @@ -28,39 +20,21 @@ Desc:   What happens to disk controller interrupts when offlining CPUs?
>>
>>   EOF
>>
>> +
>> +TST_CLEANUP=do_clean
>> +
>> +CPU_COUNT=0
>> +
>>   # Start up a process that writes to disk; keep track of its PID
>> -$LHCS_PATH/tools/do_disk_write_loop > /dev/null 2>&1 &
>> +./cpu_hotplug/tools/do_disk_write_loop > /dev/null 2>&1 &
>
> The ./ here is problematic too. It expects that the CWD for the test is
> $LTPROOT/testcases/bin/ which is only true if test does not create the
> test temporary directory. This will break for no apparent reason if
> somebody adds tst_tmpdir() to the test setup.
>
> The best soulution would be renaming the tools to start with
> 'cpuhotplug_' and installing them to testcases/bin/ directory so that
> the could be executed without the leading path.
>

Got it.

>>   WRL_ID=$!
>>
>> -RC=0
>> -until [ $TST_COUNT -gt $TST_TOTAL -o $RC -ne 0 ]
>> +until [ $LOOP_COUNT -gt $HOTPLUG01_LOOPS ]
>>   do
>>
>> @@ -172,20 +184,14 @@ do
>>   	# Print out a report showing the changes in IRQs
>>   	echo
>>   	echo
>> -	$LHCS_PATH/tools/report_proc_interrupts "$IRQ_START" "$IRQ_END"
>> +	./cpu_hotplug/tools/report_proc_interrupts "$IRQ_START" "$IRQ_END"
>
> Here as well.
>

Got it.

Thanks for your suggestion. I will send a new version v2.

Regards,
Xing Gu