Re: [PATCH idlestat] Add -o option to save output report to a file
On 21 August 2014 09:43, Daniel Lezcano wrote: > On 08/19/2014 09:22 AM, pi-cheng.chen wrote: >> >> Currently the serial terminal connected to the boards running idlestat are >> restricted to be at least 80 characters wide to output the report. >> Otherwise >> idlestat quits with message "The terminal must be at least 80 columns >> wide". >> >> Fix it by adding a "-o" option to save report output to a file. > > > Yes, but please do it in the unix way. > > 1. open file > 2. close stdout > 3. dup[2] opened file to stdout (fd 1) > 4. close opened file > > So no need to change all the code around. > > Thanks > -- Daniel Thanks for your comments. I'll do it in next version. Pi-Cheng ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH v2] idlestat: Allocate struct cpufreq_pstate dynamically
On 10 September 2014 17:15, Daniel Lezcano wrote: > On 09/10/2014 09:33 AM, pi-cheng.chen wrote: >> >> When using intel_pstate driver, "scaling_available_freqs" attr is not >> exported to sysfs. It causes assertion of idlestat due to memory of >> struct cpufreq_pstate was not allocated. >> >> Allocate struct cpufreq_pstate dynamically when getting frequency >> information from trace file instead of parsing available frequencies >> from sysfs >> >> Changes v1 to v2: >> Sort the cpufreq_pstate list when parsing events >> >> Signed-off-by: Pi-Cheng Chen > > > Tested on intel_pstate and legacy cpufreq driver. > > In complement of this feature, I think the cpus cpufreq must be initialized > with the 'cpuinfo_cur_freq' with the start time initialized to the beginning > of the acquisition. If a cpu is in a specific freq without any changes > (understand: ftrace transitions which trigger the p-state creation), we will > have a long freq residency (the initial one). > > Today we don't have this, so the cpus without freq changes are not > displayed. > > At least the minimal number of p-state should be '1' not '0' (except if > cpufreq is disabled). > > -- Daniel How about to create some "fake records" manually in the trace file? Get p-states of all CPUs first before starting tracing and write them as fake records of p-state transitions before storing the traces from sysfs to trace file. So that the report will be consistent with that generated by import mode. Pi-Cheng ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH v2] idlestat: Allocate struct cpufreq_pstate dynamically
On 10 September 2014 21:17, Daniel Lezcano wrote: > On 09/10/2014 03:08 PM, Pi-Cheng Chen wrote: >> >> On 10 September 2014 17:15, Daniel Lezcano >> wrote: >>> >>> On 09/10/2014 09:33 AM, pi-cheng.chen wrote: >>>> >>>> >>>> When using intel_pstate driver, "scaling_available_freqs" attr is not >>>> exported to sysfs. It causes assertion of idlestat due to memory of >>>> struct cpufreq_pstate was not allocated. >>>> >>>> Allocate struct cpufreq_pstate dynamically when getting frequency >>>> information from trace file instead of parsing available frequencies >>>> from sysfs >>>> >>>> Changes v1 to v2: >>>> Sort the cpufreq_pstate list when parsing events >>>> >>>> Signed-off-by: Pi-Cheng Chen >>> >>> >>> >>> Tested on intel_pstate and legacy cpufreq driver. >>> >>> In complement of this feature, I think the cpus cpufreq must be >>> initialized >>> with the 'cpuinfo_cur_freq' with the start time initialized to the >>> beginning >>> of the acquisition. If a cpu is in a specific freq without any changes >>> (understand: ftrace transitions which trigger the p-state creation), we >>> will >>> have a long freq residency (the initial one). >>> >>> Today we don't have this, so the cpus without freq changes are not >>> displayed. >>> >>> At least the minimal number of p-state should be '1' not '0' (except if >>> cpufreq is disabled). >>> >>> -- Daniel >> >> >> How about to create some "fake records" manually in the trace file? >> Get p-states of all CPUs first before starting tracing and write them >> as fake records of p-state transitions before storing the traces from >> sysfs to trace file. So that the report will be consistent with that >> generated by import mode. > > > Well I think initializing directly the p-state structure with one element at > init time will be simpler and cleaner. For the C-state it is not possible > because we don't know idle state the cpu is, so we have to force a wakeup on > all cpus but for the p-state this info is available in the sysfs. > Please let me clarify it. First we read the current P-state for all CPUs before starting tracing and initialize the P-state structure with one element and initial P-state we read. So that the P-states of CPUs with no freq transition can be displayed correctly. Is my understanding correct? By the way, if doing so, the P-states statistics table reported by idlestat running "--trace" mode might be different from the table report by idlestat running "--import" mode with some other trace file captured before? > > > -- > <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs > > Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | > <http://twitter.com/#!/linaroorg> Twitter | > <http://www.linaro.org/linaro-blog/> Blog > ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH v3] idlestat: Allocate struct cpufreq_pstate dynamically
On 21 October 2014 21:49, Daniel Lezcano wrote:
> On 10/21/2014 03:47 PM, Tuukka Tikkanen wrote:
>>
>> On Tue, Oct 21, 2014 at 3:42 PM, Daniel Lezcano
>> wrote:
>>>
>>> On 10/04/2014 06:33 AM, pi-cheng.chen wrote:
>>>>
>>>>
>>>> Initialize struct cpufreq_pstates with initial P-state of CPUs and
>>>> allocate
>>>> struct cpufreq_pstate dynamically when parsing trace file to solve the
>>>> issue
>>>> caused by missing "scaling_avaialable_freqs" attr when using
>>>> intel_pstate
>>>> driver.
>>>>
>>>> Changes v2 to v3:
>>>> Initialize struct cpufreq_pstates with initial P-state of all CPUs and
>>>> beginning timestamp before trace acquisition and close all P-states with
>>>> ending timestamp
>>>>
>>>> hanges v1 to v2:
>>>> Sort the cpufreq_pstate list when parsing events
>>>>
>>>> Signed-off-by: Pi-Cheng Chen
>>>
>>>
>>>
>>> At the first glance it looks ok for me.
>>>
>>> Thanks
>>>-- Daniel
>>
>>
>> Good enough for a reviewed-by with your name?
>
>
> Good enough for an Acked-by: Daniel Lezcano
Thanks for reviewing.
Pi-Cheng
>
>
>
>> Tuukka
>>
>>>
>>>
>>>> ---
>>>>idlestat.c | 259
>>>> -
>>>>idlestat.h | 7 ++
>>>>trace.h| 1 +
>>>>3 files changed, 195 insertions(+), 72 deletions(-)
>>>>
>>>> diff --git a/idlestat.c b/idlestat.c
>>>> index da615cb..8230067 100644
>>>> --- a/idlestat.c
>>>> +++ b/idlestat.c
>>>> @@ -536,6 +536,121 @@ static struct cpuidle_cstates
>>>> *build_cstate_info(int
>>>> nrcpus)
>>>> return cstates;
>>>>}
>>>>
>>>> +#define TRACE_STAT_FORMAT "%*[^:]:%lf"
>>>> +
>>>> +static double get_trace_ts(void)
>>>> +{
>>>> + FILE *f;
>>>> + double ts;
>>>> +
>>>> + f = fopen(TRACE_STAT_FILE, "r");
>>>> + if (!f)
>>>> + return -1;
>>>> +
>>>> + while (fgets(buffer, BUFSIZE, f)) {
>>>> + if (!strstr(buffer, "now ts"))
>>>> + continue;
>>>> + if (!sscanf(buffer, TRACE_STAT_FORMAT, &ts))
>>>> + ts = -1;
>>>> + break;
>>>> + }
>>>> + fclose(f);
>>>> +
>>>> + return ts;
>>>> +}
>>>> +
>>>> +static void release_init_pstates(struct init_pstates *init)
>>>> +{
>>>> + free(init->freqs);
>>>> + free(init);
>>>> +}
>>>> +
>>>> +static struct init_pstates *build_init_pstates(void)
>>>> +{
>>>> + struct init_pstates *init;
>>>> + int nr_cpus, cpu;
>>>> + unsigned int *freqs;
>>>> +
>>>> + nr_cpus = sysconf(_SC_NPROCESSORS_CONF);
>>>> + if (nr_cpus < 0)
>>>> + return NULL;
>>>> +
>>>> + init = malloc(sizeof(*init));
>>>> + if (!init)
>>>> + return NULL;
>>>> +
>>>> + freqs = calloc(nr_cpus, sizeof(*freqs));
>>>> + if (!freqs) {
>>>> + free(init);
>>>> + return NULL;
>>>> + }
>>>> + memset(freqs, 0, sizeof(*freqs) * nr_cpus);
>>>> +
>>>> + for (cpu = 0; cpu < nr_cpus; cpu++) {
>>>> + char *fpath;
>>>> + unsigned int *freq = &(freqs[cpu]);
>>>> +
>>>> + if (asprintf(&fpath, CPUFREQ_CURFREQ_PATH_FORMAT, cpu) <
>>>> 0) {
>>>> + release_init_pstates(init);
>>>> + return NULL;
>>>> + }
>>>> + if (read_int(fpath, (int *)freq))
>>>> + *freq = 0;
>>>> + free(fpath);
>>>> + }
>>>> + init->nr_cpus = nr_cpus;
>>>> + init->freqs = freqs;
Re: [RFC] rt-app: add script to generate small task workloads
On 6 November 2014 16:14, Vincent Guittot wrote:
> On 22 October 2014 03:39, pi-cheng.chen wrote:
>> Some examples to use the script to generate small tasks workloads:
>> 1. to generate a workload with 4 threads, 10% load and 30ms period
>> $ ./small_task_gen -t 4 -l 10 -p 3
>
> The period unit seems to be 10ms which is quite long, please use a
> more fine grained value; at least ms or even us
>
>>
>> 2. to generate a workload with 3 threads, 15% load and random tick-aligned
>>period
>> $ ./small_task_gen -t 3 -l 15 -r --aligned
>>
>> 3. to generate a workload with 5 threads, 20% load and random
>> non-tick-aligned
>>period
>> $ ./small_task_gen -t 5 -l 20 -r --unaligned
>
> What about a defined but unaligned period ? it doesn't seems to be possible
>
>> ---
>> doc/examples/small_task_gen | 117
>>
>> 1 file changed, 117 insertions(+)
>> create mode 100755 doc/examples/small_task_gen
>>
>> diff --git a/doc/examples/small_task_gen b/doc/examples/small_task_gen
>> new file mode 100755
>> index 000..ba74753
>> --- /dev/null
>> +++ b/doc/examples/small_task_gen
>> @@ -0,0 +1,117 @@
>> +#!/usr/bin/env python
>> +
>> +import sys
>> +import getopt
>> +import random
>> +
>> +outfile = "small_tasks.json"
>> +tasks = None
>> +loading = None
>> +period = None
>> +randomized = None
>> +aligned = None
>> +
>> +
>> +def usage():
>> +print "Usage:"
>> +print sys.argv[0] + " -t "
>> +print "\t\t -l (1 - 100)"
>> +print "\t\t -p "
>> +print "\t\t -r [--aligned | --unaligned] (randomize periods of tasks)"
>> +print "\t\t -o (default: small_tasks.json)"
>> +return
>> +
>> +
>> +def parse_options():
>> +global outfile
>> +global tasks
>> +global loading
>> +global period
>> +global randomized
>> +global aligned
>> +
>> +try:
>> +opts, args = getopt.getopt(sys.argv[1:], "t:l:o:p:rh",
>> + ["aligned", "unaligned"])
>> +except getopt.GetoptError:
>> +usage()
>> +sys.exit(2)
>> +
>> +for o, a in opts:
>> +if o == "-t":
>> +tasks = int(a)
>> +print "number of tasks: %d" % tasks
>> +if o == "-l":
>> +loading = int(a)
>> +print "task loading: %d" % loading
>> +if o == "-o":
>> +outfile = a;
>> +print "output workload JSON file: " + outfile
>> +if o == "-p":
>> +period = int(a)
>> +print "period of tasks: %d" % period
>> +if o == "-r":
>> +randomized = True
>> +print "randomized: %r" % randomized
>> +if o == "--aligned":
>> +if aligned == False:
>> +usage()
>> +sys.exit(2)
>> +aligned = True
>> +print "aligned: %r" % aligned
>> +if o in "--unaligned":
>> +if aligned == True:
>> +usage()
>> +sys.exit(2)
>> +aligned = False
>> +print "aligned: %r" % aligned
>> +if o == "-h":
>> +usage()
>> +sys.exit(2)
>> +
>> +if not period is None and not randomized is None:
>> +usage()
>> +sys.exit(2)
>> +
>> +if tasks is None or (period is None and randomized is None):
>> +usage()
>> +sys.exit(2)
>> +
>> +return
>> +
>> +
>> +def generate_workload():
>> +try:
>> +f = open(outfile, "w")
>> +except IOError:
>> +print "WARN: Unable to open " + infile
>> +sys.exit(2)
>> +
>> +f.write("{\n")
>> +f.write("\t\"tasks\" : {\n")
>> +
>> +for i in range(0, tasks):
>> +
>> +if randomized is None:
>> +period_ns = period * 1000
>> +else:
>> +period_ns = random.randrange(1, 6) * 1 # random
>> tick-aligned period from 10ms~50ms
>
> the [10:50]ms range for the random period is quite arbitrary, we
> should be able to specify a range for the random value
>
>> +if aligned == False: # add a period offset randomly from 1 of
>> [300, 500, 700]
>> +period_ns += (3000, 5000, 7000)[random.randrange(0, 3)]
>
> yous should not specify a restricted list of random offset
>
>> +
>> +run_ns = period_ns * loading / 100
>> +
>> +f.write("\t\t\"thread%d\" : {\n" % i)
>> +f.write("\t\t\t\"loop\" : -1,\n")
>> +f.write("\t\t\t\"run\" : %d,\n" % run_ns)
>> +f.write("\t\t\t\"timer\" : { \"ref\" : \"tick\", \"period\" : %d
>> },\n" % period_ns)
>
> you should use a different timer id for each task instead of sharing
> the tick timer between tasks
>
>> +f.write("\t\t},\n")
>> +
>> +f.write("\t},\n")
>> +f.write("}\n")
>
> As discussed, I still think that it's not straight forward to
> understand what the script is doing or to reuse it for another use
> case. But I haven't found a better way so far. IMO, the use of a model
> that is then modifed according
Re: [PATCH 2/3] rt-app: Add a basic json file for small task workload
On 19 November 2014 16:32, Vincent Guittot wrote: > On 12 November 2014 02:49, pi-cheng.chen wrote: >> A JSON file to describe a workload with several 10% loading small tasks. > Hi Pi-Cheng, > > Your example6.json is quite similar to example 2. Please add an > "instance" : 1, in example2.json instead of creating a new example > > Regards, > Vincent Hi Vincent, Ok. I'll do it. Thanks for reviewing. Best Regards, Pi-Cheng ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH] idlestat: Replace asserts with proper error handlings
On 26 November 2014 at 22:28, Tuukka Tikkanen
wrote:
> Hi,
>
> committing amended (see below).
>
Hi Tuukka,
thanks for reviewing and fixing.
Pi-Cheng
> On Wed, Nov 26, 2014 at 10:15 AM, pi-cheng.chen
> wrote:
>> Replace asserts with displaying error messages and/or terminating program in
>> a
>> controlled manner. Also refine some error handling paths.
>
> This description doesn't really tell what is wrong in the original
> code. Replaced the commit message with the card description:
>
> "Some parts of the code use asserts as a way to terminate execution in
> case input data processing fails. This leaves the user in the black
> regarding the reason why the tool terminated (and possibly dumped a
> core file).
> Replace the abnormal termination with displaying error message and
> terminating the application in a controlled fashion."
>
>>
>> Signed-off-by: Pi-Cheng Chen
>> ---
>> energy_model.c | 3 ---
>> idlestat.c | 62
>> +++---
>> topology.c | 12 ++--
>> 3 files changed, 52 insertions(+), 25 deletions(-)
>>
>> diff --git a/energy_model.c b/energy_model.c
>> index 87ea3a8..b564ec6 100644
>> --- a/energy_model.c
>> +++ b/energy_model.c
>> @@ -3,7 +3,6 @@
>> #include
>> #include
>> #include
>> -#include
>>
>> #include "energy_model.h"
>> #include "idlestat.h"
>> @@ -92,8 +91,6 @@ int parse_energy_model(struct program_options *options)
>> char *path = options->energy_model_filename;
>> int ret;
>>
>> - assert(path != NULL);
>> -
>
> This is a valid assertation (the function should never be called with
> a NULL energy model filename and the caller should enforce it and this
> condition does not depend directly on user input). Removed removing
> it.
>
>> f = fopen(path, "r");
>> if (!f) {
>> if (errno == ENOENT) {
>> diff --git a/idlestat.c b/idlestat.c
>> index eff04e5..5bd2a14 100644
>> --- a/idlestat.c
>> +++ b/idlestat.c
>> @@ -40,7 +40,6 @@
>> #include
>> #include
>> #include
>> -#include
>>
>> #include "idlestat.h"
>> #include "utils.h"
>> @@ -647,7 +646,8 @@ static int alloc_pstate(struct cpufreq_pstates *pstates,
>> unsigned int freq)
>>
>> tmp = realloc(pstate, sizeof(*pstate) * (nrfreq + 1));
>> if (!tmp) {
>> - perror("realloc pstate");
>> + fprintf(stderr, "%s: Failed ot realloc memory for pstate. "
>> + "Exiting.\n", __func__);
>> return -1;
>
> Massaged the message and replaced return with exit(1), as we don't
> gain anything by propagating this error.
>
>> }
>> pstate = tmp;
>> @@ -864,7 +864,8 @@ static void cpu_change_pstate(struct cpuidle_datas
>> *datas, int cpu,
>> next = freq_to_pstate_index(ps, freq);
>> if (next < 0)
>> next = alloc_pstate(ps, freq);
>> - assert(next >= 0);
>> + if (next < 0)
>> + exit(1);
>
> Especially given the above change, this assert is valid and should not
> be removed.
>
>>
>> switch (cur) {
>> case 1:
>> @@ -1090,15 +1091,24 @@ static int get_wakeup_irq(struct cpuidle_datas
>> *datas, char *buffer, int count)
>> char irqname[NAMELEN+1];
>>
>> if (strstr(buffer, "irq_handler_entry")) {
>> - assert(sscanf(buffer, TRACE_IRQ_FORMAT, &cpu, &irqid,
>> - irqname) == 3);
>> + if (sscanf(buffer, TRACE_IRQ_FORMAT, &cpu, &irqid,
>> + irqname) != 3) {
>> + fprintf(stderr, "warning: Unrecognized"
>
> Added missing space.
>
>> + "irq_handler_entry record. Skip
>> it.\n");
>> + return -1;
>> + }
>>
>> store_irq(cpu, irqid, irqname, datas);
>> return 0;
>> }
>>
>> if (strstr(buffer, "ipi_entry")) {
>> - assert(sscanf(buffer, TRACE_IPIIRQ_FORMAT, &cpu, irqname) ==
>> 2);
>> + if (sscanf(buffer, TRACE_IPIIRQ_FORMAT, &cpu, irqname) != 2)
>> {
>> +
Re: [PATCH] Add IO-bounded and memory-bounded events
On 2 December 2014 at 15:09, Viresh Kumar wrote:
> On 2 December 2014 at 12:34, pi-cheng.chen wrote:
>> Add 2 new kind of event for running a memory or a io bounded load.
>> "mem" name for a load is memory bounded, and "iorun" name for a load is io
>> bounded. The default file to be written to create the load is /dev/null and
>> the device/file could be specified with "io_device" key in "global" section.
>>
>> E.g.
>> "tasks" : {
>> "thread0" :
>> { "sleep" : 1000, "run" : 100, "mem" : 1000, "sleep" 1, "iorun" : 1000 }
>> },
>> "global" : { "io_device" : "/dev/ttyS0" }
>>
>> Signed-off-by: pi-cheng.chen
>> ---
>> src/rt-app.c | 74
>> +++
>> src/rt-app.h | 2 ++
>> src/rt-app_parse_config.c | 23 +++
>> src/rt-app_types.h| 4 +++
>> 4 files changed, 103 insertions(+)
>
> Please mention somewhere in subject line to which repository
> this patch belongs to.
>
> By default [PATCH] is considered to be for the kernel.
> This is how we follow it for uboot: [U-Boot] [PATCH]
>
> Similar should be done for these utilities as well..
Thanks for pointing out my error.
Just forgot to specify the project in subject.
Will resend it again.
Sorry for the spam.
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH] rt-app: Add IO-bounded and memory-bounded events
On 2 December 2014 at 19:02, Ivan T. Ivanov wrote:
>
> On Tue, 2014-12-02 at 15:21 +0800, pi-cheng.chen wrote:
>> Add 2 new kind of event for running a memory or a io bounded load.
>> "mem" name for a load is memory bounded, and "iorun" name for a load is io
>> bounded. The default file to be written to create the load is /dev/null and
>> the device/file could be specified with "io_device" key in "global" section.
>>
>> E.g.
>> "tasks" : {
>> "thread0" :
>> { "sleep" : 1000, "run" : 100, "mem" : 1000, "sleep" 1, "iorun" : 1000 }
>> },
>> "global" : { "io_device" : "/dev/ttyS0" }
>>
>
> Wouldn't be better if we can specify size of the accessed
> memory region, instead of iteration count? We can use phases
> to create arbitrary number of iterations for memory access.
>
> Thanks,
> Ivan
Hi Ivan,
I think the value of "mem" key we specify here is taken as the size of
the memory
to be memcpy() in the patch, not as iteration count.
Did I misunderstand what you mean?
Could you please be clearer?
Thanks.
Pi-Cheng
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH] rt-app: Add IO-bounded and memory-bounded events
On 2 December 2014 at 19:04, Vincent Guittot wrote:
> On 2 December 2014 at 08:21, pi-cheng.chen wrote:
>> Add 2 new kind of event for running a memory or a io bounded load.
>> "mem" name for a load is memory bounded, and "iorun" name for a load is io
>> bounded. The default file to be written to create the load is /dev/null and
>> the device/file could be specified with "io_device" key in "global" section.
>
> Hi pi-cheng
>
> What's the unit of the mem and iorun ?
> As an example, "mem" : 2000 will do a copy of 2000 Bytes ? KBytes ?
>
Hi Vincent,
Thanks for reviewing.
The unit of mem and iorun here is the size to be copied/written in byte.
As an example, "mem" : 2000 will do a 2000 bytes copy.
>>
>> E.g.
>> "tasks" : {
>> "thread0" :
>> { "sleep" : 1000, "run" : 100, "mem" : 1000, "sleep" 1, "iorun" : 1000 }
>> },
>> "global" : { "io_device" : "/dev/ttyS0" }
>>
>> Signed-off-by: pi-cheng.chen
>> ---
>> src/rt-app.c | 74
>> +++
>> src/rt-app.h | 2 ++
>> src/rt-app_parse_config.c | 23 +++
>> src/rt-app_types.h| 4 +++
>> 4 files changed, 103 insertions(+)
>>
>> diff --git a/src/rt-app.c b/src/rt-app.c
>> index 3cd601d..13f72e4 100644
>> --- a/src/rt-app.c
>> +++ b/src/rt-app.c
>> @@ -33,6 +33,8 @@ static volatile int continue_running;
>> static pthread_t *threads;
>> static int nthreads;
>> static int p_load;
>> +static char *buffer[2];
>> +static int io_fd;
>> rtapp_options_t opts;
>>
>> static ftrace_data_t ft_data = {
>> @@ -110,6 +112,45 @@ static inline loadwait(unsigned long exec)
>> return load_count;
>> }
>>
>> +static void ioload(unsigned long count)
>> +{
>> + ssize_t ret;
>> + char *buf = buffer[0];
>> +
>> + while (count != 0) {
>> + ret = write(io_fd, buffer, count);
>
> count can be higher than buffer size
>
> so you have defined a MEM_BUFFER_SIZE for mem transfer but not for iorun ?
>
I forgot to do such check for iorun. Will do it.
>> + if (ret == -1) {
>> + perror("write");
>> + return;
>> + }
>> + count -= ret;
>> + buf += ret;
>> + }
>> +}
>> +
>> +static void memload(unsigned long count)
>> +{
>> + static unsigned long current = 0;
>> +
>> + while (count > 0) {
>> + unsigned long size;
>> +
>> + if (count > MEM_BUFFER_SIZE)
>> + size = MEM_BUFFER_SIZE;
>> + else
>> + size = count;
>> +
>> + if (size > (MEM_BUFFER_SIZE - current))
>> + size = MEM_BUFFER_SIZE - current;
>> +
>> + memcpy(buffer[0], buffer[1], size);
>
> I wonder if a memset would be better
>
Sure. Will do it.
>> + count -= size;
>> + current += size;
>> + if (current >= MEM_BUFFER_SIZE)
>> + current -= MEM_BUFFER_SIZE;
>
> The size of MEM_BUFFER_SIZE will deeply impact how we will trash the
> cache and how the memory access (up to the ddr) will be the
> bottleneck. This should be at least configurable in the global section
>
I was thinking is there a generic way the get the cache size of the CPU,
but I failed to do so. So I just set it as a size bigger than the size of cache
on my laptop, which is 3MB. Yes I should make it configurable in the
global section.
>> + }
>> +}
>> +
>> static int run_event(event_data_t *event, int dry_run,
>> unsigned long *perf, unsigned long *duration,
>> rtapp_resource_t *resources)
>> {
>> @@ -196,6 +237,18 @@ static int run_event(event_data_t *event, int dry_run,
>> pthread_mutex_unlock(&(ddata->res.mtx.obj));
>> break;
>> }
>> + case rtapp_mem:
>> + {
>> + log_debug("mem %d", event->count);
>> + memload(event->count);
>> + }
>> + break;
>> + case rtapp_iorun:
>> + {
>> + log_debug("iorun %d", event->count);
>> + ioload(event->count);
>> + }
>> + break;
>> }
>>
>> return lock;
>> @@ -488,6 +541,22 @@ int main(int argc, char* argv[])
>>
>> parse_command_line(argc, argv, &opts);
>>
>> + /* allocate memory buffers for memory-bound and IO-bound busy loops
>> */
>> + buffer[0] = malloc(MEM_BUFFER_SIZE);
>> + buffer[1] = malloc(MEM_BUFFER_SIZE);
>
> You use the same buffer for all threads
>
I thought since we don't care about the content of the memory buffer,
we could use
the same buffer for all threads. But it just comes to my mind that the
cache effect for
each thread might be unpredictable in this case. I should allocate
different buffers for
different threads in the use case.
BTW, shall I allocate 2 buffers for each thread, one for
Re: [PATCH] rt-app: Add IO-bounded and memory-bounded events
On 3 December 2014 at 17:24, Ivan T. Ivanov wrote:
>
> On Wed, 2014-12-03 at 15:08 +0800, Pi-Cheng Chen wrote:
>> On 2 December 2014 at 19:04, Vincent Guittot guit...@linaro.org> wrote:
>> > On 2 December 2014 at 08:21, pi-cheng.chen c...@linaro.org> wrote:
>> > > Add 2 new kind of event for running a memory or a io bounded load.
>> > > "mem" name for a load is memory bounded, and "iorun" name for a load is
>> > > io
>> > > bounded. The default file to be written to create the load is /dev/null
>> > > and
>> > > the device/file could be specified with "io_device" key in "global"
>> > > section.
>> >
>> > Hi pi-cheng
>> >
>> > What's the unit of the mem and iorun ?
>> > As an example, "mem" : 2000 will do a copy of 2000 Bytes ? KBytes ?
>> >
>> Hi Vincent,
>>
>> Thanks for reviewing.
>> The unit of mem and iorun here is the size to be copied/written in byte.
>> As an example, "mem" : 2000 will do a 2000 bytes copy.
>>
>> > > E.g.
>> > > "tasks" : {
>> > > "thread0" :
>> > > { "sleep" : 1000, "run" : 100, "mem" : 1000, "sleep" 1, "iorun" :
>> > > 1000 }
>> > > },
>> > > "global" : { "io_device" : "/dev/ttyS0" }
>> > >
>> > > Signed-off-by: pi-cheng.chen c...@linaro.org>
>> > > ---
>> > > src/rt-app.c | 74
>> > > +++
>> > > src/rt-app.h | 2 ++
>> > > src/rt-app_parse_config.c | 23 +++
>> > > src/rt-app_types.h| 4 +++
>> > > 4 files changed, 103 insertions(+)
>> > >
>> > > diff --git a/src/rt-app.c b/src/rt-app.c
>> > > index 3cd601d..13f72e4 100644
>> > > --- a/src/rt-app.c
>> > > +++ b/src/rt-app.c
>> > > @@ -33,6 +33,8 @@ static volatile int continue_running;
>> > > static pthread_t *threads;
>> > > static int nthreads;
>> > > static int p_load;
>> > > +static char *buffer[2];
>> > > +static int io_fd;
>> > > rtapp_options_t opts;
>> > >
>> > > static ftrace_data_t ft_data = {
>> > > @@ -110,6 +112,45 @@ static inline loadwait(unsigned long exec)
>> > > return load_count;
>> > > }
>> > >
>> > > +static void ioload(unsigned long count)
>> > > +{
>> > > + ssize_t ret;
>> > > + char *buf = buffer[0];
>> > > +
>> > > + while (count != 0) {
>> > > + ret = write(io_fd, buffer, count);
>> >
>> > count can be higher than buffer size
>> >
>> > so you have defined a MEM_BUFFER_SIZE for mem transfer but not for iorun ?
>> >
>>
>> I forgot to do such check for iorun. Will do it.
>>
>> > > + if (ret == -1) {
>> > > + perror("write");
>> > > + return;
>> > > + }
>> > > + count -= ret;
>> > > + buf += ret;
>> > > + }
>> > > +}
>> > > +
>> > > +static void memload(unsigned long count)
>> > > +{
>> > > + static unsigned long current = 0;
>> > > +
>> > > + while (count > 0) {
>> > > + unsigned long size;
>> > > +
>> > > + if (count > MEM_BUFFER_SIZE)
>> > > + size = MEM_BUFFER_SIZE;
>> > > + else
>> > > + size = count;
>> > > +
>> > > + if (size > (MEM_BUFFER_SIZE - current))
>> > > + size = MEM_BUFFER_SIZE - current;
>> > > +
>> > > + memcpy(buffer[0], buffer[1], size);
>> >
>> > I wonder if a memset would be better
>> >
>>
>> Sure. Will do it.
>>
>> > > + count -= size;
>> > > + current += size;
>> > > + if (current >= MEM_BUFFER_SIZE)
>> > > + current -= MEM_BUFFER_SIZE;
>> >
>> > The size of MEM_BUFFER_SIZE will de
Re: [PATCH v2 1/2] rt-app: Add a script to tune parameters in json file
On Thu, Mar 5, 2015 at 7:20 PM, Amit Kucheria wrote: > On Thu, Mar 5, 2015 at 3:49 PM, Vincent Guittot > wrote: >> On 19 November 2014 at 14:14, pi-cheng.chen wrote: >>> + >>> +if __name__ == '__main__': >>> +tmp = 'tmp.json' >> >> your tmp file should be a bit more unique than tmp.json. I can imagine >> that the user of tune_json.py already has a tmp.json in the dir which >> will be overwritten by the script. >> You could add something like the current time stamp in the temporary >> file name to reduce the probability to overwrite a user's file > > Or actually use the python standard library functions to generate > unique temporary files: > https://docs.python.org/2/library/tempfile.html > > If you care about referencing the file externally, NamedTemporaryFile > might be the right method to use. Hi Vincent & Amit, Thanks for reviewing. I will fix the temporary file naming. Best Regards, Pi-Cheng ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
