[ltt-dev] [PATCH v2 1/3] Separate ltt_trace_create() intoltt_trace_create() and ltt_trace_alloc()
Zhaolei
zhaolei at cn.fujitsu.com
Tue Nov 11 01:51:58 EST 2008
* From: "Mathieu Desnoyers" <compudj at krystal.dyndns.org>
> Hi Zhao Lei,
>
> Nice work, somme comments below,
Hello, Mathieu
Thanks for your advice.
> You can probably remove the channel_index, because it is known through
> offsetof within the array.
I agree it.
>
>> + 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.
Your way is better.
I will change source to this way.
>> + for (chan--; chan >= 0; chan--)
>> + trace->ops->remove_channel(*(struct ltt_channel_struct **)
>> + ((char *)trace + chan_infos[chan].channel_index));
>
> That looks a bit weird. Is there a neater way to do this without that
> many casts ?
I think it looks a bit weird too.
But I can't find out a better way.
Except we modify ltt_trace_struct to following:
struct {
struct ltt_channel_struct *cpu;
struct ltt_channel_struct *processes;
struct ltt_channel_struct *interrupts;
struct ltt_channel_struct *network;
struct ltt_channel_struct *modules;
struct ltt_channel_struct *metadata;
} channel;
--->
struct ltt_channel_struct *channels[NR_LTT_CHANNELS];
Then we can get it simper.
B.R.
Zhaolei
>
> Thanks,
>
> Mathieu
>
>> +
>> 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);
>> +transport_error:
>> put_trace_clock();
>> traces_error:
>> + ltt_unlock_traces();
>> + return err;
>> +}
>> +EXPORT_SYMBOL_GPL(ltt_trace_alloc);
>> +
>> +/*
>> + * 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;
>> }
>>
>> @@ -581,26 +841,42 @@ static void __ltt_trace_destroy(struct ltt_trace_struct *trace)
>> kref_put(&trace->kref, ltt_release_trace);
>> }
>>
>> -static int ltt_trace_destroy(const char *trace_name)
>> +int ltt_trace_destroy(const char *trace_name)
>> {
>> int err = 0;
>> struct ltt_trace_struct *trace;
>>
>> ltt_lock_traces();
>> +
>> trace = _ltt_trace_find(trace_name);
>> - err = _ltt_trace_destroy(trace);
>> - if (err)
>> - goto error;
>> - ltt_unlock_traces();
>> - __ltt_trace_destroy(trace);
>> - put_trace_clock();
>> - return err;
>> + if (trace) {
>> + err = _ltt_trace_destroy(trace);
>> + if (err)
>> + goto error;
>> +
>> + ltt_unlock_traces();
>> +
>> + __ltt_trace_destroy(trace);
>> + put_trace_clock();
>> +
>> + return 0;
>> + }
>> +
>> + trace = _ltt_trace_find_setup(trace_name);
>> + if (trace) {
>> + _ltt_trace_free(trace);
>> + ltt_unlock_traces();
>> + return 0;
>> + }
>> +
>> + err = ENOENT;
>>
>> /* Error handling */
>> error:
>> ltt_unlock_traces();
>> return err;
>> }
>> +EXPORT_SYMBOL_GPL(ltt_trace_destroy);
>>
>> /* must be called from within a traces lock. */
>> static int _ltt_trace_start(struct ltt_trace_struct *trace)
>> @@ -630,7 +906,7 @@ traces_error:
>> return err;
>> }
>>
>> -static int ltt_trace_start(const char *trace_name)
>> +int ltt_trace_start(const char *trace_name)
>> {
>> int err = 0;
>> struct ltt_trace_struct *trace;
>> @@ -670,6 +946,7 @@ no_trace:
>> ltt_unlock_traces();
>> return err;
>> }
>> +EXPORT_SYMBOL_GPL(ltt_trace_start);
>>
>> /* must be called from within traces lock */
>> static int _ltt_trace_stop(struct ltt_trace_struct *trace)
>> @@ -697,7 +974,7 @@ traces_error:
>> return err;
>> }
>>
>> -static int ltt_trace_stop(const char *trace_name)
>> +int ltt_trace_stop(const char *trace_name)
>> {
>> int err = 0;
>> struct ltt_trace_struct *trace;
>> @@ -708,6 +985,7 @@ static int ltt_trace_stop(const char *trace_name)
>> ltt_unlock_traces();
>> return err;
>> }
>> +EXPORT_SYMBOL_GPL(ltt_trace_stop);
>>
>> /**
>> * ltt_control : Trace control in-kernel API
>> @@ -820,6 +1098,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, <t_traces.setup_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
>
>
More information about the lttng-dev
mailing list