Mirror of the lttng-dev mailing list
 help / color / mirror / Atom feed
* [ltt-dev] TODO list before releasing LTTng-buffering to LKML]
       [not found] <48F733E6.9010007@cn.fujitsu.com>
@ 2008-10-17  3:56 ` Zhaolei
  2008-10-17  3:56   ` Zhaolei
                     ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Zhaolei @ 2008-10-17  3:56 UTC (permalink / raw)


>> - switch ltt-control.ko (currently over netlink) to debugfs
>> - switch marker list (and marker activation (currently in /proc/ltt) to debugfs
>>   Marker directory. One file per marker.
>> - Create a ltt-ascii.ko kernel module which merge-sorts the buffers and exports
>>   them to userspace through a debugfs file.
>>
> 
> Hi, Mathieu,
> 
> We are interested with the above TODOs, and we are planning to implement them.
> 
Hi Mathieu,

I finished a simple ltt-control.ko based on debugfs.

With this module, we can control ltt by following command now:
Start trace1 channel:
echo "0 trace1" > /sys/kernel/debug/ltt_control/control
Stop trace1 channel:
echo "1 trace1" > /sys/kernel/debug/ltt_control/control
Create trace1 channel:
echo "2 trace1 relay 0 0 0 0 0 0 0" > /sys/kernel/debug/ltt_control/control
Destroy trace1 channel:
echo "3 trace1" > /sys/kernel/debug/ltt_control/control

I tested it on i386 linux 2.6.27 with lttng.

User-mode lttctl also needs some modification to support this kind of control,
but at first, I'd like to hear some advice on this module.

Thanks!
Zhaolei
> Thanks, Lai
> 

>> If some of you are interested to work on any of these, you are welcome.
>> I'll be glad to post more details about these items upon request.
>>
>> Mathieu
>>
> 
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: Makefile
URL: <http://lists.casi.polymtl.ca/pipermail/lttng-dev/attachments/20081017/6261648f/attachment-0001.asc>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: ltt_control.c
URL: <http://lists.casi.polymtl.ca/pipermail/lttng-dev/attachments/20081017/6261648f/attachment-0003.txt>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [ltt-dev] TODO list before releasing LTTng-buffering to LKML]
  2008-10-17  3:56 ` [ltt-dev] TODO list before releasing LTTng-buffering to LKML] Zhaolei
@ 2008-10-17  3:56   ` Zhaolei
  2008-10-17  3:56   ` Zhaolei
       [not found]   ` <20081017042045.GB5067@Krystal>
  2 siblings, 0 replies; 8+ messages in thread
From: Zhaolei @ 2008-10-17  3:56 UTC (permalink / raw)


>> - switch ltt-control.ko (currently over netlink) to debugfs
>> - switch marker list (and marker activation (currently in /proc/ltt) to debugfs
>>   Marker directory. One file per marker.
>> - Create a ltt-ascii.ko kernel module which merge-sorts the buffers and exports
>>   them to userspace through a debugfs file.
>>
> 
> Hi, Mathieu,
> 
> We are interested with the above TODOs, and we are planning to implement them.
> 
Hi Mathieu,

I finished a simple ltt-control.ko based on debugfs.

With this module, we can control ltt by following command now:
Start trace1 channel:
echo "0 trace1" > /sys/kernel/debug/ltt_control/control
Stop trace1 channel:
echo "1 trace1" > /sys/kernel/debug/ltt_control/control
Create trace1 channel:
echo "2 trace1 relay 0 0 0 0 0 0 0" > /sys/kernel/debug/ltt_control/control
Destroy trace1 channel:
echo "3 trace1" > /sys/kernel/debug/ltt_control/control

I tested it on i386 linux 2.6.27 with lttng.

User-mode lttctl also needs some modification to support this kind of control,
but at first, I'd like to hear some advice on this module.

Thanks!
Zhaolei
> Thanks, Lai
> 

>> If some of you are interested to work on any of these, you are welcome.
>> I'll be glad to post more details about these items upon request.
>>
>> Mathieu
>>
> 
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: Makefile
URL: <http://lists.casi.polymtl.ca/pipermail/ltt-dev/attachments/20081017/6261648f/attachment.ksh>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: ltt_control.c
URL: <http://lists.casi.polymtl.ca/pipermail/ltt-dev/attachments/20081017/6261648f/attachment.txt>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [ltt-dev] TODO list before releasing LTTng-buffering to LKML]
  2008-10-17  3:56 ` [ltt-dev] TODO list before releasing LTTng-buffering to LKML] Zhaolei
  2008-10-17  3:56   ` Zhaolei
@ 2008-10-17  3:56   ` Zhaolei
       [not found]   ` <20081017042045.GB5067@Krystal>
  2 siblings, 0 replies; 8+ messages in thread
From: Zhaolei @ 2008-10-17  3:56 UTC (permalink / raw)


>> - switch ltt-control.ko (currently over netlink) to debugfs
>> - switch marker list (and marker activation (currently in /proc/ltt) to debugfs
>>   Marker directory. One file per marker.
>> - Create a ltt-ascii.ko kernel module which merge-sorts the buffers and exports
>>   them to userspace through a debugfs file.
>>
> 
> Hi, Mathieu,
> 
> We are interested with the above TODOs, and we are planning to implement them.
> 
Hi Mathieu,

I finished a simple ltt-control.ko based on debugfs.

With this module, we can control ltt by following command now:
Start trace1 channel:
echo "0 trace1" > /sys/kernel/debug/ltt_control/control
Stop trace1 channel:
echo "1 trace1" > /sys/kernel/debug/ltt_control/control
Create trace1 channel:
echo "2 trace1 relay 0 0 0 0 0 0 0" > /sys/kernel/debug/ltt_control/control
Destroy trace1 channel:
echo "3 trace1" > /sys/kernel/debug/ltt_control/control

I tested it on i386 linux 2.6.27 with lttng.

User-mode lttctl also needs some modification to support this kind of control,
but at first, I'd like to hear some advice on this module.

Thanks!
Zhaolei
> Thanks, Lai
> 

>> If some of you are interested to work on any of these, you are welcome.
>> I'll be glad to post more details about these items upon request.
>>
>> Mathieu
>>
> 
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: Makefile
URL: <http://lists.casi.polymtl.ca/pipermail/lttng-dev/attachments/20081017/6261648f/attachment.asc>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: ltt_control.c
URL: <http://lists.casi.polymtl.ca/pipermail/lttng-dev/attachments/20081017/6261648f/attachment-0002.txt>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [ltt-dev] TODO list before releasing LTTng-buffering to LKML]
       [not found]   ` <20081017042045.GB5067@Krystal>
@ 2008-10-17  6:31     ` Zhaolei
       [not found]       ` <20081017161429.GA5696@Krystal>
  0 siblings, 1 reply; 8+ messages in thread
From: Zhaolei @ 2008-10-17  6:31 UTC (permalink / raw)


>* Zhaolei (zhaolei at cn.fujitsu.com) wrote:
>> >> - switch ltt-control.ko (currently over netlink) to debugfs
>> >> - switch marker list (and marker activation (currently in /proc/ltt) to debugfs
>> >>   Marker directory. One file per marker.
>> >> - Create a ltt-ascii.ko kernel module which merge-sorts the buffers and exports
>> >>   them to userspace through a debugfs file.
>> >>
>> > 
>> > Hi, Mathieu,
>> > 
>> > We are interested with the above TODOs, and we are planning to implement them.
>> > 
>> Hi Mathieu,
>> 
>> I finished a simple ltt-control.ko based on debugfs.
>> 
>> With this module, we can control ltt by following command now:
>> Start trace1 channel:
>> echo "0 trace1" > /sys/kernel/debug/ltt_control/control
>> Stop trace1 channel:
>> echo "1 trace1" > /sys/kernel/debug/ltt_control/control
>> Create trace1 channel:
>> echo "2 trace1 relay 0 0 0 0 0 0 0" > /sys/kernel/debug/ltt_control/control
>> Destroy trace1 channel:
>> echo "3 trace1" > /sys/kernel/debug/ltt_control/control
>> 
>> I tested it on i386 linux 2.6.27 with lttng.
>> 
>> User-mode lttctl also needs some modification to support this kind of control,
>> but at first, I'd like to hear some advice on this module.
>> 
> 
> Hrm, let me see.. can we have a directory hierarchy like this ?
> 
Hello, Mathieu,

Thanks for your advice!

Are you means make debugfs like this?
(let us assume er have a channel named trace1)

`-- debugfs
    `-- ltt
        |-- buffers
        |   `-- trace1
        |       |-- control
        |       |   |-- interrupts_0
        |       |   |-- metadata_0
        |       |   |-- modules_0
        |       |   |-- network_0
        |       |   `-- processes_0
        |       `-- cpu_0
        |-- control
        |   `-- trace1
        |       |-- alloc
        |       |-- channel
        |       |   |-- control
        |       |   |   |-- interrupts
        |       |   |   |   |-- subbuf_num
        |       |   |   |   `-- subbuf_size
        |       |   |   |-- metadata
        |       |   |   |   |-- subbuf_num
        |       |   |   |   `-- subbuf_size
        |       |   |   |-- modules
        |       |   |   |   |-- subbuf_num
        |       |   |   |   `-- subbuf_size
        |       |   |   |-- network
        |       |   |   |   |-- subbuf_num
        |       |   |   |   `-- subbuf_size
        |       |   |   `-- processes
        |       |   |       |-- subbuf_num
        |       |   |       `-- subbuf_size
        |       |   `-- cpu
        |       |       |-- mode
        |       |       |-- subbuf_num
        |       |       `-- subbuf_size
        |       `-- enabled
        |-- setup_trace
        `-- trace?

> (/sys/kernel/debug seems to be the same as /mnt/debugfs)
> 
> #define LTT_ROOT_DENTRY "/mnt/debugfs/ltt"
> 
> All the following are subdirectories of LTT_ROOT_DENTRY
> 
> #define LTT_BUFFERS_DENTRY LTT_ROOT_DENTRY "/buffers"
> #define LTT_CONTROL_DENTRY LTT_ROOT_DENTRY "/control"
> 
> #define LTT_TRACE_DENTRY LTT_ROOT_DENTRY "/trace"
> file -> read() : shows how to use it
Could you teach me detail about LTT_TRACE_DENTRY?
It it used to export trace status?
and what is hierarchy in LTT_TRACE_DENTRY?

> Create a new trace :
> echo trace_name > /mnt/debugfs/ltt/setup_trace
> 
> Note that a "setup" trace will be _different_ from the current
> "creation" notion in LTTng. I would like to have a state where a trace
> is created in the sense that its data structures are there, but the
> buffers are not allocated and it's not ready for tracing. A second
> "alloc" operation will have to be performed after the buffer sizes have
> been entered to perform the actual buffer allocation. After that, a
> trace start can be issued.
Are you means we need following steps to start a trace:
1: setup trace
2: alloc trace
3: start trace

I understood differece between "setup" and "alloc", but why we need a "setup" status?
(used for what? or what is the merit?)

> 
> Under LTT_BUFFERS_DENTRY, we create a new directory to contain each trace
> buffers.
> 
> e.g. :
> LTT_BUFFERS_DENTRY "/trace_name/cpu_0"
> LTT_BUFFERS_DENTRY "/trace_name/control/metadata_0"
> ...
> 
> Under LTT_CONTROL_DENTRY, we create a new directory to contain each
> trace control files.
> 
> (in parenthesis, this is probably a tiny description of what the files
> do and of what should be returned on read())
> 
> e.g. :
> LTT_CONTROL_DENTRY "/trace_name/alloc" (just writing to the file should
>  be enough) (allocate the buffers)
> LTT_CONTROL_DENTRY "/trace_name/enabled" (echo 0 et 1) (tracing enabled)
> LTT_CONTROL_DENTRY "/trace_name/channel/cpu/subbuf_size" (subbuffer size)
> LTT_CONTROL_DENTRY "/trace_name/channel/cpu/subbuf_num" (number of
>   subbuffers)
> LTT_CONTROL_DENTRY "/trace_name/channel/cpu/mode" (normal, flight)
> LTT_CONTROL_DENTRY "/trace_name/channel/control/metadata/subbuf_size"
>  (subbuffer size)
> LTT_CONTROL_DENTRY "/trace_name/channel/control/metadata/subbuf_num"
>  (number of subbuffers)
Are you means we can control each channel's (interrupt, process, ...) subbuf in future version?
(I think current version can only control low, med and high)
If this is true, maybe we need modify ltt_trace_create() before impl ltt_control.

> 
> Does it make sense ?
Thanks for your advice.
I think maybe we need to decide some detail action of this implement.
For example, for user,
correct operation is
  echo "trace1" > debugfs/ltt/setup_trace
  echo 65536 > debugfs/ltt/control/trace1/channel/interrupts/subbuf_num
  echo 2 > debugfs/ltt/control/trace1/channel/interrupts/subbuf_size
  echo 262144 > debugfs/ltt/control/trace1/channel/process/subbuf_num
  echo 2 > debugfs/ltt/control/trace1/channel/process/subbuf_size
  ...
  echo 1 > debugfs/ltt/control/trace1/alloc
  echo 1 > debugfs/ltt/control/trace1/enable
but if user:
  echo 4 > debugfs/ltt/control/trace1/channel/process/subbuf_size
  after echo 1 > debugfs/ltt/control/trace1/alloc
  maybe we need return a error, because it is already allocated.

> 
> Mathieu
> 
>> Thanks!
>> Zhaolei
>> > Thanks, Lai
>> > 
>> 
>> >> If some of you are interested to work on any of these, you are welcome.
>> >> I'll be glad to post more details about these items upon request.
>> >>
>> >> Mathieu
>> >>
>> > 
> 
>> obj-m := ltt_control.o
>> KDIR := /lib/modules/$(shell uname -r)/build
>> PWD := $(shell pwd)
>> 
>> default:
>> $(MAKE) -C $(KDIR) SUBDIRS=$(PWD) modules
>> clean:
>> rm -rf *.mod.c *.ko *.o .*.cmd .tmp_versions Module.symvers Module.markers modules.order
> 
>> /*
>>  * LTT control module over debugfs.
>>  *
>>  * Copyright 2008 -
>>  * Zhaolei <zhaolei at cn.fujitsu.com>
>>  */
>> 
>> #include <linux/module.h>
>> #include <linux/fs.h>
>> #include <linux/uaccess.h>
>> #include <linux/debugfs.h>
>> #include <linux/ltt-tracer.h>
>> 
>> #define LTT_CONTROL_DIR "ltt_control"
>> #define LTT_CONTROL_RECV "control"
>> 
>> #define LTT_CONTROL_RECVMAXLINE (128)
>> 
>> struct dentry *lttctrl_dir, *lttctrl_file;
>> 
>> static ssize_t read_control(struct file *file, char __user *user_buf,
>> size_t count, loff_t *ppos)
>> {
>> char info[] = "ltt_control informations\nTodo: impl it\n";
>> 
>> return simple_read_from_buffer(user_buf, count, ppos, info,
>> sizeof(info));
>> }
>> 
>> /*
>>  * line format:
>>  * Start:
>>  * control_msg(0)
>>  * name
>>  * Stop:
>>  * control_msg(1)
>>  * name
>>  * Create:
>>  * control_msg(2)
>>  * name
>>  * type
>>  * trace_mode
>>  * subbuf_size_low
>>  * n_subbufs_low
>>  * subbuf_size_med
>>  * n_subbufs_med
>>  * subbuf_size_high
>>  * n_subbufs_high
>>  * Destroy:
>>  * control_msg(3)
>>  * name
>>  *
>>  * control_msg:
>>  * 0: LTT_CONTROL_START
>>  * 1: LTT_CONTROL_STOP
>>  * 2: LTT_CONTROL_CREATE_TRACE
>>  * 3: LTT_CONTROL_DESTROY_TRACE
>>  *
>>  * trace_mode:
>>  * 0: LTT_TRACE_NORMAL
>>  * 1: LTT_TRACE_FLIGHT
>>  * 2: LTT_TRACE_HYBRID
>>  *
>>  * Ex:
>>  * Create a normal trace named trace1:
>>  * echo "2 trace1 relay 0 0 0 0 0 0 0" > \
>>  * /sys/kernel/debug/ltt_control/control
>>  * Start trace1 channel:
>>  * echo "0 trace1" > /sys/kernel/debug/ltt_control/control
>>  * Stop trace1 channel:
>>  * echo "1 trace1" > /sys/kernel/debug/ltt_control/control
>>  * Destroy trace1 channel:
>>  * echo "3 trace1" > /sys/kernel/debug/ltt_control/control
>>  */
>> static ssize_t write_control(struct file *file, const char __user *user_buf,
>> size_t count, loff_t *ppos)
>> {
>> char buf[LTT_CONTROL_RECVMAXLINE];
>> int buf_size;
>> 
>> enum ltt_control_msg msg;
>> char trace_name[NAME_MAX];
>> char trace_type[NAME_MAX];
>> union ltt_control_args args;
>> unsigned int op_n, mode_n;
>> 
>> int err = 0;
>> 
>> buf_size = min(count, sizeof(buf) - 1);
>> err = copy_from_user(buf, user_buf, buf_size);
>> if (err)
>> return err;
>> buf[buf_size] = 0;
>> 
>> if (1 != sscanf(buf, "%u", &op_n))
>> return -EPERM;
>> 
>> /*
>> * Not necessary to clear memory, only for safe, maybe I will
>> * remove it
>> */
>> msg = 0;
>> trace_name[0] = 0;
>> trace_type[0] = 0;
>> memset(&args, 0, sizeof(args));
>> 
>> switch (op_n) {
>> case 0:
>> msg = LTT_CONTROL_START;
>> if (1 != sscanf(buf, "%*u %s", trace_name))
>> return -EPERM;
>> break;
>> case 1:
>> msg = LTT_CONTROL_STOP;
>> if (1 != sscanf(buf, "%*u %s", trace_name))
>> return -EPERM;
>> break;
>> case 2:
>> msg = LTT_CONTROL_CREATE_TRACE;
>> if (9 != sscanf(buf, "%*u %s %s %u %u %u %u %u %u %u",
>> trace_name,
>> trace_type,
>> &mode_n,
>> &args.new_trace.subbuf_size_low,
>> &args.new_trace.n_subbufs_low,
>> &args.new_trace.subbuf_size_med,
>> &args.new_trace.n_subbufs_med,
>> &args.new_trace.subbuf_size_high,
>> &args.new_trace.n_subbufs_high
>> ))
>> return -EPERM;
>> switch (mode_n) {
>> case 0:
>> args.new_trace.mode = LTT_TRACE_NORMAL;
>> break;
>> case 1:
>> args.new_trace.mode = LTT_TRACE_FLIGHT;
>> break;
>> case 2:
>> args.new_trace.mode = LTT_TRACE_HYBRID;
>> break;
>> default:
>> return -EPERM;
>> }
>> break;
>> case 3:
>> msg = LTT_CONTROL_DESTROY_TRACE;
>> if (1 != sscanf(buf, "%*u %s", trace_name))
>> return -EPERM;
>> break;
>> default:
>> return -EPERM;
>> }
>> 
>> err = ltt_control(msg, trace_name, trace_type, args);
>> if (IS_ERR(err))
>> return err;
>> 
>> return count;
>> }
>> 
>> static struct file_operations ltt_control_operations = {
>> .read = read_control,
>> .write = write_control,
>> };
>> 
>> static int __init init_ltt_control(void)
>> {
>> lttctrl_dir = debugfs_create_dir(LTT_CONTROL_DIR, NULL);
>> if (IS_ERR(lttctrl_dir) || !lttctrl_dir)
>> return -ENOMEM;
>> 
>> lttctrl_file = debugfs_create_file(LTT_CONTROL_RECV, 0600, lttctrl_dir,
>> NULL, &ltt_control_operations);
>> if (IS_ERR(lttctrl_file) || !lttctrl_file) {
>> debugfs_remove(lttctrl_dir);
>> return -ENOMEM;
>> }
>> 
>> return 0;
>> }
>> 
>> static void __exit exit_ltt_control(void)
>> {
>> debugfs_remove(lttctrl_file);
>> debugfs_remove(lttctrl_dir);
>> }
>> 
>> module_init(init_ltt_control);
>> module_exit(exit_ltt_control);
>> 
>> MODULE_LICENSE("GPL");
> 
> 
> -- 
> Mathieu Desnoyers
> OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
> 
>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [ltt-dev] TODO list before releasing LTTng-buffering to LKML]
       [not found]       ` <20081017161429.GA5696@Krystal>
@ 2008-10-20  3:03         ` Zhaolei
       [not found]           ` <20081020031634.GA14209@Krystal>
  0 siblings, 1 reply; 8+ messages in thread
From: Zhaolei @ 2008-10-20  3:03 UTC (permalink / raw)


Hello, Mathieu.

>> Are you means we can control each channel's (interrupt, process, ...) subbuf in future version?
>> (I think current version can only control low, med and high)
>> If this is true, maybe we need modify ltt_trace_create() before impl ltt_control.
>> 
> Yes, this is the goal.
So, IMHO, we can do it in following steps:

Step1: modify ltt_trace_create(), .etc, separate ltt_trace_create() into
  ltt_trace_setup(), ltt_set_channelsize(), ltt_set_channelnum(),
  ltt_trace_alloc(). But still use old netlink_based ltt_control to make
  it simple. 
  Although we modify ltt_set_channelsize() to support setting each channel's
  buffer, we make ltt_control.ko using old UI(only able to set low, med and
  high). So we don't need to modify other program such as user-mode lttctl.

Step2: Write a new module named ltt-channel-control.
  ( I think name of ltt-channel-control will be better than ltt-control,
    because we already have a module named ltt-marker-control. )
  ltt-channel-control is designed on debugfs, as we discussed.
  We can test it by echo XX > /mnt/debugfs/ltt/...

Step3: modify user-mode programs to support ltt-channel-control.
  We also need to modify lttctl's command arguments, to support setup each
  channel's buffer. (Are we take out -Z, -X, -V, -B, -z, -x in lttctl?)
  I haven't read source of lttv, it is necessary to modify?

B.R.
Zhaolei


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [ltt-dev] [PATCH] Separate ltt_trace_create() into ltt_trace_create() and ltt_trace_alloc()
       [not found]           ` <20081020031634.GA14209@Krystal>
@ 2008-10-27  3:47             ` Zhaolei
  2008-10-28 14:25               ` Mathieu Desnoyers
  0 siblings, 1 reply; 8+ messages in thread
From: Zhaolei @ 2008-10-27  3:47 UTC (permalink / raw)


This is step1 of:
"switch marker list and marker activation (currently in /proc/ltt) to debugfs"

> >> Are you means we can control each channel's (interrupt, process, ...)
> >> subbuf in future version?
> >> (I think current version can only control low, med and high)
> >> If this is true, maybe we need modify ltt_trace_create() before impl
> >> ltt_control.
> >>
> > Yes, this is the goal.
> So, IMHO, we can do it in following steps:
>
> Step1: modify ltt_trace_create(), .etc, separate ltt_trace_create() into
>   ltt_trace_setup(), ltt_set_channelsize(), ltt_set_channelnum(),
>   ltt_trace_alloc(). But still use old netlink_based ltt_control to make
>   it simple.
>   Although we modify ltt_set_channelsize() to support setting each channel's
>   buffer, we make ltt_control.ko using old UI(only able to set low, med and
>   high). So we don't need to modify other program such as user-mode lttctl.
>

Yes. Just make sure a "destroy" deals correctly with a trace that is
"setup" but not "allocated" yet.

> Step2: Write a new module named ltt-channel-control.
>   ( I think name of ltt-channel-control will be better than ltt-control,
>     because we already have a module named ltt-marker-control. )
>   ltt-channel-control is designed on debugfs, as we discussed.
>   We can test it by echo XX > /mnt/debugfs/ltt/...
>

Yes, that would be perfect.

> Step3: modify user-mode programs to support ltt-channel-control.
>   We also need to modify lttctl's command arguments, to support setup each
>   channel's buffer. (Are we take out -Z, -X, -V, -B, -z, -x in lttctl?)
>   I haven't read source of lttv, it is necessary to modify?
>

Maybe we could replace lttctl by simple shell scripts ?

Yes, small modification of lttv would be required. There is a GUI module
which executes lttctl to start/stop tracing. It would have to be
modified so it uses the debugfs files instead.

Signed-off-by: Zhao Lei <zhaolei at cn.fujitsu.com>
---
 include/linux/ltt-core.h   |    3 +-
 include/linux/ltt-tracer.h |   40 ++--
 ltt/ltt-core.c             |    1 +
 ltt/ltt-tracer.c           |  560 ++++++++++++++++++++++++++++++++++----------
 4 files changed, 464 insertions(+), 140 deletions(-)

diff --git a/include/linux/ltt-core.h b/include/linux/ltt-core.h
index 1681e82..db890d8 100644
--- a/include/linux/ltt-core.h
+++ b/include/linux/ltt-core.h
@@ -17,7 +17,8 @@
  * list.
  */
 struct ltt_traces {
-	struct list_head head;		/* Traces list */
+	struct list_head prealloc_head;	/* Pre-allocated traces list */
+	struct list_head head;		/* Allocated Traces list */
 	unsigned int num_active_traces;	/* Number of active traces */
 } ____cacheline_aligned;
 
diff --git a/include/linux/ltt-tracer.h b/include/linux/ltt-tracer.h
index d46bc88..042f518 100644
--- a/include/linux/ltt-tracer.h
+++ b/include/linux/ltt-tracer.h
@@ -197,6 +197,23 @@ struct ltt_transport {
 	struct ltt_trace_ops ops;
 };
 
+/*
+ * First and last channels in ltt_trace_struct.
+ */
+#define ltt_channel_index_size()	sizeof(struct ltt_channel_struct *)
+#define ltt_channel_index_begin()	GET_CHANNEL_INDEX(cpu)
+#define ltt_channel_index_end()	\
+	(GET_CHANNEL_INDEX(metadata) + ltt_channel_index_size())
+
+enum ltt_channels {
+	LTT_CHANNEL_CPU,
+	LTT_CHANNEL_PROCESSES,
+	LTT_CHANNEL_INTERRUPTS,
+	LTT_CHANNEL_NETWORK,
+	LTT_CHANNEL_MODULES,
+	LTT_CHANNEL_METADATA,
+	NR_LTT_CHANNELS,
+};
 
 enum trace_mode { LTT_TRACE_NORMAL, LTT_TRACE_FLIGHT, LTT_TRACE_HYBRID };
 
@@ -218,6 +235,12 @@ struct ltt_trace_struct {
 		struct ltt_channel_struct	*modules;
 		struct ltt_channel_struct	*metadata;
 	} channel;
+	struct {
+		struct {
+			unsigned subbuf_size;
+			unsigned subbuf_cnt;
+		} channels[NR_LTT_CHANNELS];
+	} setting;
 	u32 freq_scale;
 	u64 start_freq;
 	u64 start_tsc;
@@ -236,23 +259,6 @@ struct ltt_trace_struct {
 	char trace_name[NAME_MAX];
 } ____cacheline_aligned;
 
-/*
- * First and last channels in ltt_trace_struct.
- */
-#define ltt_channel_index_size()	sizeof(struct ltt_channel_struct *)
-#define ltt_channel_index_begin()	GET_CHANNEL_INDEX(cpu)
-#define ltt_channel_index_end()	\
-	(GET_CHANNEL_INDEX(metadata) + ltt_channel_index_size())
-
-enum ltt_channels {
-	LTT_CHANNEL_CPU,
-	LTT_CHANNEL_PROCESSES,
-	LTT_CHANNEL_INTERRUPTS,
-	LTT_CHANNEL_NETWORK,
-	LTT_CHANNEL_MODULES,
-	LTT_CHANNEL_METADATA,
-};
-
 /* Hardcoded event headers
  *
  * event header for a trace with active heartbeat : 27 bits timestamps
diff --git a/ltt/ltt-core.c b/ltt/ltt-core.c
index abd3d6d..3d7015e 100644
--- a/ltt/ltt-core.c
+++ b/ltt/ltt-core.c
@@ -12,6 +12,7 @@
 
 /* Traces structures */
 struct ltt_traces ltt_traces = {
+	.prealloc_head = LIST_HEAD_INIT(ltt_traces.prealloc_head),
 	.head = LIST_HEAD_INIT(ltt_traces.head),
 };
 EXPORT_SYMBOL(ltt_traces);
diff --git a/ltt/ltt-tracer.c b/ltt/ltt-tracer.c
index e4358cf..5eae2b5 100644
--- a/ltt/ltt-tracer.c
+++ b/ltt/ltt-tracer.c
@@ -268,31 +268,20 @@ static struct ltt_trace_struct *_ltt_trace_find(const char *trace_name)
 	return NULL;
 }
 
-/* This function must be called with traces semaphore held. */
-static int _ltt_trace_create(const char *trace_name, enum trace_mode mode,
-				struct ltt_trace_struct *new_trace)
+/* _ltt_trace_find_setup :
+ * find a trace in setup list by given name.
+ *
+ * Returns a pointer to the trace structure, NULL if not found.
+ */
+static struct ltt_trace_struct *_ltt_trace_find_setup(const char *trace_name)
 {
-	int err = EPERM;
+	struct ltt_trace_struct *trace;
 
-	if (_ltt_trace_find(trace_name) != NULL) {
-		printk(KERN_ERR "LTT : Trace %s already exists\n", trace_name);
-		err = EEXIST;
-		goto traces_error;
-	}
-	if (list_empty(&ltt_traces.head)) {
-		probe_id_defrag();
-		mod_timer(&ltt_async_wakeup_timer,
-				jiffies + LTT_PERCPU_TIMER_INTERVAL);
-		set_kernel_trace_flag_all_tasks();
-	}
-	list_add_rcu(&new_trace->list, &ltt_traces.head);
-	synchronize_sched();
-	/* Everything went fine, finish creation */
-	return 0;
+	list_for_each_entry(trace, &ltt_traces.prealloc_head, list)
+		if (!strncmp(trace->trace_name, trace_name, NAME_MAX))
+			return trace;
 
-	/* Error handling */
-traces_error:
-	return err;
+	return NULL;
 }
 
 /**
@@ -334,170 +323,491 @@ static inline void prepare_chan_size_num(unsigned *subbuf_size,
 	WARN_ON(hweight32(*n_subbufs) != 1);
 }
 
-static int ltt_trace_create(const char *trace_name, const char *trace_type,
-		enum trace_mode mode,
-		unsigned subbuf_size_low, unsigned n_subbufs_low,
-		unsigned subbuf_size_med, unsigned n_subbufs_med,
-		unsigned subbuf_size_high, unsigned n_subbufs_high)
+static int ltt_trace_setup(const char *trace_name)
 {
 	int err = 0;
-	struct ltt_trace_struct *new_trace, *trace;
-	unsigned long flags;
-	struct ltt_transport *tran, *transport = NULL;
+	struct ltt_trace_struct *new_trace = NULL;
 
-	prepare_chan_size_num(&subbuf_size_low, &n_subbufs_low,
-		LTT_DEFAULT_SUBBUF_SIZE_LOW, LTT_DEFAULT_N_SUBBUFS_LOW);
+	ltt_lock_traces();
 
-	prepare_chan_size_num(&subbuf_size_med, &n_subbufs_med,
-		LTT_DEFAULT_SUBBUF_SIZE_MED, LTT_DEFAULT_N_SUBBUFS_MED);
+	if (_ltt_trace_find_setup(trace_name)) {
+		printk(KERN_ERR	"LTT : Trace name %s already used.\n",
+				trace_name);
+		err = -EEXIST;
+		goto traces_error;
+	}
 
-	prepare_chan_size_num(&subbuf_size_high, &n_subbufs_high,
-		LTT_DEFAULT_SUBBUF_SIZE_HIGH, LTT_DEFAULT_N_SUBBUFS_HIGH);
+	if (_ltt_trace_find(trace_name)) {
+		printk(KERN_ERR	"LTT : Trace name %s already used.\n",
+				trace_name);
+		err = -EEXIST;
+		goto traces_error;
+	}
 
 	new_trace = kzalloc(sizeof(struct ltt_trace_struct), GFP_KERNEL);
 	if (!new_trace) {
 		printk(KERN_ERR
 			"LTT : Unable to allocate memory for trace %s\n",
 			trace_name);
-		err = ENOMEM;
-		goto traces_error;
+		err = -ENOMEM;
+		goto trace_free;
 	}
 
-	kref_init(&new_trace->kref);
-	kref_init(&new_trace->ltt_transport_kref);
-	init_waitqueue_head(&new_trace->kref_wq);
-	new_trace->active = 0;
 	strncpy(new_trace->trace_name, trace_name, NAME_MAX);
-	new_trace->mode = mode;
-	new_trace->freq_scale = trace_clock_freq_scale();
+	list_add(&new_trace->list, &ltt_traces.prealloc_head);
+
+	ltt_unlock_traces();
+
+	return 0;
+
+trace_free:
+	kfree(new_trace);
+traces_error:
+	ltt_unlock_traces();
+	return err;
+}
+
+/* must be called from within a traces lock. */
+static void _ltt_trace_free(struct ltt_trace_struct *trace)
+{
+	list_del(&trace->list);
+	kfree(trace);
+}
+
+static int ltt_trace_set_type(const char *trace_name, const char *trace_type)
+{
+	int err = 0;
+	struct ltt_trace_struct *trace;
+	struct ltt_transport *tran_iter, *transport = NULL;
 
 	ltt_lock_traces();
-	list_for_each_entry(tran, &ltt_transport_list, node) {
-		if (!strcmp(tran->name, trace_type)) {
-			transport = tran;
+
+	trace = _ltt_trace_find_setup(trace_name);
+	if (!trace) {
+		printk(KERN_ERR "LTT : Trace not found %s\n", trace_name);
+		err = -ENOENT;
+		goto traces_error;
+	}
+
+	list_for_each_entry(tran_iter, &ltt_transport_list, node) {
+		if (!strcmp(tran_iter->name, trace_type)) {
+			transport = tran_iter;
 			break;
 		}
 	}
-
 	if (!transport) {
-		err = EINVAL;
 		printk(KERN_ERR	"LTT : Transport %s is not present.\n",
 			trace_type);
-		ltt_unlock_traces();
-		goto trace_error;
+		err = -EINVAL;
+		goto traces_error;
 	}
 
-	if (!try_module_get(transport->owner)) {
-		err = ENODEV;
-		printk(KERN_ERR	"LTT : Can't lock transport module.\n");
-		ltt_unlock_traces();
-		goto trace_error;
+	trace->transport = transport;
+
+traces_error:
+	ltt_unlock_traces();
+	return err;
+}
+
+static int ltt_trace_set_mode(const char *trace_name, enum trace_mode mode)
+{
+	int err = 0;
+	struct ltt_trace_struct *trace;
+
+	ltt_lock_traces();
+
+	trace = _ltt_trace_find_setup(trace_name);
+	if (!trace) {
+		printk(KERN_ERR "LTT : Trace not found %s\n", trace_name);
+		err = -ENOENT;
+		goto traces_error;
 	}
 
-	trace = _ltt_trace_find(trace_name);
-	if (trace) {
-		printk(KERN_ERR	"LTT : Trace name %s already used.\n",
-			trace_name);
-		err = EEXIST;
-		goto trace_error;
+	trace->mode = mode;
+
+traces_error:
+	ltt_unlock_traces();
+	return err;
+}
+
+/*
+ * Todo:
+ * Make similar function in channel.c,
+ * so it will be useful for both ltt-tracer.c and ltt-marker-control.c
+ */
+/*
+ * Its order is MUST be same with enum ltt_channels
+ */
+static char *chan_names[] = {
+	LTT_CPU_CHANNEL,
+	LTT_PROCESSES_CHANNEL,
+	LTT_INTERRUPTS_CHANNEL,
+	LTT_NETWORK_CHANNEL,
+	LTT_MODULES_CHANNEL,
+	LTT_METADATA_CHANNEL,
+};
+
+static enum ltt_channels get_channel_type_from_name(const char *name)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(chan_names); i++)
+		if (!strcmp(name, chan_names[i]))
+			return (enum ltt_channels)i;
+
+	return NR_LTT_CHANNELS;
+}
+
+static int ltt_trace_set_channel_subbufsize(const char *trace_name,
+		const char *channel_name, unsigned size)
+{
+	int err = 0;
+	struct ltt_trace_struct *trace;
+	enum ltt_channels channel;
+
+	ltt_lock_traces();
+
+	trace = _ltt_trace_find_setup(trace_name);
+	if (!trace) {
+		printk(KERN_ERR "LTT : Trace not found %s\n", trace_name);
+		err = -ENOENT;
+		goto traces_error;
 	}
 
-	new_trace->transport = transport;
-	new_trace->ops = &transport->ops;
+	channel = get_channel_type_from_name(channel_name);
+	if (channel == NR_LTT_CHANNELS) {
+		printk(KERN_ERR "LTT : Channel %s is not present.\n",
+			channel_name);
+		err = -EINVAL;
+		goto traces_error;
+	}
 
-	err = new_trace->ops->create_dirs(new_trace);
-	if (err)
+	trace->setting.channels[channel].subbuf_size = size;
+
+traces_error:
+	ltt_unlock_traces();
+	return err;
+}
+
+static int ltt_trace_set_channel_subbufcount(const char *trace_name,
+		const char *channel_name, unsigned cnt)
+{
+	int err = 0;
+	struct ltt_trace_struct *trace;
+	enum ltt_channels channel;
+
+	ltt_lock_traces();
+
+	trace = _ltt_trace_find_setup(trace_name);
+	if (!trace) {
+		printk(KERN_ERR "LTT : Trace not found %s\n", trace_name);
+		err = -ENOENT;
+		goto traces_error;
+	}
+
+	channel = get_channel_type_from_name(channel_name);
+	if (channel == NR_LTT_CHANNELS) {
+		printk(KERN_ERR "LTT : Channel %s is not present.\n",
+			channel_name);
+		err = -EINVAL;
+		goto traces_error;
+	}
+
+	trace->setting.channels[channel].subbuf_cnt = cnt;
+
+traces_error:
+	ltt_unlock_traces();
+	return err;
+}
+
+static int ltt_trace_alloc(const char *trace_name)
+{
+	int err = 0;
+	struct ltt_trace_struct *trace;
+	int subbuf_size, subbuf_cnt;
+	unsigned long flags;
+
+	ltt_lock_traces();
+
+	trace = _ltt_trace_find_setup(trace_name);
+	if (!trace) {
+		printk(KERN_ERR "LTT : Trace not found %s\n", trace_name);
+		err = -ENOENT;
+		goto traces_error;
+	}
+
+	kref_init(&trace->kref);
+	kref_init(&trace->ltt_transport_kref);
+	init_waitqueue_head(&trace->kref_wq);
+	trace->active = 0;
+	trace->freq_scale = trace_clock_freq_scale();
+
+	if (!trace->transport) {
+		printk(KERN_ERR "LTT : Transport is not set.\n");
+		err = -EINVAL;
+		goto traces_error;
+	}
+	if (!try_module_get(trace->transport->owner)) {
+		printk(KERN_ERR	"LTT : Can't lock transport module.\n");
+		err = -ENODEV;
+		goto traces_error;
+	}
+	trace->ops = &trace->transport->ops;
+
+	err = trace->ops->create_dirs(trace);
+	if (err) {
+		printk(KERN_ERR	"LTT : Can't create dir for trace %s.\n",
+			trace_name);
 		goto dirs_error;
+	}
 
 	local_irq_save(flags);
-	new_trace->start_freq = trace_clock_frequency();
-	new_trace->start_tsc = trace_clock_read64();
-	do_gettimeofday(&new_trace->start_time);
+	trace->start_freq = trace_clock_frequency();
+	trace->start_tsc = trace_clock_read64();
+	do_gettimeofday(&trace->start_time);
 	local_irq_restore(flags);
 
 	/*
+	 * Todo:
+	 *   Config each channel's default buffersize/cnt, instead of
+	 *   LTT_DEFAULT_SUBBUF_SIZE_LOW, ...
+	 */
+
+	/*
 	 * Always put the metadata channel in non-overwrite mode :
 	 * This is a very low traffic channel and it can't afford to have its
 	 * data overwritten : this data (marker info) is necessary to be
 	 * able to read the trace.
 	 */
-	err = new_trace->ops->create_channel(trace_name, new_trace,
-			new_trace->dentry.control_root,
+	subbuf_size = trace->setting.channels[LTT_CHANNEL_METADATA].subbuf_size;
+	subbuf_cnt = trace->setting.channels[LTT_CHANNEL_METADATA].subbuf_cnt;
+	prepare_chan_size_num(&subbuf_size, &subbuf_cnt,
+		LTT_DEFAULT_SUBBUF_SIZE_LOW, LTT_DEFAULT_N_SUBBUFS_LOW);
+	err = trace->ops->create_channel(trace_name, trace,
+			trace->dentry.control_root,
 			LTT_METADATA_CHANNEL,
-			&new_trace->channel.metadata, subbuf_size_low,
-			n_subbufs_low,
-			is_channel_overwrite(LTT_CHANNEL_METADATA, mode));
-	if (err != 0)
+			&trace->channel.metadata,
+			subbuf_size,
+			subbuf_cnt,
+			is_channel_overwrite(LTT_CHANNEL_METADATA,
+				trace->mode));
+	if (err != 0) {
+		printk(KERN_ERR	"LTT : Can't create channel %s.\n",
+			chan_names[LTT_CHANNEL_METADATA]);
 		goto metadata_error;
-	err = new_trace->ops->create_channel(trace_name, new_trace,
-			new_trace->dentry.control_root,
+	}
+
+	subbuf_size =
+		trace->setting.channels[LTT_CHANNEL_INTERRUPTS].subbuf_size;
+	subbuf_cnt = trace->setting.channels[LTT_CHANNEL_INTERRUPTS].subbuf_cnt;
+	prepare_chan_size_num(&subbuf_size, &subbuf_cnt,
+		LTT_DEFAULT_SUBBUF_SIZE_LOW, LTT_DEFAULT_N_SUBBUFS_LOW);
+	err = trace->ops->create_channel(trace_name, trace,
+			trace->dentry.control_root,
 			LTT_INTERRUPTS_CHANNEL,
-			&new_trace->channel.interrupts, subbuf_size_low,
-			n_subbufs_low,
-			is_channel_overwrite(LTT_CHANNEL_INTERRUPTS, mode));
-	if (err != 0)
+			&trace->channel.interrupts,
+			subbuf_size,
+			subbuf_cnt,
+			is_channel_overwrite(LTT_CHANNEL_INTERRUPTS,
+				trace->mode));
+	if (err != 0) {
+		printk(KERN_ERR	"LTT : Can't create channel %s.\n",
+			chan_names[LTT_CHANNEL_INTERRUPTS]);
 		goto interrupts_error;
-	err = new_trace->ops->create_channel(trace_name, new_trace,
-			new_trace->dentry.control_root,
+	}
+
+	subbuf_size =
+		trace->setting.channels[LTT_CHANNEL_PROCESSES].subbuf_size;
+	subbuf_cnt = trace->setting.channels[LTT_CHANNEL_PROCESSES].subbuf_cnt;
+	prepare_chan_size_num(&subbuf_size, &subbuf_cnt,
+		LTT_DEFAULT_SUBBUF_SIZE_MED, LTT_DEFAULT_N_SUBBUFS_MED);
+	err = trace->ops->create_channel(trace_name, trace,
+			trace->dentry.control_root,
 			LTT_PROCESSES_CHANNEL,
-			&new_trace->channel.processes, subbuf_size_med,
-			n_subbufs_med,
-			is_channel_overwrite(LTT_CHANNEL_PROCESSES, mode));
-	if (err != 0)
+			&trace->channel.processes,
+			subbuf_size,
+			subbuf_cnt,
+			is_channel_overwrite(LTT_CHANNEL_PROCESSES,
+				trace->mode));
+	if (err != 0) {
+		printk(KERN_ERR	"LTT : Can't create channel %s.\n",
+			chan_names[LTT_CHANNEL_PROCESSES]);
 		goto processes_error;
-	err = new_trace->ops->create_channel(trace_name, new_trace,
-			new_trace->dentry.control_root,
+	}
+
+	subbuf_size = trace->setting.channels[LTT_CHANNEL_MODULES].subbuf_size;
+	subbuf_cnt = trace->setting.channels[LTT_CHANNEL_MODULES].subbuf_cnt;
+	prepare_chan_size_num(&subbuf_size, &subbuf_cnt,
+		LTT_DEFAULT_SUBBUF_SIZE_LOW, LTT_DEFAULT_N_SUBBUFS_LOW);
+	err = trace->ops->create_channel(trace_name, trace,
+			trace->dentry.control_root,
 			LTT_MODULES_CHANNEL,
-			&new_trace->channel.modules, subbuf_size_low,
-			n_subbufs_low,
-			is_channel_overwrite(LTT_CHANNEL_MODULES, mode));
-	if (err != 0)
+			&trace->channel.modules,
+			subbuf_size,
+			subbuf_cnt,
+			is_channel_overwrite(LTT_CHANNEL_MODULES, trace->mode));
+	if (err != 0) {
+		printk(KERN_ERR	"LTT : Can't create channel %s.\n",
+			chan_names[LTT_CHANNEL_MODULES]);
 		goto modules_error;
-	err = new_trace->ops->create_channel(trace_name, new_trace,
-			new_trace->dentry.control_root,
+	}
+
+	subbuf_size = trace->setting.channels[LTT_CHANNEL_NETWORK].subbuf_size;
+	subbuf_cnt = trace->setting.channels[LTT_CHANNEL_NETWORK].subbuf_cnt;
+	prepare_chan_size_num(&subbuf_size, &subbuf_cnt,
+		LTT_DEFAULT_SUBBUF_SIZE_LOW, LTT_DEFAULT_N_SUBBUFS_LOW);
+	err = trace->ops->create_channel(trace_name, trace,
+			trace->dentry.control_root,
 			LTT_NETWORK_CHANNEL,
-			&new_trace->channel.network, subbuf_size_low,
-			n_subbufs_low,
-			is_channel_overwrite(LTT_CHANNEL_NETWORK, mode));
-	if (err != 0)
+			&trace->channel.network,
+			subbuf_size,
+			subbuf_cnt,
+			is_channel_overwrite(LTT_CHANNEL_NETWORK, trace->mode));
+	if (err != 0) {
+		printk(KERN_ERR	"LTT : Can't create channel %s.\n",
+			chan_names[LTT_CHANNEL_NETWORK]);
 		goto network_error;
-	err = new_trace->ops->create_channel(trace_name, new_trace,
-			new_trace->dentry.trace_root,
+	}
+
+	subbuf_size = trace->setting.channels[LTT_CHANNEL_CPU].subbuf_size;
+	subbuf_cnt = trace->setting.channels[LTT_CHANNEL_CPU].subbuf_cnt;
+	prepare_chan_size_num(&subbuf_size, &subbuf_cnt,
+		LTT_DEFAULT_SUBBUF_SIZE_HIGH, LTT_DEFAULT_N_SUBBUFS_HIGH);
+	err = trace->ops->create_channel(trace_name, trace,
+			trace->dentry.trace_root,
 			LTT_CPU_CHANNEL,
-			&new_trace->channel.cpu, subbuf_size_high,
-			n_subbufs_high,
-			is_channel_overwrite(LTT_CHANNEL_CPU, mode));
-	if (err != 0)
+			&trace->channel.cpu,
+			subbuf_size,
+			subbuf_cnt,
+			is_channel_overwrite(LTT_CHANNEL_CPU, trace->mode));
+	if (err != 0) {
+		printk(KERN_ERR	"LTT : Can't create channel %s.\n",
+			chan_names[LTT_CHANNEL_CPU]);
 		goto cpu_error;
+	}
 
-	err = _ltt_trace_create(trace_name, mode, new_trace);
+	list_del(&trace->list);
+	if (list_empty(&ltt_traces.head)) {
+		probe_id_defrag();
+		mod_timer(&ltt_async_wakeup_timer,
+				jiffies + LTT_PERCPU_TIMER_INTERVAL);
+		set_kernel_trace_flag_all_tasks();
+	}
+	list_add_rcu(&trace->list, &ltt_traces.head);
+	synchronize_sched();
 
-	if (err != 0)
-		goto lock_create_error;
 	ltt_unlock_traces();
-	return err;
 
-lock_create_error:
-	new_trace->ops->remove_channel(new_trace->channel.cpu);
+	return 0;
+
 cpu_error:
-	new_trace->ops->remove_channel(new_trace->channel.network);
+	trace->ops->remove_channel(trace->channel.network);
 network_error:
-	new_trace->ops->remove_channel(new_trace->channel.modules);
+	trace->ops->remove_channel(trace->channel.modules);
 modules_error:
-	new_trace->ops->remove_channel(new_trace->channel.processes);
+	trace->ops->remove_channel(trace->channel.processes);
 processes_error:
-	new_trace->ops->remove_channel(new_trace->channel.interrupts);
+	trace->ops->remove_channel(trace->channel.interrupts);
 interrupts_error:
-	new_trace->ops->remove_channel(new_trace->channel.metadata);
+	trace->ops->remove_channel(trace->channel.metadata);
 metadata_error:
-	kref_put(&new_trace->ltt_transport_kref, ltt_release_transport);
+	trace->ops->remove_dirs(trace);
 dirs_error:
-	module_put(transport->owner);
-trace_error:
-	kref_put(&new_trace->kref, ltt_release_trace);
-	wake_up_interruptible(&new_trace->kref_wq);
-	ltt_unlock_traces();
+	module_put(trace->transport->owner);
 traces_error:
+	ltt_unlock_traces();
+	return err;
+}
+
+/*
+ * It is worked as a wrapper for current version of ltt_control.ko.
+ * We will make a new ltt_control based on debugfs, and control each channel's
+ * buffer.
+ */
+static int ltt_trace_create(const char *trace_name, const char *trace_type,
+		enum trace_mode mode,
+		unsigned subbuf_size_low, unsigned n_subbufs_low,
+		unsigned subbuf_size_med, unsigned n_subbufs_med,
+		unsigned subbuf_size_high, unsigned n_subbufs_high)
+{
+	int err = 0;
+
+	err = ltt_trace_setup(trace_name);
+	if (IS_ERR_VALUE(err))
+		return err;
+
+	err = ltt_trace_set_type(trace_name, trace_type);
+	if (IS_ERR_VALUE(err))
+		return err;
+
+	err = ltt_trace_set_mode(trace_name, mode);
+	if (IS_ERR_VALUE(err))
+		return err;
+
+	err = ltt_trace_set_channel_subbufsize(trace_name,
+		LTT_METADATA_CHANNEL, subbuf_size_low);
+	if (IS_ERR_VALUE(err))
+		return err;
+
+	err = ltt_trace_set_channel_subbufcount(trace_name,
+		LTT_METADATA_CHANNEL, n_subbufs_low);
+	if (IS_ERR_VALUE(err))
+		return err;
+
+	err = ltt_trace_set_channel_subbufsize(trace_name,
+		LTT_INTERRUPTS_CHANNEL, subbuf_size_low);
+	if (IS_ERR_VALUE(err))
+		return err;
+
+	err = ltt_trace_set_channel_subbufcount(trace_name,
+		LTT_INTERRUPTS_CHANNEL, n_subbufs_low);
+	if (IS_ERR_VALUE(err))
+		return err;
+
+	err = ltt_trace_set_channel_subbufsize(trace_name,
+		LTT_PROCESSES_CHANNEL, subbuf_size_med);
+	if (IS_ERR_VALUE(err))
+		return err;
+
+	err = ltt_trace_set_channel_subbufcount(trace_name,
+		LTT_PROCESSES_CHANNEL, n_subbufs_med);
+	if (IS_ERR_VALUE(err))
+		return err;
+
+	err = ltt_trace_set_channel_subbufsize(trace_name, LTT_MODULES_CHANNEL,
+		subbuf_size_low);
+	if (IS_ERR_VALUE(err))
+		return err;
+
+	err = ltt_trace_set_channel_subbufcount(trace_name, LTT_MODULES_CHANNEL,
+		n_subbufs_low);
+	if (IS_ERR_VALUE(err))
+		return err;
+
+	err = ltt_trace_set_channel_subbufsize(trace_name, LTT_NETWORK_CHANNEL,
+		subbuf_size_low);
+	if (IS_ERR_VALUE(err))
+		return err;
+
+	err = ltt_trace_set_channel_subbufcount(trace_name, LTT_NETWORK_CHANNEL,
+		n_subbufs_low);
+	if (IS_ERR_VALUE(err))
+		return err;
+
+	err = ltt_trace_set_channel_subbufsize(trace_name, LTT_CPU_CHANNEL,
+		subbuf_size_high);
+	if (IS_ERR_VALUE(err))
+		return err;
+
+	err = ltt_trace_set_channel_subbufcount(trace_name, LTT_CPU_CHANNEL,
+		n_subbufs_high);
+	if (IS_ERR_VALUE(err))
+		return err;
+
+	err = ltt_trace_alloc(trace_name);
+	if (IS_ERR_VALUE(err))
+		return err;
+
 	return err;
 }
 
@@ -817,6 +1127,12 @@ static void __exit ltt_exit(void)
 		_ltt_trace_destroy(trace);
 		__ltt_trace_destroy(trace);
 	}
+	/* free traces in pre-alloc status */
+	list_for_each_safe(pos, n, &ltt_traces.prealloc_head) {
+		trace = container_of(pos, struct ltt_trace_struct, list);
+		_ltt_trace_free(trace);
+	}
+
 	ltt_unlock_traces();
 }
 
-- 
1.5.5.3






^ permalink raw reply	[flat|nested] 8+ messages in thread

* [ltt-dev] [PATCH] Separate ltt_trace_create() into ltt_trace_create() and ltt_trace_alloc()
  2008-10-27  3:47             ` [ltt-dev] [PATCH] Separate ltt_trace_create() into ltt_trace_create() and ltt_trace_alloc() Zhaolei
@ 2008-10-28 14:25               ` Mathieu Desnoyers
  2008-10-29  4:04                 ` [ltt-dev] [PATCH] Separate ltt_trace_create() intoltt_trace_create() " Zhaolei
  0 siblings, 1 reply; 8+ messages in thread
From: Mathieu Desnoyers @ 2008-10-28 14:25 UTC (permalink / raw)


* Zhaolei (zhaolei at cn.fujitsu.com) wrote:
> This is step1 of:
> "switch marker list and marker activation (currently in /proc/ltt) to debugfs"
> 

Excellent start ! :)

I think you are in the right direction. See below,


> > >> Are you means we can control each channel's (interrupt, process, ...)
> > >> subbuf in future version?
> > >> (I think current version can only control low, med and high)
> > >> If this is true, maybe we need modify ltt_trace_create() before impl
> > >> ltt_control.
> > >>
> > > Yes, this is the goal.
> > So, IMHO, we can do it in following steps:
> >
> > Step1: modify ltt_trace_create(), .etc, separate ltt_trace_create() into
> >   ltt_trace_setup(), ltt_set_channelsize(), ltt_set_channelnum(),
> >   ltt_trace_alloc(). But still use old netlink_based ltt_control to make
> >   it simple.
> >   Although we modify ltt_set_channelsize() to support setting each channel's
> >   buffer, we make ltt_control.ko using old UI(only able to set low, med and
> >   high). So we don't need to modify other program such as user-mode lttctl.
> >
> 
> Yes. Just make sure a "destroy" deals correctly with a trace that is
> "setup" but not "allocated" yet.
> 
> > Step2: Write a new module named ltt-channel-control.
> >   ( I think name of ltt-channel-control will be better than ltt-control,
> >     because we already have a module named ltt-marker-control. )
> >   ltt-channel-control is designed on debugfs, as we discussed.
> >   We can test it by echo XX > /mnt/debugfs/ltt/...
> >
> 
> Yes, that would be perfect.
> 
> > Step3: modify user-mode programs to support ltt-channel-control.
> >   We also need to modify lttctl's command arguments, to support setup each
> >   channel's buffer. (Are we take out -Z, -X, -V, -B, -z, -x in lttctl?)
> >   I haven't read source of lttv, it is necessary to modify?
> >
> 
> Maybe we could replace lttctl by simple shell scripts ?
> 
> Yes, small modification of lttv would be required. There is a GUI module
> which executes lttctl to start/stop tracing. It would have to be
> modified so it uses the debugfs files instead.
> 
> Signed-off-by: Zhao Lei <zhaolei at cn.fujitsu.com>
> ---
>  include/linux/ltt-core.h   |    3 +-
>  include/linux/ltt-tracer.h |   40 ++--
>  ltt/ltt-core.c             |    1 +
>  ltt/ltt-tracer.c           |  560 ++++++++++++++++++++++++++++++++++----------
>  4 files changed, 464 insertions(+), 140 deletions(-)
> 
> diff --git a/include/linux/ltt-core.h b/include/linux/ltt-core.h
> index 1681e82..db890d8 100644
> --- a/include/linux/ltt-core.h
> +++ b/include/linux/ltt-core.h
> @@ -17,7 +17,8 @@
>   * list.
>   */
>  struct ltt_traces {
> -	struct list_head head;		/* Traces list */
> +	struct list_head prealloc_head;	/* Pre-allocated traces list */

pre-allocated sounds like "let's allocate 10 entries before the user
asks for the first one". I think "setup_head" would be a better name.

> +	struct list_head head;		/* Allocated Traces list */
>  	unsigned int num_active_traces;	/* Number of active traces */
>  } ____cacheline_aligned;
>  
> diff --git a/include/linux/ltt-tracer.h b/include/linux/ltt-tracer.h
> index d46bc88..042f518 100644
> --- a/include/linux/ltt-tracer.h
> +++ b/include/linux/ltt-tracer.h
> @@ -197,6 +197,23 @@ struct ltt_transport {
>  	struct ltt_trace_ops ops;
>  };
>  
> +/*
> + * First and last channels in ltt_trace_struct.
> + */
> +#define ltt_channel_index_size()	sizeof(struct ltt_channel_struct *)
> +#define ltt_channel_index_begin()	GET_CHANNEL_INDEX(cpu)
> +#define ltt_channel_index_end()	\
> +	(GET_CHANNEL_INDEX(metadata) + ltt_channel_index_size())
> +
> +enum ltt_channels {
> +	LTT_CHANNEL_CPU,
> +	LTT_CHANNEL_PROCESSES,
> +	LTT_CHANNEL_INTERRUPTS,
> +	LTT_CHANNEL_NETWORK,
> +	LTT_CHANNEL_MODULES,
> +	LTT_CHANNEL_METADATA,
> +	NR_LTT_CHANNELS,
> +};
>  
>  enum trace_mode { LTT_TRACE_NORMAL, LTT_TRACE_FLIGHT, LTT_TRACE_HYBRID };
>  
> @@ -218,6 +235,12 @@ struct ltt_trace_struct {
>  		struct ltt_channel_struct	*modules;
>  		struct ltt_channel_struct	*metadata;
>  	} channel;
> +	struct {
> +		struct {
> +			unsigned subbuf_size;
> +			unsigned subbuf_cnt;
> +		} channels[NR_LTT_CHANNELS];

You could probably put the
   struct ltt_channel_struct       *channel;

   in there.

> +	} setting;

setting seems not really useful here.

  struct ltt_channel_struct channel[NR_LTT_CHANNELS];
   
And adding subbuf_size and nr_subbufs fields to struct
ltt_channel_struct seems better.   

>  	u32 freq_scale;
>  	u64 start_freq;
>  	u64 start_tsc;
> @@ -236,23 +259,6 @@ struct ltt_trace_struct {
>  	char trace_name[NAME_MAX];
>  } ____cacheline_aligned;
>  
> -/*
> - * First and last channels in ltt_trace_struct.
> - */
> -#define ltt_channel_index_size()	sizeof(struct ltt_channel_struct *)
> -#define ltt_channel_index_begin()	GET_CHANNEL_INDEX(cpu)
> -#define ltt_channel_index_end()	\
> -	(GET_CHANNEL_INDEX(metadata) + ltt_channel_index_size())
> -
> -enum ltt_channels {
> -	LTT_CHANNEL_CPU,
> -	LTT_CHANNEL_PROCESSES,
> -	LTT_CHANNEL_INTERRUPTS,
> -	LTT_CHANNEL_NETWORK,
> -	LTT_CHANNEL_MODULES,
> -	LTT_CHANNEL_METADATA,
> -};
> -
>  /* Hardcoded event headers
>   *
>   * event header for a trace with active heartbeat : 27 bits timestamps
> diff --git a/ltt/ltt-core.c b/ltt/ltt-core.c
> index abd3d6d..3d7015e 100644
> --- a/ltt/ltt-core.c
> +++ b/ltt/ltt-core.c
> @@ -12,6 +12,7 @@
>  
>  /* Traces structures */
>  struct ltt_traces ltt_traces = {
> +	.prealloc_head = LIST_HEAD_INIT(ltt_traces.prealloc_head),
>  	.head = LIST_HEAD_INIT(ltt_traces.head),
>  };
>  EXPORT_SYMBOL(ltt_traces);
> diff --git a/ltt/ltt-tracer.c b/ltt/ltt-tracer.c
> index e4358cf..5eae2b5 100644
> --- a/ltt/ltt-tracer.c
> +++ b/ltt/ltt-tracer.c
> @@ -268,31 +268,20 @@ static struct ltt_trace_struct *_ltt_trace_find(const char *trace_name)
>  	return NULL;
>  }
>  
> -/* This function must be called with traces semaphore held. */
> -static int _ltt_trace_create(const char *trace_name, enum trace_mode mode,
> -				struct ltt_trace_struct *new_trace)
> +/* _ltt_trace_find_setup :
> + * find a trace in setup list by given name.
> + *
> + * Returns a pointer to the trace structure, NULL if not found.
> + */
> +static struct ltt_trace_struct *_ltt_trace_find_setup(const char *trace_name)
>  {
> -	int err = EPERM;
> +	struct ltt_trace_struct *trace;
>  
> -	if (_ltt_trace_find(trace_name) != NULL) {
> -		printk(KERN_ERR "LTT : Trace %s already exists\n", trace_name);
> -		err = EEXIST;
> -		goto traces_error;
> -	}
> -	if (list_empty(&ltt_traces.head)) {
> -		probe_id_defrag();
> -		mod_timer(&ltt_async_wakeup_timer,
> -				jiffies + LTT_PERCPU_TIMER_INTERVAL);
> -		set_kernel_trace_flag_all_tasks();
> -	}
> -	list_add_rcu(&new_trace->list, &ltt_traces.head);
> -	synchronize_sched();
> -	/* Everything went fine, finish creation */
> -	return 0;
> +	list_for_each_entry(trace, &ltt_traces.prealloc_head, list)
> +		if (!strncmp(trace->trace_name, trace_name, NAME_MAX))
> +			return trace;
>  
> -	/* Error handling */
> -traces_error:
> -	return err;
> +	return NULL;
>  }
>  
>  /**
> @@ -334,170 +323,491 @@ static inline void prepare_chan_size_num(unsigned *subbuf_size,
>  	WARN_ON(hweight32(*n_subbufs) != 1);
>  }
>  
> -static int ltt_trace_create(const char *trace_name, const char *trace_type,
> -		enum trace_mode mode,
> -		unsigned subbuf_size_low, unsigned n_subbufs_low,
> -		unsigned subbuf_size_med, unsigned n_subbufs_med,
> -		unsigned subbuf_size_high, unsigned n_subbufs_high)
> +static int ltt_trace_setup(const char *trace_name)
>  {
>  	int err = 0;
> -	struct ltt_trace_struct *new_trace, *trace;
> -	unsigned long flags;
> -	struct ltt_transport *tran, *transport = NULL;
> +	struct ltt_trace_struct *new_trace = NULL;
>  
> -	prepare_chan_size_num(&subbuf_size_low, &n_subbufs_low,
> -		LTT_DEFAULT_SUBBUF_SIZE_LOW, LTT_DEFAULT_N_SUBBUFS_LOW);
> +	ltt_lock_traces();
>  
> -	prepare_chan_size_num(&subbuf_size_med, &n_subbufs_med,
> -		LTT_DEFAULT_SUBBUF_SIZE_MED, LTT_DEFAULT_N_SUBBUFS_MED);
> +	if (_ltt_trace_find_setup(trace_name)) {
> +		printk(KERN_ERR	"LTT : Trace name %s already used.\n",
> +				trace_name);
> +		err = -EEXIST;
> +		goto traces_error;
> +	}
>  
> -	prepare_chan_size_num(&subbuf_size_high, &n_subbufs_high,
> -		LTT_DEFAULT_SUBBUF_SIZE_HIGH, LTT_DEFAULT_N_SUBBUFS_HIGH);
> +	if (_ltt_trace_find(trace_name)) {
> +		printk(KERN_ERR	"LTT : Trace name %s already used.\n",
> +				trace_name);
> +		err = -EEXIST;
> +		goto traces_error;
> +	}
>  
>  	new_trace = kzalloc(sizeof(struct ltt_trace_struct), GFP_KERNEL);
>  	if (!new_trace) {
>  		printk(KERN_ERR
>  			"LTT : Unable to allocate memory for trace %s\n",
>  			trace_name);
> -		err = ENOMEM;
> -		goto traces_error;
> +		err = -ENOMEM;
> +		goto trace_free;
>  	}
>  
> -	kref_init(&new_trace->kref);
> -	kref_init(&new_trace->ltt_transport_kref);
> -	init_waitqueue_head(&new_trace->kref_wq);
> -	new_trace->active = 0;
>  	strncpy(new_trace->trace_name, trace_name, NAME_MAX);
> -	new_trace->mode = mode;
> -	new_trace->freq_scale = trace_clock_freq_scale();
> +	list_add(&new_trace->list, &ltt_traces.prealloc_head);
> +
> +	ltt_unlock_traces();
> +
> +	return 0;
> +
> +trace_free:
> +	kfree(new_trace);
> +traces_error:
> +	ltt_unlock_traces();
> +	return err;
> +}
> +
> +/* must be called from within a traces lock. */
> +static void _ltt_trace_free(struct ltt_trace_struct *trace)
> +{
> +	list_del(&trace->list);
> +	kfree(trace);
> +}
> +
> +static int ltt_trace_set_type(const char *trace_name, const char *trace_type)
> +{
> +	int err = 0;
> +	struct ltt_trace_struct *trace;
> +	struct ltt_transport *tran_iter, *transport = NULL;
>  
>  	ltt_lock_traces();
> -	list_for_each_entry(tran, &ltt_transport_list, node) {
> -		if (!strcmp(tran->name, trace_type)) {
> -			transport = tran;
> +
> +	trace = _ltt_trace_find_setup(trace_name);
> +	if (!trace) {
> +		printk(KERN_ERR "LTT : Trace not found %s\n", trace_name);
> +		err = -ENOENT;
> +		goto traces_error;
> +	}
> +
> +	list_for_each_entry(tran_iter, &ltt_transport_list, node) {
> +		if (!strcmp(tran_iter->name, trace_type)) {
> +			transport = tran_iter;
>  			break;
>  		}
>  	}
> -
>  	if (!transport) {
> -		err = EINVAL;
>  		printk(KERN_ERR	"LTT : Transport %s is not present.\n",
>  			trace_type);
> -		ltt_unlock_traces();
> -		goto trace_error;
> +		err = -EINVAL;
> +		goto traces_error;
>  	}
>  
> -	if (!try_module_get(transport->owner)) {
> -		err = ENODEV;
> -		printk(KERN_ERR	"LTT : Can't lock transport module.\n");
> -		ltt_unlock_traces();
> -		goto trace_error;
> +	trace->transport = transport;
> +
> +traces_error:
> +	ltt_unlock_traces();
> +	return err;
> +}
> +
> +static int ltt_trace_set_mode(const char *trace_name, enum trace_mode mode)
> +{
> +	int err = 0;
> +	struct ltt_trace_struct *trace;
> +
> +	ltt_lock_traces();
> +
> +	trace = _ltt_trace_find_setup(trace_name);
> +	if (!trace) {
> +		printk(KERN_ERR "LTT : Trace not found %s\n", trace_name);
> +		err = -ENOENT;
> +		goto traces_error;
>  	}
>  
> -	trace = _ltt_trace_find(trace_name);
> -	if (trace) {
> -		printk(KERN_ERR	"LTT : Trace name %s already used.\n",
> -			trace_name);
> -		err = EEXIST;
> -		goto trace_error;
> +	trace->mode = mode;
> +
> +traces_error:
> +	ltt_unlock_traces();
> +	return err;
> +}
> +
> +/*
> + * Todo:
> + * Make similar function in channel.c,
> + * so it will be useful for both ltt-tracer.c and ltt-marker-control.c
> + */
> +/*
> + * Its order is MUST be same with enum ltt_channels
> + */
> +static char *chan_names[] = {
> +	LTT_CPU_CHANNEL,
> +	LTT_PROCESSES_CHANNEL,
> +	LTT_INTERRUPTS_CHANNEL,
> +	LTT_NETWORK_CHANNEL,
> +	LTT_MODULES_CHANNEL,
> +	LTT_METADATA_CHANNEL,
> +};
> +

If you add a const char *name field into struct ltt_channel_struct, you won't
need this.

> +static enum ltt_channels get_channel_type_from_name(const char *name)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(chan_names); i++)
> +		if (!strcmp(name, chan_names[i]))
> +			return (enum ltt_channels)i;
> +
> +	return NR_LTT_CHANNELS;
> +}
> +
> +static int ltt_trace_set_channel_subbufsize(const char *trace_name,
> +		const char *channel_name, unsigned size)
> +{
> +	int err = 0;
> +	struct ltt_trace_struct *trace;
> +	enum ltt_channels channel;
> +
> +	ltt_lock_traces();
> +
> +	trace = _ltt_trace_find_setup(trace_name);
> +	if (!trace) {
> +		printk(KERN_ERR "LTT : Trace not found %s\n", trace_name);
> +		err = -ENOENT;
> +		goto traces_error;
>  	}
>  
> -	new_trace->transport = transport;
> -	new_trace->ops = &transport->ops;
> +	channel = get_channel_type_from_name(channel_name);
> +	if (channel == NR_LTT_CHANNELS) {
> +		printk(KERN_ERR "LTT : Channel %s is not present.\n",
> +			channel_name);
> +		err = -EINVAL;
> +		goto traces_error;
> +	}
>  
> -	err = new_trace->ops->create_dirs(new_trace);
> -	if (err)
> +	trace->setting.channels[channel].subbuf_size = size;
> +
> +traces_error:
> +	ltt_unlock_traces();
> +	return err;
> +}
> +
> +static int ltt_trace_set_channel_subbufcount(const char *trace_name,
> +		const char *channel_name, unsigned cnt)
> +{
> +	int err = 0;
> +	struct ltt_trace_struct *trace;
> +	enum ltt_channels channel;
> +
> +	ltt_lock_traces();
> +
> +	trace = _ltt_trace_find_setup(trace_name);
> +	if (!trace) {
> +		printk(KERN_ERR "LTT : Trace not found %s\n", trace_name);
> +		err = -ENOENT;
> +		goto traces_error;
> +	}
> +
> +	channel = get_channel_type_from_name(channel_name);
> +	if (channel == NR_LTT_CHANNELS) {
> +		printk(KERN_ERR "LTT : Channel %s is not present.\n",
> +			channel_name);
> +		err = -EINVAL;
> +		goto traces_error;
> +	}
> +
> +	trace->setting.channels[channel].subbuf_cnt = cnt;
> +
> +traces_error:
> +	ltt_unlock_traces();
> +	return err;
> +}
> +
> +static int ltt_trace_alloc(const char *trace_name)
> +{
> +	int err = 0;
> +	struct ltt_trace_struct *trace;
> +	int subbuf_size, subbuf_cnt;
> +	unsigned long flags;
> +
> +	ltt_lock_traces();
> +
> +	trace = _ltt_trace_find_setup(trace_name);
> +	if (!trace) {
> +		printk(KERN_ERR "LTT : Trace not found %s\n", trace_name);
> +		err = -ENOENT;
> +		goto traces_error;
> +	}
> +
> +	kref_init(&trace->kref);
> +	kref_init(&trace->ltt_transport_kref);
> +	init_waitqueue_head(&trace->kref_wq);
> +	trace->active = 0;
> +	trace->freq_scale = trace_clock_freq_scale();
> +
> +	if (!trace->transport) {
> +		printk(KERN_ERR "LTT : Transport is not set.\n");
> +		err = -EINVAL;
> +		goto traces_error;
> +	}
> +	if (!try_module_get(trace->transport->owner)) {
> +		printk(KERN_ERR	"LTT : Can't lock transport module.\n");
> +		err = -ENODEV;
> +		goto traces_error;
> +	}
> +	trace->ops = &trace->transport->ops;
> +
> +	err = trace->ops->create_dirs(trace);
> +	if (err) {
> +		printk(KERN_ERR	"LTT : Can't create dir for trace %s.\n",
> +			trace_name);
>  		goto dirs_error;
> +	}
>  
>  	local_irq_save(flags);
> -	new_trace->start_freq = trace_clock_frequency();
> -	new_trace->start_tsc = trace_clock_read64();
> -	do_gettimeofday(&new_trace->start_time);
> +	trace->start_freq = trace_clock_frequency();
> +	trace->start_tsc = trace_clock_read64();
> +	do_gettimeofday(&trace->start_time);
>  	local_irq_restore(flags);
>  
>  	/*
> +	 * Todo:
> +	 *   Config each channel's default buffersize/cnt, instead of
> +	 *   LTT_DEFAULT_SUBBUF_SIZE_LOW, ...
> +	 */
> +

Those below should really be turned into a loop once you can simply
iterate on the channel [] elements in the trace setup structure.


Mathieu

> +	/*
>  	 * Always put the metadata channel in non-overwrite mode :
>  	 * This is a very low traffic channel and it can't afford to have its
>  	 * data overwritten : this data (marker info) is necessary to be
>  	 * able to read the trace.
>  	 */
> -	err = new_trace->ops->create_channel(trace_name, new_trace,
> -			new_trace->dentry.control_root,
> +	subbuf_size = trace->setting.channels[LTT_CHANNEL_METADATA].subbuf_size;
> +	subbuf_cnt = trace->setting.channels[LTT_CHANNEL_METADATA].subbuf_cnt;
> +	prepare_chan_size_num(&subbuf_size, &subbuf_cnt,
> +		LTT_DEFAULT_SUBBUF_SIZE_LOW, LTT_DEFAULT_N_SUBBUFS_LOW);
> +	err = trace->ops->create_channel(trace_name, trace,
> +			trace->dentry.control_root,
>  			LTT_METADATA_CHANNEL,
> -			&new_trace->channel.metadata, subbuf_size_low,
> -			n_subbufs_low,
> -			is_channel_overwrite(LTT_CHANNEL_METADATA, mode));
> -	if (err != 0)
> +			&trace->channel.metadata,
> +			subbuf_size,
> +			subbuf_cnt,
> +			is_channel_overwrite(LTT_CHANNEL_METADATA,
> +				trace->mode));
> +	if (err != 0) {
> +		printk(KERN_ERR	"LTT : Can't create channel %s.\n",
> +			chan_names[LTT_CHANNEL_METADATA]);
>  		goto metadata_error;
> -	err = new_trace->ops->create_channel(trace_name, new_trace,
> -			new_trace->dentry.control_root,
> +	}
> +
> +	subbuf_size =
> +		trace->setting.channels[LTT_CHANNEL_INTERRUPTS].subbuf_size;
> +	subbuf_cnt = trace->setting.channels[LTT_CHANNEL_INTERRUPTS].subbuf_cnt;
> +	prepare_chan_size_num(&subbuf_size, &subbuf_cnt,
> +		LTT_DEFAULT_SUBBUF_SIZE_LOW, LTT_DEFAULT_N_SUBBUFS_LOW);
> +	err = trace->ops->create_channel(trace_name, trace,
> +			trace->dentry.control_root,
>  			LTT_INTERRUPTS_CHANNEL,
> -			&new_trace->channel.interrupts, subbuf_size_low,
> -			n_subbufs_low,
> -			is_channel_overwrite(LTT_CHANNEL_INTERRUPTS, mode));
> -	if (err != 0)
> +			&trace->channel.interrupts,
> +			subbuf_size,
> +			subbuf_cnt,
> +			is_channel_overwrite(LTT_CHANNEL_INTERRUPTS,
> +				trace->mode));
> +	if (err != 0) {
> +		printk(KERN_ERR	"LTT : Can't create channel %s.\n",
> +			chan_names[LTT_CHANNEL_INTERRUPTS]);
>  		goto interrupts_error;
> -	err = new_trace->ops->create_channel(trace_name, new_trace,
> -			new_trace->dentry.control_root,
> +	}
> +
> +	subbuf_size =
> +		trace->setting.channels[LTT_CHANNEL_PROCESSES].subbuf_size;
> +	subbuf_cnt = trace->setting.channels[LTT_CHANNEL_PROCESSES].subbuf_cnt;
> +	prepare_chan_size_num(&subbuf_size, &subbuf_cnt,
> +		LTT_DEFAULT_SUBBUF_SIZE_MED, LTT_DEFAULT_N_SUBBUFS_MED);
> +	err = trace->ops->create_channel(trace_name, trace,
> +			trace->dentry.control_root,
>  			LTT_PROCESSES_CHANNEL,
> -			&new_trace->channel.processes, subbuf_size_med,
> -			n_subbufs_med,
> -			is_channel_overwrite(LTT_CHANNEL_PROCESSES, mode));
> -	if (err != 0)
> +			&trace->channel.processes,
> +			subbuf_size,
> +			subbuf_cnt,
> +			is_channel_overwrite(LTT_CHANNEL_PROCESSES,
> +				trace->mode));
> +	if (err != 0) {
> +		printk(KERN_ERR	"LTT : Can't create channel %s.\n",
> +			chan_names[LTT_CHANNEL_PROCESSES]);
>  		goto processes_error;
> -	err = new_trace->ops->create_channel(trace_name, new_trace,
> -			new_trace->dentry.control_root,
> +	}
> +
> +	subbuf_size = trace->setting.channels[LTT_CHANNEL_MODULES].subbuf_size;
> +	subbuf_cnt = trace->setting.channels[LTT_CHANNEL_MODULES].subbuf_cnt;
> +	prepare_chan_size_num(&subbuf_size, &subbuf_cnt,
> +		LTT_DEFAULT_SUBBUF_SIZE_LOW, LTT_DEFAULT_N_SUBBUFS_LOW);
> +	err = trace->ops->create_channel(trace_name, trace,
> +			trace->dentry.control_root,
>  			LTT_MODULES_CHANNEL,
> -			&new_trace->channel.modules, subbuf_size_low,
> -			n_subbufs_low,
> -			is_channel_overwrite(LTT_CHANNEL_MODULES, mode));
> -	if (err != 0)
> +			&trace->channel.modules,
> +			subbuf_size,
> +			subbuf_cnt,
> +			is_channel_overwrite(LTT_CHANNEL_MODULES, trace->mode));
> +	if (err != 0) {
> +		printk(KERN_ERR	"LTT : Can't create channel %s.\n",
> +			chan_names[LTT_CHANNEL_MODULES]);
>  		goto modules_error;
> -	err = new_trace->ops->create_channel(trace_name, new_trace,
> -			new_trace->dentry.control_root,
> +	}
> +
> +	subbuf_size = trace->setting.channels[LTT_CHANNEL_NETWORK].subbuf_size;
> +	subbuf_cnt = trace->setting.channels[LTT_CHANNEL_NETWORK].subbuf_cnt;
> +	prepare_chan_size_num(&subbuf_size, &subbuf_cnt,
> +		LTT_DEFAULT_SUBBUF_SIZE_LOW, LTT_DEFAULT_N_SUBBUFS_LOW);
> +	err = trace->ops->create_channel(trace_name, trace,
> +			trace->dentry.control_root,
>  			LTT_NETWORK_CHANNEL,
> -			&new_trace->channel.network, subbuf_size_low,
> -			n_subbufs_low,
> -			is_channel_overwrite(LTT_CHANNEL_NETWORK, mode));
> -	if (err != 0)
> +			&trace->channel.network,
> +			subbuf_size,
> +			subbuf_cnt,
> +			is_channel_overwrite(LTT_CHANNEL_NETWORK, trace->mode));
> +	if (err != 0) {
> +		printk(KERN_ERR	"LTT : Can't create channel %s.\n",
> +			chan_names[LTT_CHANNEL_NETWORK]);
>  		goto network_error;
> -	err = new_trace->ops->create_channel(trace_name, new_trace,
> -			new_trace->dentry.trace_root,
> +	}
> +
> +	subbuf_size = trace->setting.channels[LTT_CHANNEL_CPU].subbuf_size;
> +	subbuf_cnt = trace->setting.channels[LTT_CHANNEL_CPU].subbuf_cnt;
> +	prepare_chan_size_num(&subbuf_size, &subbuf_cnt,
> +		LTT_DEFAULT_SUBBUF_SIZE_HIGH, LTT_DEFAULT_N_SUBBUFS_HIGH);
> +	err = trace->ops->create_channel(trace_name, trace,
> +			trace->dentry.trace_root,
>  			LTT_CPU_CHANNEL,
> -			&new_trace->channel.cpu, subbuf_size_high,
> -			n_subbufs_high,
> -			is_channel_overwrite(LTT_CHANNEL_CPU, mode));
> -	if (err != 0)
> +			&trace->channel.cpu,
> +			subbuf_size,
> +			subbuf_cnt,
> +			is_channel_overwrite(LTT_CHANNEL_CPU, trace->mode));
> +	if (err != 0) {
> +		printk(KERN_ERR	"LTT : Can't create channel %s.\n",
> +			chan_names[LTT_CHANNEL_CPU]);
>  		goto cpu_error;
> +	}
>  
> -	err = _ltt_trace_create(trace_name, mode, new_trace);
> +	list_del(&trace->list);
> +	if (list_empty(&ltt_traces.head)) {
> +		probe_id_defrag();
> +		mod_timer(&ltt_async_wakeup_timer,
> +				jiffies + LTT_PERCPU_TIMER_INTERVAL);
> +		set_kernel_trace_flag_all_tasks();
> +	}
> +	list_add_rcu(&trace->list, &ltt_traces.head);
> +	synchronize_sched();
>  
> -	if (err != 0)
> -		goto lock_create_error;
>  	ltt_unlock_traces();
> -	return err;
>  
> -lock_create_error:
> -	new_trace->ops->remove_channel(new_trace->channel.cpu);
> +	return 0;
> +
>  cpu_error:
> -	new_trace->ops->remove_channel(new_trace->channel.network);
> +	trace->ops->remove_channel(trace->channel.network);
>  network_error:
> -	new_trace->ops->remove_channel(new_trace->channel.modules);
> +	trace->ops->remove_channel(trace->channel.modules);
>  modules_error:
> -	new_trace->ops->remove_channel(new_trace->channel.processes);
> +	trace->ops->remove_channel(trace->channel.processes);
>  processes_error:
> -	new_trace->ops->remove_channel(new_trace->channel.interrupts);
> +	trace->ops->remove_channel(trace->channel.interrupts);
>  interrupts_error:
> -	new_trace->ops->remove_channel(new_trace->channel.metadata);
> +	trace->ops->remove_channel(trace->channel.metadata);
>  metadata_error:
> -	kref_put(&new_trace->ltt_transport_kref, ltt_release_transport);
> +	trace->ops->remove_dirs(trace);
>  dirs_error:
> -	module_put(transport->owner);
> -trace_error:
> -	kref_put(&new_trace->kref, ltt_release_trace);
> -	wake_up_interruptible(&new_trace->kref_wq);
> -	ltt_unlock_traces();
> +	module_put(trace->transport->owner);
>  traces_error:
> +	ltt_unlock_traces();
> +	return err;
> +}
> +
> +/*
> + * It is worked as a wrapper for current version of ltt_control.ko.
> + * We will make a new ltt_control based on debugfs, and control each channel's
> + * buffer.
> + */
> +static int ltt_trace_create(const char *trace_name, const char *trace_type,
> +		enum trace_mode mode,
> +		unsigned subbuf_size_low, unsigned n_subbufs_low,
> +		unsigned subbuf_size_med, unsigned n_subbufs_med,
> +		unsigned subbuf_size_high, unsigned n_subbufs_high)
> +{
> +	int err = 0;
> +
> +	err = ltt_trace_setup(trace_name);
> +	if (IS_ERR_VALUE(err))
> +		return err;
> +
> +	err = ltt_trace_set_type(trace_name, trace_type);
> +	if (IS_ERR_VALUE(err))
> +		return err;
> +
> +	err = ltt_trace_set_mode(trace_name, mode);
> +	if (IS_ERR_VALUE(err))
> +		return err;
> +
> +	err = ltt_trace_set_channel_subbufsize(trace_name,
> +		LTT_METADATA_CHANNEL, subbuf_size_low);
> +	if (IS_ERR_VALUE(err))
> +		return err;
> +
> +	err = ltt_trace_set_channel_subbufcount(trace_name,
> +		LTT_METADATA_CHANNEL, n_subbufs_low);
> +	if (IS_ERR_VALUE(err))
> +		return err;
> +
> +	err = ltt_trace_set_channel_subbufsize(trace_name,
> +		LTT_INTERRUPTS_CHANNEL, subbuf_size_low);
> +	if (IS_ERR_VALUE(err))
> +		return err;
> +
> +	err = ltt_trace_set_channel_subbufcount(trace_name,
> +		LTT_INTERRUPTS_CHANNEL, n_subbufs_low);
> +	if (IS_ERR_VALUE(err))
> +		return err;
> +
> +	err = ltt_trace_set_channel_subbufsize(trace_name,
> +		LTT_PROCESSES_CHANNEL, subbuf_size_med);
> +	if (IS_ERR_VALUE(err))
> +		return err;
> +
> +	err = ltt_trace_set_channel_subbufcount(trace_name,
> +		LTT_PROCESSES_CHANNEL, n_subbufs_med);
> +	if (IS_ERR_VALUE(err))
> +		return err;
> +
> +	err = ltt_trace_set_channel_subbufsize(trace_name, LTT_MODULES_CHANNEL,
> +		subbuf_size_low);
> +	if (IS_ERR_VALUE(err))
> +		return err;
> +
> +	err = ltt_trace_set_channel_subbufcount(trace_name, LTT_MODULES_CHANNEL,
> +		n_subbufs_low);
> +	if (IS_ERR_VALUE(err))
> +		return err;
> +
> +	err = ltt_trace_set_channel_subbufsize(trace_name, LTT_NETWORK_CHANNEL,
> +		subbuf_size_low);
> +	if (IS_ERR_VALUE(err))
> +		return err;
> +
> +	err = ltt_trace_set_channel_subbufcount(trace_name, LTT_NETWORK_CHANNEL,
> +		n_subbufs_low);
> +	if (IS_ERR_VALUE(err))
> +		return err;
> +
> +	err = ltt_trace_set_channel_subbufsize(trace_name, LTT_CPU_CHANNEL,
> +		subbuf_size_high);
> +	if (IS_ERR_VALUE(err))
> +		return err;
> +
> +	err = ltt_trace_set_channel_subbufcount(trace_name, LTT_CPU_CHANNEL,
> +		n_subbufs_high);
> +	if (IS_ERR_VALUE(err))
> +		return err;
> +
> +	err = ltt_trace_alloc(trace_name);
> +	if (IS_ERR_VALUE(err))
> +		return err;
> +
>  	return err;
>  }
>  
> @@ -817,6 +1127,12 @@ static void __exit ltt_exit(void)
>  		_ltt_trace_destroy(trace);
>  		__ltt_trace_destroy(trace);
>  	}
> +	/* free traces in pre-alloc status */
> +	list_for_each_safe(pos, n, &ltt_traces.prealloc_head) {
> +		trace = container_of(pos, struct ltt_trace_struct, list);
> +		_ltt_trace_free(trace);
> +	}
> +
>  	ltt_unlock_traces();
>  }
>  
> -- 
> 1.5.5.3
> 
> 
> 
> _______________________________________________
> ltt-dev mailing list
> ltt-dev at lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
> 

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [ltt-dev] [PATCH] Separate ltt_trace_create() intoltt_trace_create() and ltt_trace_alloc()
  2008-10-28 14:25               ` Mathieu Desnoyers
@ 2008-10-29  4:04                 ` Zhaolei
  0 siblings, 0 replies; 8+ messages in thread
From: Zhaolei @ 2008-10-29  4:04 UTC (permalink / raw)


* Mathieu Desnoyers <compudj at krystal.dyndns.org> wrote:
>* Zhaolei (zhaolei at cn.fujitsu.com) wrote:
>> This is step1 of:
>> "switch marker list and marker activation (currently in /proc/ltt) to debugfs"
>> 
> 
> Excellent start ! :)
> 
> I think you are in the right direction. See below,
Hello, Mathieu

Thanks for your suggestion.
We will fix step1 on your suggestion and continue to do step2.

But I have some question below:

>>  struct ltt_traces {
>> - struct list_head head; /* Traces list */
>> + struct list_head prealloc_head; /* Pre-allocated traces list */
> 
> pre-allocated sounds like "let's allocate 10 entries before the user
> asks for the first one". I think "setup_head" would be a better name.
Thanks, I agree.

>> @@ -218,6 +235,12 @@ struct ltt_trace_struct {
>>  struct ltt_channel_struct *modules;
>>  struct ltt_channel_struct *metadata;
>>  } channel;
>> + struct {
>> + struct {
>> + unsigned subbuf_size;
>> + unsigned subbuf_cnt;
>> + } channels[NR_LTT_CHANNELS];
> 
> You could probably put the
>   struct ltt_channel_struct       *channel;
> 
>   in there.
> 
>> + } setting;
> 
> setting seems not really useful here.
> 
>  struct ltt_channel_struct channel[NR_LTT_CHANNELS];
>   
> And adding subbuf_size and nr_subbufs fields to struct
> ltt_channel_struct seems better.   
But channel is allocated in other function as ltt_relay_create_channel:
static int ltt_relay_create_channel(char *trace_name,
 struct ltt_trace_struct *trace, struct dentry *dir,
 char *channel_name, struct ltt_channel_struct **ltt_chan,
 unsigned int subbuf_size, unsigned int n_subbufs,
 int overwrite)
{
 ...
 *ltt_chan = kzalloc(sizeof(struct ltt_channel_struct), GFP_KERNEL);
 ...

If we define struct ltt_channel_struct channel[NR_LTT_CHANNELS](now is pointer)
We need to modify ltt_relay_create_channel and other functions and make
 things complex.

So, IMHO, we can use current struct at present, and select to impl your idea
 after "switch marker list and marker activation to debugfs" finished.

>> +/*
>> + * Todo:
>> + * Make similar function in channel.c,
>> + * so it will be useful for both ltt-tracer.c and ltt-marker-control.c
>> + */
>> +/*
>> + * Its order is MUST be same with enum ltt_channels
>> + */
>> +static char *chan_names[] = {
>> + LTT_CPU_CHANNEL,
>> + LTT_PROCESSES_CHANNEL,
>> + LTT_INTERRUPTS_CHANNEL,
>> + LTT_NETWORK_CHANNEL,
>> + LTT_MODULES_CHANNEL,
>> + LTT_METADATA_CHANNEL,
>> +};
>> +
> 
> If you add a const char *name field into struct ltt_channel_struct, you won't
> need this.
It have relation with prev qusetion.
If we don't define struct ltt_channel_struct channel[NR_LTT_CHANNELS]
 in ltt_trace_struct, we need to get channel-name from channel-id by this struct.
But it is a temp struct, because it is better to use a unifed struct for both
 ltt-tracer.c and ltt-marker-control.c

> > Those below should really be turned into a loop once you can simply
> iterate on the channel [] elements in the trace setup structure.
Thanks, I agree.

> 
> 
> Mathieu
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2008-10-29  4:04 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <48F733E6.9010007@cn.fujitsu.com>
2008-10-17  3:56 ` [ltt-dev] TODO list before releasing LTTng-buffering to LKML] Zhaolei
2008-10-17  3:56   ` Zhaolei
2008-10-17  3:56   ` Zhaolei
     [not found]   ` <20081017042045.GB5067@Krystal>
2008-10-17  6:31     ` Zhaolei
     [not found]       ` <20081017161429.GA5696@Krystal>
2008-10-20  3:03         ` Zhaolei
     [not found]           ` <20081020031634.GA14209@Krystal>
2008-10-27  3:47             ` [ltt-dev] [PATCH] Separate ltt_trace_create() into ltt_trace_create() and ltt_trace_alloc() Zhaolei
2008-10-28 14:25               ` Mathieu Desnoyers
2008-10-29  4:04                 ` [ltt-dev] [PATCH] Separate ltt_trace_create() intoltt_trace_create() " Zhaolei

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox