Re: [PATCH POWERTOP] Remove temporary file created during calibration
Mohammad, This fix should go upstream. cc'ing the powertop list. Regards, Amit On Thu, Jun 26, 2014 at 12:27 PM, Mohammad Merajul Islam Molla wrote: > powertop creates temporary file in /tmp folder during calibration. > Simply closing does not remove these files. > Instead use unlink to remove the file. > > diff --git a/src/calibrate/calibrate.cpp b/src/calibrate/calibrate.cpp > index db368e0..1336c3f 100644 > --- a/src/calibrate/calibrate.cpp > +++ b/src/calibrate/calibrate.cpp > @@ -242,7 +242,7 @@ static void *burn_disk(void *dummy) > write(fd, buffer, 64*1024); > fdatasync(fd); > } > - close(fd); > + unlink(filename); > return NULL; > } > > > > -- > Thanks, > -Meraj > > ___ > linaro-dev mailing list > linaro-dev@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/linaro-dev ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH POWERTOP] Fix various resource leaks
Mohammad,
This fix should go upstream. cc'ing the powertop list.
Regards,
Amit
On Thu, Jun 26, 2014 at 12:42 PM, Mohammad Merajul Islam Molla
wrote:
> Fixes some resource leaks detected by valgrind and coverity scan.
>
>
> diff --git a/src/devices/ahci.cpp b/src/devices/ahci.cpp
> index ac06460..7f704b6 100644
> --- a/src/devices/ahci.cpp
> +++ b/src/devices/ahci.cpp
> @@ -64,8 +64,10 @@ static string disk_name(char *path, char *target,
> char *shortname)
> sprintf(line, "%s/%s/model", pathname, dirent->d_name);
> file = fopen(line, "r");
> if (file) {
> - if (fgets(line, 4096, file) == NULL)
> + if (fgets(line, 4096, file) == NULL) {
> + fclose(file);
> break;
> + }
> fclose(file);
> c = strchr(line, '\n');
> if (c)
> diff --git a/src/devices/devfreq.cpp b/src/devices/devfreq.cpp
> index e16951c..23c4b0c 100644
> --- a/src/devices/devfreq.cpp
> +++ b/src/devices/devfreq.cpp
> @@ -238,6 +238,7 @@ void create_all_devfreq_devices(void)
>
> callback fn = &devfreq_dev_callback;
> process_directory(p.c_str(), fn);
> + closedir(dir);
> }
>
> void initialize_devfreq(void)
> diff --git a/src/perf/perf_bundle.cpp b/src/perf/perf_bundle.cpp
> index b0e982b..cf1ae11 100644
> --- a/src/perf/perf_bundle.cpp
> +++ b/src/perf/perf_bundle.cpp
> @@ -142,8 +142,10 @@ static void parse_event_format(const char *event_name)
>
> buf = read_file(file);
> free(file);
> - if (!buf)
> + if (!buf) {
> + free(name);
> return;
> + }
>
> pevent_parse_event(perf_event::pevent, buf, strlen(buf), sys);
> free(name);
> diff --git a/src/tuning/bluetooth.cpp b/src/tuning/bluetooth.cpp
> index e0bdf12..5100a8a 100644
> --- a/src/tuning/bluetooth.cpp
> +++ b/src/tuning/bluetooth.cpp
> @@ -144,8 +144,10 @@ int bt_tunable::good_bad(void)
> if (file) {
> char line[2048];
> /* first line is standard header */
> - if (fgets(line, 2047, file) == NULL)
> + if (fgets(line, 2047, file) == NULL) {
> + pclose(file);
> goto out;
> + }
> memset(line, 0, 2048);
> if (fgets(line, 2047, file) == NULL) {
> result = last_check_result = TUNE_GOOD;
>
>
>
> --
> Thanks,
> -Meraj
>
> ___
> linaro-dev mailing list
> linaro-dev@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-dev
>
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH v2] pm-qa/cpuhotplug: Run cpuhotplug on all cpus
Hi Amit,
On 30 June 2014 22:43, Amit Kucheria wrote:
> Some nits below:
>
> On Tue, Jul 1, 2014 at 5:35 AM, Lisa Nguyen wrote:
>> Run the cpuhotplug test suite on all cpus including cpu0. If we wish
>> to skip cpu0, pass the parameter, hotplug_allow_cpu0=1, to make check
>> like in these examples:
>
> Umm, this description is the opposite of what the default code does.
>
> cpuhotplug testsuite does *not* run on cpu0 by default. If we wish to
> *allow* cpu0, we set hotplug_allow_cpu0 to 1.
>
Honest mistake. Somehow I was thinking hotplug_allow_cpu0=0 as true,
=1 as false. You're correct: cpuhotplug does not run on cpu0 by
default unless we pass in the parameter.
>> sudo make -C cpuhotplug hotplug_allow_cpu0=1 check
>> sudo make hotplug_allow_cpu0=1 check
>>
>> Changes from v1 to v2:
>> - Rename the parameter from hotplug_cpu_start to hotplug_allow_cpu0
>> - Add is_cpu0_allowed() function to check if we want to start running
>> test scripts from cpu0..cpuN
>>
>> Signed-off-by: Lisa Nguyen
>> ---
>> Makefile|3 ++-
>> cpuhotplug/Makefile |2 +-
>> cpuhotplug/cpuhotplug_02.sh |3 +--
>> cpuhotplug/cpuhotplug_03.sh |2 +-
>> cpuhotplug/cpuhotplug_04.sh |2 +-
>> cpuhotplug/cpuhotplug_05.sh |4 ++--
>> cpuhotplug/cpuhotplug_06.sh |4 ++--
>> cpuhotplug/cpuhotplug_07.sh |2 +-
>> cpuhotplug/cpuhotplug_08.sh |7 ++-
>> include/functions.sh| 12
>> 10 files changed, 29 insertions(+), 12 deletions(-)
>>
>> diff --git a/Makefile b/Makefile
>> index 731619d..9f26fd3 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -21,6 +21,7 @@
>> # Torez Smith (IBM Corporation)
>> # - initial API and implementation
>> #
>> +hotplug_allow_cpu0?=0
>>
>> all:
>> @(cd utils; $(MAKE))
>> @@ -28,7 +29,7 @@ all:
>> check:
>> @(cd utils; $(MAKE) check)
>> @(cd cpufreq; $(MAKE) check)
>> - @(cd cpuhotplug; $(MAKE) check)
>> + @(cd cpuhotplug; $(MAKE) hotplug_allow_cpu0=${hotplug_allow_cpu0}
>> check)
>> @(cd cpuidle; $(MAKE) check)
>> # @(cd suspend; $(MAKE) check)
>> @(cd thermal; $(MAKE) check)
>> diff --git a/cpuhotplug/Makefile b/cpuhotplug/Makefile
>> index df0b8f4..6ee600d 100644
>> --- a/cpuhotplug/Makefile
>> +++ b/cpuhotplug/Makefile
>> @@ -21,5 +21,5 @@
>> # Daniel Lezcano (IBM Corporation)
>> # - initial API and implementation
>> #
>> -
>> +export hotplug_allow_cpu0?=0
>> include ../Test.mk
>> diff --git a/cpuhotplug/cpuhotplug_02.sh b/cpuhotplug/cpuhotplug_02.sh
>> index 3157307..58e1f02 100755
>> --- a/cpuhotplug/cpuhotplug_02.sh
>> +++ b/cpuhotplug/cpuhotplug_02.sh
>> @@ -24,7 +24,6 @@
>> #
>>
>> # URL :
>> https://wiki.linaro.org/WorkingGroups/PowerManagement/Resources/TestSuite/PmQaSpecification#cpuhotplug_02
>> -
>> source ../include/functions.sh
>>
>> check_state() {
>> @@ -34,7 +33,7 @@ check_state() {
>> shift 1
>>
>> if [ "$cpu" == "cpu0" ]; then
>> - return 0
>> + is_cpu0_allowed $cpu $hotplug_allow_cpu0 || return 0
>
> Since is_cpu0_allowed has cpu0 in its name, why pass it $cpu as a
> parameter? You are already checking for cpu0 in the if statement. This
> can be simplified as
>
> if [ "$cpu" == "cpu0" ]; then
> is_cpu0_allowed $hotplug_allow_cpu0 || return 0
>
Yes, I agree with that.
>> fi
>>
>> set_offline $cpu
>> diff --git a/cpuhotplug/cpuhotplug_03.sh b/cpuhotplug/cpuhotplug_03.sh
>> index 13a0ce9..b29ff8e 100755
>> --- a/cpuhotplug/cpuhotplug_03.sh
>> +++ b/cpuhotplug/cpuhotplug_03.sh
>> @@ -34,7 +34,7 @@ check_affinity_fails() {
>> local ret=
>>
>> if [ "$cpu" == "cpu0" ]; then
>> - return 0
>> + is_cpu0_allowed $cpu $hotplug_allow_cpu0 || return 0
>> fi
>>
>> set_offline $cpu
>> diff --git a/cpuhotplug/cpuhotplug_04.sh b/cpuhotplug/cpuhotplug_04.sh
>> index 394a512..543853f 100755
>> --- a/cpuhotplug/cpuhotplug_04.sh
>> +++ b/cpuhotplug/cpuhotplug_04.sh
>> @@ -37,7 +37,7 @@ check_task_migrate() {
>> local ret=
>>
>> if [ "$cpu" == "cpu0" ]; then
>> - return 0
>> +is_cpu0_allowed $cpu $hotplug_allow_cpu0 || return 0
>> fi
>>
>> taskset 0x$cpumask $CPUBURN $cpu &
>> diff --git a/cpuhotplug/cpuhotplug_05.sh b/cpuhotplug/cpuhotplug_05.sh
>> index a8eb312..2f33a0c 100755
>> --- a/cpuhotplug/cpuhotplug_05.sh
>> +++ b/cpuhotplug/cpuhotplug_05.sh
>> @@ -33,9 +33,9 @@ check_procinfo() {
>> local ret=
>>
>> if [ "$cpu" == "cpu0" ]; then
>> - return 0
>> +is_cpu0_allowed $cpu $hotplug_allow_cpu0 || return 0
>> fi
>> -
>> +
>> set_offline $cpu
>>
>> TAB=$(printf "\t"); grep "processor$TAB: $cpuid" /proc/cpuinfo
>> diff --git a/cpuhotplug/cpuhotplug_06.sh b/cpuhotplug/cpuhotplug_06.sh
>> index 347906d..528aeca 100755
>> --- a/cpuhotplug/cpuhotplug_06.sh
>> +++ b/cpuhotplug/cpuhotplug_06.sh
>> @@ -33,9 +33,9 @@ check_procinfo() {
>> local ret=
>>
>> if [ "$cpu" == "cpu0" ]; then
>>
