[ltt-dev] [PATCH v2 1/3] Separate ltt_trace_create() intoltt_trace_create() and ltt_trace_alloc()
Mathieu Desnoyers
compudj at krystal.dyndns.org
Wed Nov 12 00:01:48 EST 2008
* Zhaolei (zhaolei at cn.fujitsu.com) wrote:
> * 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.
>
We could even think of a :
struct ltt_channel_struct *channels;
And then we can allocate dynamically a array containing elements of
type struct ltt_channel_struct. This would permit new channels to be
created without having to hardcode them.
Mathieu
> 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
> >
> >
> _______________________________________________
> 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