[ltt-dev] [PATCH v3 0/4] Separate ltt_trace_create() into ltt_trace_create() and ltt_trace_alloc()

[Mathieu Desnoyers]

* Zhaolei (zhaolei at cn.fujitsu.com) wrote:
> Hello, I modified patchset of "Separate ltt_trace_create() into
> ltt_trace_create() and ltt_trace_alloc()" based on your advice.
> 
> >> -static struct dentry *ltt_root_dentry;
> >> +struct dentry *ltt_root_dentry;
> >> +EXPORT_SYMBOL_GPL(ltt_root_dentry);
> >> +
> > 
> > Hrm, I think it will cause a double declaration of ltt_root_dentry if
> > both ltt-relay and ltt-relay-locked are loaded. ltt_root_dentry should
> > probably sit in ltt-core.c.
> ltt_root_dentry is moved into ltt-core.c.
> But I think it is ltt-core.c's duty to create ltt-root in debugfs(not ltt-relay)
> ltt-relay and ltt-relay-locked's dir should be in ltt-root, so dir should be:
> debugfs/
> `-- ltt
>     |-- control
>     |   `-- ...
>     |-- relay
>     |   `-- ...
>     `-- relay-locked
>         `-- ...
> Maybe it will also affect user-mode tools, I'd like to impl it after new
> ltt-trace-control get merged.
> 

OK, as long as we don't have duplicate definitions for ltt_root_dentry,
we can proceed incrementally.

> >> + unsigned int def_subbufsize;
> >> + unsigned int def_subbufcount;
> >> +} chan_infos[] = {
> > 
> > A declaration like this seems to work :
> > 
> > 
> > int blah[] = {
> >        [0] = 1,
> >        [1] = 2,
> >        [5] = 3,
> >        [4] = 4,
> > };
> > 
> > So I think you can populate the array by using :
> > 
> > struct chan_info_struct {
> > const char *name;
> > unsigned int channel_index;
> > unsigned int def_subbufsize;
> > unsigned int def_subbufcount;
> > } chan_infos[] = {
> >  [GET_CHANNEL_INDEX(cpu)] = {
> >    LTT_CPU_CHANNEL,
> >    LTT_DEFAULT_SUBBUF_SIZE_HIGH,
> >    LTT_DEFAULT_N_SUBBUFS_HIGH,
> >  },
> >  .........
> > };
> > 
> > So you don't have to force the correct declaration order.
> Thanks, I changed source.
> But I think "[LTT_CHANNEL_CPU] = {" is better, because using
> GET_CHANNEL_INDEX() as array's index will leave a lot of blank items as:
> [0] = [BLANK]
> [1] = [BLANK]
> [2] = [BLANK]
> [3] = [BLANK]
> [4] = [BLANK]
> [5] = [BLANK]
> [6] = [BLANK]
> [7] = [BLANK]
> [8] = [BLANK]
> [9] = [BLANK]
> [10] = [BLANK]
> [11] = [BLANK]
> [12] = [BLANK]
> [13] = [BLANK]
> [14] = [BLANK]
> [15] = [BLANK]
> [16] = [CPU's item]
> [17] = [BLANK]
> [18] = [BLANK]
> [19] = [BLANK]
> [20] = [PROCESSES's item]
> [21] = [BLANK]
> [22] = [BLANK]
> [23] = [BLANK]
> ...
> And make search and iter operation complex.
> 

Ah, yes, that was not my intent. Your solution is appropriate.

> Per-channel's overwrite control function is added.
> I'll post new ltt-trace-control in next mail.
> 
> After we merge this patch, we need to do following work:
> 1: Change user-mode tools to support debugfs-based control interface.
>    (now we support both control interface)
> 2: Remove old netlink-based code from source.

Yes, I'll go through a final review before the merge, I think we should
be able to complete this soon. :)

Thanks,

Mathieu

> 
> Thanks!
> 
> 
> _______________________________________________
> 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