* Nils Carlson ([email protected]) wrote:
> Make the locking around the transport list more intelligent, give
> it its own mutex.
>
> Signed-off-by: Nils Carlson <[email protected]>
Hi Nils,
When adding this kind of new mutex, can you:
a) explain why it is needed (or what the advantage is)
b) comment, beside the mutex, what the nesting of the mutex with
respect to other is.
In the past, I went for a very fine-grained locking scheme in the LTTng
control internals, only to find out that it complexified the code
needlessly and added race conditions here and there. At that point, I
went back to the big hammer (lock_traces()).
As long as adding more mutexes does not change anything in terms of
end-user scalability experience, I don't see any reason to add more.
Thanks,
Mathieu
> ---
> libust/tracer.c | 13 ++++++++-----
> 1 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/libust/tracer.c b/libust/tracer.c
> index 100dec0..ceb5b74 100644
> --- a/libust/tracer.c
> +++ b/libust/tracer.c
> @@ -182,7 +182,7 @@ static enum ltt_channels get_channel_type_from_name(const
> char *name)
> //ust// }
>
> static CDS_LIST_HEAD(ltt_transport_list);
> -
> +static DEFINE_MUTEX(ltt_transport_mutex);
> /**
> * ltt_transport_register - LTT transport registration
> * @transport: transport structure
> @@ -204,9 +204,9 @@ void ltt_transport_register(struct ltt_transport
> *transport)
> */
> //ust// vmalloc_sync_all();
>
> - ltt_lock_traces();
> + pthread_mutex_lock(<t_transport_mutex);
> cds_list_add_tail(&transport->node, <t_transport_list);
> - ltt_unlock_traces();
> + pthread_mutex_unlock(<t_transport_mutex);
> }
>
> /**
> @@ -215,9 +215,9 @@ void ltt_transport_register(struct ltt_transport
> *transport)
> */
> void ltt_transport_unregister(struct ltt_transport *transport)
> {
> - ltt_lock_traces();
> + pthread_mutex_lock(<t_transport_mutex);
> cds_list_del(&transport->node);
> - ltt_unlock_traces();
> + pthread_mutex_unlock(<t_transport_mutex);
> }
>
> static inline int is_channel_overwrite(enum ltt_channels chan,
> @@ -458,12 +458,15 @@ int ltt_trace_set_type(const char *trace_name, const
> char *trace_type)
> goto traces_error;
> }
>
> + pthread_mutex_lock(<t_transport_mutex);
> cds_list_for_each_entry(tran_iter, <t_transport_list, node) {
> if (!strcmp(tran_iter->name, trace_type)) {
> transport = tran_iter;
> break;
> }
> }
> + pthread_mutex_unlock(<t_transport_mutex);
> +
> if (!transport) {
> ERR("Transport %s is not present", trace_type);
> err = -EINVAL;
> --
> 1.7.1
>
>
> _______________________________________________
> ltt-dev mailing list
> [email protected]
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
>
--
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
_______________________________________________
ltt-dev mailing list
[email protected]
http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
