[RFC Patch Ust 1/5] Update data structures to support CTF global structures

[RFC Patch Ust 1/5] Update data structures to support CTF global structures

[Geneviève Bastien]

The structures match those in LTTng-tools to support global structures.

The struct lttng_structure contains the global types required by the
structure, but not ustctl_structure because global types will be flattened
in the event description when serializing.

Signed-off-by: Geneviève Bastien <gbastien+lttng@versatic.net>
---
 include/lttng/ust-ctl.h    | 14 ++++++++++++++
 include/lttng/ust-events.h | 17 +++++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/include/lttng/ust-ctl.h b/include/lttng/ust-ctl.h
index ba3a271..708501d 100644
--- a/include/lttng/ust-ctl.h
+++ b/include/lttng/ust-ctl.h
@@ -265,6 +265,7 @@ enum ustctl_abstract_types {
 	ustctl_atype_sequence,
 	ustctl_atype_string,
 	ustctl_atype_float,
+	ustctl_atype_structure,
 	NR_USTCTL_ABSTRACT_TYPES,
 };
 
@@ -335,6 +336,9 @@ struct ustctl_type {
 			struct ustctl_basic_type length_type;
 			struct ustctl_basic_type elem_type;
 		} sequence;
+		struct {
+			char name[LTTNG_UST_SYM_NAME_LEN];
+		} structure;
 		char padding[USTCTL_UST_TYPE_PADDING];
 	} u;
 } LTTNG_PACKED;
@@ -348,9 +352,18 @@ struct ustctl_enum {
 	char padding[USTCTL_UST_ENUM_TYPE_PADDING];
 } LTTNG_PACKED;
 
+#define USTCTL_UST_STRUCT_TYPE_PADDING	368
+struct ustctl_structure {
+	char name[LTTNG_UST_SYM_NAME_LEN];
+	size_t nr_fields;
+	struct ustctl_field *fields;
+	char padding[USTCTL_UST_STRUCT_TYPE_PADDING];
+};
+
 /* CTF categories for global types declared outside event descriptions */
 enum ustctl_global_type_categories {
 	ustctl_mtype_enum,
+	ustctl_mtype_structure,
 	NR_USTCTL_GLOBAL_TYPES,
 };
 
@@ -359,6 +372,7 @@ struct ustctl_global_type_decl {
 	uint32_t mtype;
 	union {
 		struct ustctl_enum ctf_enum;
+		struct ustctl_structure ctf_structure;
 		char padding[USTCTL_UST_GLOBAL_TYPE_DECL_PADDING];
 	} u;
 } LTTNG_PACKED;
diff --git a/include/lttng/ust-events.h b/include/lttng/ust-events.h
index 6f11e84..89155ae 100644
--- a/include/lttng/ust-events.h
+++ b/include/lttng/ust-events.h
@@ -82,6 +82,7 @@ enum lttng_abstract_types {
 	atype_sequence,
 	atype_string,
 	atype_float,
+	atype_structure,
 	NR_ABSTRACT_TYPES,
 };
 
@@ -96,6 +97,7 @@ enum lttng_string_encodings {
 /* CTF categories for global types declared outside event descriptions */
 enum lttng_global_type_categories {
 	mtype_enum,
+	mtype_structure,
 	NR_GLOBAL_TYPES,
 };
 
@@ -207,6 +209,9 @@ struct lttng_type {
 			struct lttng_basic_type length_type;
 			struct lttng_basic_type elem_type;
 		} sequence;
+		struct {
+			const char *name;
+		} structure;
 		char padding[LTTNG_UST_TYPE_PADDING];
 	} u;
 };
@@ -220,12 +225,24 @@ struct lttng_enum {
 	char padding[LTTNG_UST_ENUM_TYPE_PADDING];
 };
 
+#define LTTNG_UST_STRUCT_TYPE_PADDING 20
+struct lttng_structure {
+	const char *name;
+	size_t nr_fields;
+	const struct lttng_event_field *fields;
+	/* Global types required by this structure. */
+	unsigned int nr_global_type_decl;
+	const struct lttng_global_type_decl *global_type_decl;
+	char padding[LTTNG_UST_STRUCT_TYPE_PADDING];
+};
+
 #define LTTNG_UST_GLOBAL_TYPES_PADDING	60
 struct lttng_global_type_decl {
 	enum lttng_global_type_categories mtype;
 	unsigned int nowrite; /* inherited from the field using it */
 	union {
 		const struct lttng_enum *ctf_enum;
+		const struct lttng_structure *ctf_structure;
 		char padding[LTTNG_UST_GLOBAL_TYPES_PADDING];
 	} u;
 };
-- 
1.9.1


_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

[RFC Patch Ust 2/5] Serialize the CTF global structures for ust-comm

[Geneviève Bastien]

It flatten the global types so that global types used by other global types
all belong to the event at serialization. This avoids recursive calls to the
send and receive functions and simplifies error handling (no recursive frees).

Signed-off-by: Geneviève Bastien <gbastien+lttng@versatic.net>
---
 liblttng-ust-comm/lttng-ust-comm.c | 170 +++++++++++++++++++++++++++++++++----
 liblttng-ust-ctl/ustctl.c          |  26 ++++++
 2 files changed, 179 insertions(+), 17 deletions(-)

diff --git a/liblttng-ust-comm/lttng-ust-comm.c b/liblttng-ust-comm/lttng-ust-comm.c
index bc22130..90274c4 100644
--- a/liblttng-ust-comm/lttng-ust-comm.c
+++ b/liblttng-ust-comm/lttng-ust-comm.c
@@ -742,6 +742,7 @@ int serialize_basic_type(enum ustctl_abstract_types *uatype,
 	}
 	case atype_array:
 	case atype_sequence:
+	case atype_structure:
 	default:
 		return -EINVAL;
 	}
@@ -800,6 +801,14 @@ int serialize_one_type(struct ustctl_type *ut, const struct lttng_type *lt)
 		ut->atype = ustctl_atype_sequence;
 		break;
 	}
+	case atype_structure:
+	{
+		strncpy(ut->u.structure.name, lt->u.structure.name,
+						LTTNG_UST_SYM_NAME_LEN);
+		ut->u.structure.name[LTTNG_UST_SYM_NAME_LEN - 1] = '\0';
+		ut->atype = ustctl_atype_structure;
+		break;
+	}
 	default:
 		return -EINVAL;
 	}
@@ -892,25 +901,82 @@ int serialize_enum(struct ustctl_enum *uenum,
 }
 
 static
-int serialize_global_type_decl(size_t *_nr_write_global_type_decl,
-		struct ustctl_global_type_decl **ustctl_global_type,
+int serialize_structure(struct ustctl_structure *ustruct,
+		const struct lttng_structure *lstruct)
+{
+	int ret;
+
+	strncpy(ustruct->name, lstruct->name, LTTNG_UST_SYM_NAME_LEN);
+	ustruct->name[LTTNG_UST_SYM_NAME_LEN - 1] = '\0';
+
+	/* Serialize the fields */
+	if (lstruct->nr_fields > 0) {
+		ret = serialize_fields(&ustruct->nr_fields, &ustruct->fields,
+				lstruct->nr_fields,	lstruct->fields);
+		if (ret) {
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static size_t get_child_global_type_count(size_t nr_global_type_decl,
+		const struct lttng_global_type_decl *lttng_global_type)
+{
+	size_t total_global_types = 0;
+	int i;
+	const struct lttng_global_type_decl *lg;
+
+	for (i = 0; i < nr_global_type_decl; i++) {
+		lg = &lttng_global_type[i];
+
+		if (lg->nowrite)
+			continue;
+
+		/* Count the global types in children as well */
+		switch (lg->mtype) {
+		case mtype_structure:
+		{
+			const struct lttng_structure *ls;
+
+			ls = lg->u.ctf_structure;
+			/* Add the number of global types it contains and the count of its own children */
+			if (ls->nr_global_type_decl > 0) {
+				total_global_types += ls->nr_global_type_decl;
+				total_global_types += get_child_global_type_count(ls->nr_global_type_decl,
+								ls->global_type_decl);
+			}
+			break;
+		}
+		case mtype_enum:
+		default:
+			break;
+		}
+	}
+
+	return total_global_types;
+}
+
+/*
+ * When this method is called, index is the next index to write to and the
+ * ustctl_global_type structure is already initialized and will not overflow.
+ *
+ * The global type declarations are flattened at serialization to avoid
+ * needing to do recursive frees on error.
+ */
+static int _serialize_global_type_decl(size_t *index,
+		struct ustctl_global_type_decl *global_type_decl,
 		size_t nr_global_type_decl,
 		const struct lttng_global_type_decl *lttng_global_type)
 {
-	struct ustctl_global_type_decl *global_type_decl;
 	int i, ret;
-	size_t nr_write_global_type_decl = 0;
-
-	global_type_decl = zmalloc(nr_global_type_decl
-					* sizeof(*global_type_decl));
-	if (!global_type_decl)
-		return -ENOMEM;
 
 	for (i = 0; i < nr_global_type_decl; i++) {
 		struct ustctl_global_type_decl *f;
 		const struct lttng_global_type_decl *lf;
 
-		f = &global_type_decl[nr_write_global_type_decl];
+		f = &global_type_decl[*index];
 		lf = &lttng_global_type[i];
 
 		/* skip 'nowrite' fields */
@@ -928,18 +994,66 @@ int serialize_global_type_decl(size_t *_nr_write_global_type_decl,
 			le = lf->u.ctf_enum;
 			ret = serialize_enum(ue, le);
 			if (ret)
-				goto error;
+				return ret;
 
 			f->mtype = ustctl_mtype_enum;
 			break;
 		}
+		case mtype_structure:
+		{
+			struct ustctl_structure *us;
+			const struct lttng_structure *ls;
+
+			/* Serialize children global types first */
+			ls = lf->u.ctf_structure;
+			ret = _serialize_global_type_decl(index,
+					global_type_decl,
+					ls->nr_global_type_decl,
+					ls->global_type_decl);
+			if (ret)
+				return ret;
+
+			/* Reinitialize f since the index may have changed */
+			f = &global_type_decl[*index];
+			us = &f->u.ctf_structure;
+
+			ret = serialize_structure(us, ls);
+			if (ret)
+				return ret;
+
+			f->mtype = ustctl_mtype_structure;
+			break;
+		}
 		default:
-			ret = -EINVAL;
-			goto error;
+			return -EINVAL;
 		}
 
-		nr_write_global_type_decl++;
+		*index = *index + 1;
 	}
+	return 0;
+}
+
+static
+int serialize_global_type_decl(size_t *_nr_write_global_type_decl,
+		struct ustctl_global_type_decl **ustctl_global_type,
+		size_t nr_global_type_decl,
+		const struct lttng_global_type_decl *lttng_global_type)
+{
+	struct ustctl_global_type_decl *global_type_decl;
+	int i, ret;
+	size_t nr_write_global_type_decl = 0;
+	size_t nr_child_global_type_cnt = get_child_global_type_count(nr_global_type_decl,
+							lttng_global_type);
+
+	global_type_decl = zmalloc((nr_global_type_decl + nr_child_global_type_cnt)
+					* sizeof(*global_type_decl));
+	if (!global_type_decl)
+		return -ENOMEM;
+
+	ret = _serialize_global_type_decl(&nr_write_global_type_decl, global_type_decl,
+			nr_global_type_decl, lttng_global_type);
+	if (ret)
+		goto error;
 
 	*_nr_write_global_type_decl = nr_write_global_type_decl;
 	*ustctl_global_type = global_type_decl;
@@ -954,6 +1068,9 @@ error:
 		case ustctl_mtype_enum:
 			free(m->u.ctf_enum.entries);
 			break;
+		case ustctl_mtype_structure:
+			free(m->u.ctf_structure.fields);
+			break;
 		default:
 			break;
 		}
@@ -1158,15 +1275,31 @@ int ustcomm_register_event(int sock,
 				}
 				break;
 			}
+			case ustctl_mtype_structure:
+			{
+				int field_len = one_global_type->u.ctf_structure.nr_fields * sizeof(*one_global_type->u.ctf_structure.fields);
+
+				/* Send the fields */
+				DBG("Sending fields for global type structure %s.\n",
+						one_global_type->u.ctf_structure.name);
+				len = ustcomm_send_unix_sock(sock, one_global_type->u.ctf_structure.fields, field_len);
+				free(one_global_type->u.ctf_structure.fields);
+				one_global_type->u.ctf_structure.fields = NULL;
+				if (len > 0 && len != field_len) {
+					goto error_global_type;
+				}
+				if (len < 0) {
+					goto error_global_type;
+				}
+				break;
+			}
 			default:
 				break;
 			}
 		}
-		free(global_type_decl);
 
-	} else {
-		free(global_type_decl);
 	}
+	free(global_type_decl);
 
 	/* receive reply */
 	len = ustcomm_recv_unix_sock(sock, &reply, sizeof(reply));
@@ -1212,6 +1345,9 @@ error_global_type:
 		case ustctl_mtype_enum:
 			free(one_global_type->u.ctf_enum.entries);
 			break;
+		case ustctl_mtype_structure:
+			free(one_global_type->u.ctf_structure.fields);
+			break;
 		default:
 			break;
 		}
diff --git a/liblttng-ust-ctl/ustctl.c b/liblttng-ust-ctl/ustctl.c
index a181e62..3100f1b 100644
--- a/liblttng-ust-ctl/ustctl.c
+++ b/liblttng-ust-ctl/ustctl.c
@@ -1896,6 +1896,32 @@ int ustctl_recv_register_event(int sock,
 				}
 				break;
 			}
+			case ustctl_mtype_structure:
+			{
+				int entry_len = one_global_type->u.ctf_structure.nr_fields * sizeof(*one_global_type->u.ctf_structure.fields);
+				/* Receive the entries */
+				one_global_type->u.ctf_structure.fields = zmalloc(entry_len);
+				if (!one_global_type->u.ctf_structure.fields) {
+					len = -ENOMEM;
+					goto global_type_error;
+				}
+				len = ustcomm_recv_unix_sock(sock,
+						one_global_type->u.ctf_structure.fields,
+						entry_len);
+				DBG("Received fields for struct %s.\n", one_global_type->u.ctf_structure.name);
+				if (len > 0 && len != entry_len) {
+					len = -EIO;
+					goto global_type_error;
+				}
+				if (len == 0) {
+					len = -EPIPE;
+					goto global_type_error;
+				}
+				if (len < 0) {
+					goto global_type_error;
+				}
+				break;
+			}
 			default:
 				break;
 			}
-- 
1.9.1


_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

[RFC Patch Ust 3/5] Add the macros to generate the data structures for CTF global structures

[Geneviève Bastien]

The structure global type is very similar to an event, and thus reuse most of
the macros for the event fields.

Signed-off-by: Geneviève Bastien <gbastien+lttng@versatic.net>
---
 include/lttng/tracepoint.h                   |  67 ++++++++++++++
 include/lttng/ust-tracepoint-event-nowrite.h |   4 +
 include/lttng/ust-tracepoint-event-reset.h   |  12 +++
 include/lttng/ust-tracepoint-event-write.h   |   4 +
 include/lttng/ust-tracepoint-event.h         | 128 ++++++++++++++++++++++++++-
 5 files changed, 211 insertions(+), 4 deletions(-)

diff --git a/include/lttng/tracepoint.h b/include/lttng/tracepoint.h
index 27aa01c..4b2e85f 100644
--- a/include/lttng/tracepoint.h
+++ b/include/lttng/tracepoint.h
@@ -466,6 +466,70 @@ __tracepoints__ptrs_destroy(void)
 
 #endif /* #ifndef TRACEPOINT_ENUM */
 
+#ifndef TRACEPOINT_STRUCT
+
+/*
+ * Tracepoint Structures
+ *
+ * Structures are compound types used to group fields together. These types
+ * can be reused by different tracepoints.
+ *
+ * An example:
+ *
+ * TRACEPOINT_STRUCT(someproject_component, structname,
+ *
+ *	   * TP_ARGS takes from 0 to 10 "type, field_name" pairs *
+ *
+ *     TP_ARGS(int, arg0, void *, arg1, char *, string, size_t, strlen,
+ *             long *, arg4, size_t, arg4_len),
+ *
+ *     * TP_FIELDS describes the structure payload layout in the trace *
+ *
+ *     TP_FIELDS(
+ *         * Integer, printed in base 10 *
+ *         ctf_integer(int, field_a, arg0)
+ *
+ *         * Integer, printed with 0x base 16 *
+ *         ctf_integer_hex(unsigned long, field_d, arg1)
+ *
+ *         * Enumeration *
+ *         ctf_enum(component, name, field_e, arg)
+ *
+ *         * Array Sequence, printed as UTF8-encoded array of bytes *
+ *         ctf_array_text(char, field_b, string, FIXED_LEN)
+ *         ctf_sequence_text(char, field_c, string, size_t, strlen)
+ *
+ *         * String, printed as UTF8-encoded string *
+ *         ctf_string(field_e, string)
+ *
+ *         * Array sequence of signed integer values *
+ *         ctf_array(long, field_f, arg4, FIXED_LEN4)
+ *         ctf_sequence(long, field_g, arg4, size_t, arg4_len)
+ *
+ *         * Structures *
+ *         ctf_struct(component, struct_name, field_s, args...)
+ *     )
+ * )
+ *
+ * Where "someproject_component" is the name of the component this structure
+ * belongs to and "structname" identifies this structure. The arguments and
+ * fields described after are the same as for an event. See the event
+ * documentation for more information.
+ *
+ * That structure can then be used in a field inside the TP_FIELD macro, either
+ * in another structure or in an event using the following line:
+ *
+ * ctf_struct(someproject_component, structname, field, arguments...)
+ *
+ * Where "someproject_component" and "structname" match those in the
+ * TRACEPOINT_STRUCT, "field" is the name of the field and the arguments
+ * correspond to what the TRACEPOINT_STRUCT receives.
+ */
+
+#define TRACEPOINT_STRUCT(provider, name, args, fields)
+
+#endif /* #ifndef TRACEPOINT_STRUCT */
+
 #ifndef TRACEPOINT_EVENT
 
 /*
@@ -502,6 +566,9 @@ __tracepoints__ptrs_destroy(void)
  *         * Array sequence of signed integer values * 
  *         ctf_array(long, field_f, arg4, FIXED_LEN4)
  *         ctf_sequence(long, field_g, arg4, size_t, arg4_len)
+ *
+ *         * Structures *
+ *         ctf_struct(component, struct_name, field_s, args...)
  *     )
  * )
  *
diff --git a/include/lttng/ust-tracepoint-event-nowrite.h b/include/lttng/ust-tracepoint-event-nowrite.h
index 7f5b1d9..68efb73 100644
--- a/include/lttng/ust-tracepoint-event-nowrite.h
+++ b/include/lttng/ust-tracepoint-event-nowrite.h
@@ -53,3 +53,7 @@
 #undef ctf_enum_nowrite
 #define ctf_enum_nowrite(_provider, _name, _item, _src)		\
 	_ctf_enum(_provider, _name, _item, _src, 1)
+
+#undef ctf_struct_nowrite
+#define ctf_struct_nowrite(_provider, _name, _item, _src...)	\
+	_ctf_struct(_provider, _name, _item, 1, _src)
diff --git a/include/lttng/ust-tracepoint-event-reset.h b/include/lttng/ust-tracepoint-event-reset.h
index 4b795c9..6342d25 100644
--- a/include/lttng/ust-tracepoint-event-reset.h
+++ b/include/lttng/ust-tracepoint-event-reset.h
@@ -31,6 +31,9 @@
 #undef TRACEPOINT_ENUM
 #define TRACEPOINT_ENUM(_provider, _name, _type, _values)
 
+#undef TRACEPOINT_STRUCT
+#define TRACEPOINT_STRUCT(_provider, _name, _args, _fields)
+
 #undef TP_ARGS
 #define TP_ARGS(...)
 
@@ -67,6 +70,9 @@
 #undef _ctf_enum
 #define _ctf_enum(_provider, _name, _item, _src, _nowrite)
 
+#undef _ctf_struct
+#define _ctf_struct(_provider, _name, _item, _nowrite, _src...)
+
 #undef ctf_enum_integer
 #define ctf_enum_integer(_type)
 
@@ -104,6 +110,9 @@
 #undef ctf_enum
 #define ctf_enum(_provider, _name, _item, _src)
 
+#undef ctf_struct
+#define ctf_struct(_provider, _name, _item, _src...)
+
 /* "nowrite" */
 #undef ctf_integer_nowrite
 #define ctf_integer_nowrite(_type, _item, _src)
@@ -128,3 +137,6 @@
 
 #undef ctf_enum_nowrite
 #define ctf_enum_nowrite(_provider, _name, _item, _src)
+
+#undef ctf_struct_nowrite
+#define ctf_struct_nowrite(_provider, _name, _item, _src...)
diff --git a/include/lttng/ust-tracepoint-event-write.h b/include/lttng/ust-tracepoint-event-write.h
index 7dd06ed..4844a72 100644
--- a/include/lttng/ust-tracepoint-event-write.h
+++ b/include/lttng/ust-tracepoint-event-write.h
@@ -65,3 +65,7 @@
 #undef ctf_enum
 #define ctf_enum(_provider, _name, _item, _src)			\
 	_ctf_enum(_provider, _name, _item, _src, 0)
+
+#undef ctf_struct
+#define ctf_struct(_provider, _name, _item, _src...)	\
+	_ctf_struct(_provider, _name, _item, 0, _src)
diff --git a/include/lttng/ust-tracepoint-event.h b/include/lttng/ust-tracepoint-event.h
index c75ed73..9c351a3 100644
--- a/include/lttng/ust-tracepoint-event.h
+++ b/include/lttng/ust-tracepoint-event.h
@@ -255,6 +255,21 @@ static const char							\
 		  .encoding = lttng_encode_none,		\
 	}
 
+#undef _ctf_struct
+#define _ctf_struct(_provider, _name, _item, _nowrite, _src...)		\
+		{							\
+		.name = #_item,						\
+		.type = {						\
+			.atype = atype_structure,			\
+			.u = {						\
+				.structure = {				\
+					.name = #_provider "_" #_name,	\
+				},					\
+			},						\
+		},							\
+		.nowrite = _nowrite,					\
+	}
+
 #undef TP_FIELDS
 #define TP_FIELDS(...) __VA_ARGS__	/* Only one used in this phase */
 
@@ -273,6 +288,13 @@ static const char							\
 		.len = _TP_ARRAY_SIZE(__enum_values__##_provider##_##_name), \
 	};
 
+#undef TRACEPOINT_STRUCT
+#define TRACEPOINT_STRUCT(_provider, _name, _args, _fields)		\
+	static const struct lttng_event_field __struct_fields___##_provider##___##_name[] = { \
+		_fields							\
+	};
+
+
 #include TRACEPOINT_INCLUDE
 
 /*
@@ -296,6 +318,16 @@ static const char							\
 		},							\
 	},
 
+#undef _ctf_struct
+#define _ctf_struct(_provider, _name, _item, _nowrite, _src...)		    \
+	{								    \
+		.mtype = mtype_structure,				    \
+		.nowrite = _nowrite,					    \
+		.u = {							    \
+			.ctf_structure = &__structure_##_provider##_##_name \
+		},							    \
+	}
+
 #undef TP_FIELDS
 #define TP_FIELDS(...) __VA_ARGS__	/* Only one used in this phase */
 
@@ -305,6 +337,19 @@ static const char							\
 		_fields						 \
 	};
 
+#undef TRACEPOINT_STRUCT
+#define TRACEPOINT_STRUCT(_provider, _name, _args, _fields)		\
+	static const struct lttng_global_type_decl __global_types_for_struct___##_provider##___##_name[] = { \
+		_fields							\
+	};								\
+	static const struct lttng_structure __structure_##_provider##_##_name = { \
+		.name = #_provider "_" #_name,				\
+		.nr_fields = _TP_ARRAY_SIZE(__struct_fields___##_provider##___##_name), \
+		.fields = __struct_fields___##_provider##___##_name,	\
+		.nr_global_type_decl = _TP_ARRAY_SIZE(__global_types_for_struct___##_provider##___##_name), \
+		.global_type_decl = __global_types_for_struct___##_provider##___##_name, \
+	};
+
 #include TRACEPOINT_INCLUDE
 
 /*
@@ -372,6 +417,10 @@ static void __event_probe__##_provider##___##_name(_TP_ARGS_DATA_PROTO(_args));
 #define ctf_enum_integer(_type)						       \
 	_type
 
+#undef _ctf_struct
+#define _ctf_struct(_provider, _name, _item, _nowrite, _src...)				   \
+	__event_len += __struct_get_size__##_provider##___##_name(__dynamic_len, __dynamic_len_idx, _src);
+
 #undef TP_ARGS
 #define TP_ARGS(...) __VA_ARGS__
 
@@ -405,6 +454,17 @@ size_t __enum_get_size__##_provider##___##_name(size_t __event_len)	      \
 	return __enum_len;						      \
 }
 
+#undef TRACEPOINT_STRUCT
+#define TRACEPOINT_STRUCT(_provider, _name, _args, _fields)		\
+static inline								\
+size_t __struct_get_size__##_provider##___##_name(size_t *__dynamic_len,\
+		unsigned int __dynamic_len_idx, _TP_ARGS_PROTO(_args)) \
+{									\
+	size_t __event_len = 0;						\
+	_fields								\
+	return __event_len;						\
+}
+
 #include TRACEPOINT_INCLUDE
 
 /*
@@ -638,6 +698,10 @@ void __event_prepare_filter_stack__##_provider##___##_name(char *__stack_data,\
 #define _ctf_enum(_provider, _name, _item, _src, _nowrite)		      \
 	__event_align = _tp_max_t(size_t, __event_align, __enum_get_align__##_provider##___##_name());
 
+#undef _ctf_struct
+#define _ctf_struct(_provider, _name, _item, _nowrite, _src...)				   \
+	__event_align = _tp_max_t(size_t, __event_align, __struct_get_align__##_provider##___##_name(_src));
+
 #undef TP_ARGS
 #define TP_ARGS(...) __VA_ARGS__
 
@@ -664,8 +728,43 @@ size_t __enum_get_align__##_provider##___##_name(void)			      \
 	return lttng_alignof(_type);					      \
 }
 
+#undef TRACEPOINT_STRUCT
+#define TRACEPOINT_STRUCT(_provider, _name, _args, _fields)		      \
+static inline								      \
+size_t __struct_get_align__##_provider##___##_name(_TP_ARGS_PROTO(_args))\
+{									      \
+	size_t __event_align = 1;					      \
+	_fields								      \
+	return __event_align;						      \
+}
+
 #include TRACEPOINT_INCLUDE
 
+/*
+ * State 4.9 of tracepoint event generation
+ *
+ * Get the maximum field count from event fields recursively
+ */
+/* Reset all macros within TRACEPOINT_EVENT */
+#include <lttng/ust-tracepoint-event-reset.h>
+#include <lttng/ust-tracepoint-event-write.h>
+
+#undef _ctf_struct
+#define _ctf_struct(_provider, _name, _item, _nowrite, _src...)		      \
+	+ __struct_event_count___##_provider##___##_name
+
+#undef TRACEPOINT_STRUCT
+#define TRACEPOINT_STRUCT(_provider, _name, _args, _fields)		      \
+static const size_t __struct_field_count___##_provider##___##_name =	      \
+	_TP_ARRAY_SIZE(__struct_fields___##_provider##___##_name) _fields;
+
+#undef TRACEPOINT_EVENT_CLASS
+#define TRACEPOINT_EVENT_CLASS(_provider, _name, _args, _fields)	      \
+static const size_t __event_field_count___##_provider##___##_name =	      \
+	_TP_ARRAY_SIZE(__event_fields___##_provider##___##_name) _fields;
+
+
+#include TRACEPOINT_INCLUDE
 
 /*
  * Stage 5 of tracepoint event generation.
@@ -703,7 +802,7 @@ size_t __enum_get_align__##_provider##___##_name(void)			      \
 #define _ctf_sequence_encoded(_type, _item, _src, _length_type,		\
 			_src_length, _encoding, _nowrite)		\
 	{								\
-		_length_type __tmpl = __stackvar.__dynamic_len[__dynamic_len_idx]; \
+		_length_type __tmpl = __dynamic_len[__dynamic_len_idx]; \
 		lib_ring_buffer_align_ctx(&__ctx, lttng_alignof(_length_type));\
 		__chan->ops->event_write(&__ctx, &__tmpl, sizeof(_length_type));\
 	}								\
@@ -790,9 +889,13 @@ size_t __enum_get_align__##_provider##___##_name(void)			      \
 	}								\
 }
 
+#undef _ctf_struct
+#define _ctf_struct(_provider, _name, _item, _nowrite, _src...)		\
+	__struct_probe__##_provider##___##_name(__chan, __ctx, __dynamic_len_idx, __dynamic_len, _src);
+
 /* Beware: this get len actually consumes the len value */
 #undef __get_dynamic_len
-#define __get_dynamic_len(field)	__stackvar.__dynamic_len[__dynamic_len_idx++]
+#define __get_dynamic_len(field)	__dynamic_len[__dynamic_len_idx++]
 
 #undef TP_ARGS
 #define TP_ARGS(...) __VA_ARGS__
@@ -801,6 +904,22 @@ size_t __enum_get_align__##_provider##___##_name(void)			      \
 #define TP_FIELDS(...) __VA_ARGS__
 
 /*
+ * Probe for structures will call this method
+ */
+#undef TRACEPOINT_STRUCT
+#define TRACEPOINT_STRUCT(_provider, _name, _args, _fields)		      \
+static inline								      \
+void __struct_probe__##_provider##___##_name(struct lttng_channel *__chan,  \
+	struct lttng_ust_lib_ring_buffer_ctx __ctx, size_t __dynamic_len_idx, \
+	size_t *__dynamic_len, _TP_ARGS_PROTO(_args))			      \
+{									      \
+	if (0) {							      \
+		(void) __dynamic_len;					      \
+	}								      \
+	_fields								      \
+}
+
+/*
  * For state dump, check that "session" argument (mandatory) matches the
  * session this event belongs to. Ensures that we write state dump data only
  * into the started session, not into all sessions.
@@ -832,9 +951,10 @@ void __event_probe__##_provider##___##_name(_TP_ARGS_DATA_PROTO(_args))	      \
 	size_t __event_len, __event_align;				      \
 	size_t __dynamic_len_idx = 0;					      \
 	union {								      \
-		size_t __dynamic_len[_TP_ARRAY_SIZE(__event_fields___##_provider##___##_name)]; \
+		size_t __dynamic_len[__event_field_count___##_provider##___##_name];			      \
 		char __filter_stack_data[2 * sizeof(unsigned long) * _TP_ARRAY_SIZE(__event_fields___##_provider##___##_name)]; \
 	} __stackvar;							      \
+	size_t *__dynamic_len = __stackvar.__dynamic_len;		      \
 	int __ret;							      \
 									      \
 	if (0)								      \
@@ -863,7 +983,7 @@ void __event_probe__##_provider##___##_name(_TP_ARGS_DATA_PROTO(_args))	      \
 		if (caa_likely(!__filter_record))			      \
 			return;						      \
 	}								      \
-	__event_len = __event_get_size__##_provider##___##_name(__stackvar.__dynamic_len, \
+	__event_len = __event_get_size__##_provider##___##_name(__dynamic_len, \
 		 _TP_ARGS_DATA_VAR(_args));				      \
 	__event_align = __event_get_align__##_provider##___##_name(_TP_ARGS_VAR(_args)); \
 	lib_ring_buffer_ctx_init(&__ctx, __chan->chan, __event, __event_len,  \
-- 
1.9.1


_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

[RFC Patch Ust 4/5] Update the ctf-global-type example to show the usage of a global structure

[Geneviève Bastien]

Signed-off-by: Geneviève Bastien <gbastien+lttng@versatic.net>
---
 tests/ctf-global-types/ctf-global-types.c          |  8 ++++++--
 .../ctf-global-types/ust_tests_ctf_global_types.h  | 22 +++++++++++++++++++++-
 2 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/tests/ctf-global-types/ctf-global-types.c b/tests/ctf-global-types/ctf-global-types.c
index 81b69e1..55c2ad0 100644
--- a/tests/ctf-global-types/ctf-global-types.c
+++ b/tests/ctf-global-types/ctf-global-types.c
@@ -25,6 +25,7 @@ int main(int argc, char **argv)
 {
 	int i;
 	int delay = 0;
+	char text[10] = "test";
 
 	if (argc == 2)
 		delay = atoi(argv[1]);
@@ -34,8 +35,11 @@ int main(int argc, char **argv)
 	sleep(delay);
 
 	fprintf(stderr, "Tracing... ");
-	for (i = 0; i < 100; i++) {
-		tracepoint(ust_tests_ctf_global_types, tptest, i, (i % 6));
+	for (i = 0; i < 20; i++) {
+		tracepoint(ust_tests_ctf_global_types, tptest, i, (i % 6), text, strlen(text));
+		if (i == 10) {
+			strcpy(text, "abcdefgh");
+		}
 	}
 
 	for (i = 0; i < 10; i++) {
diff --git a/tests/ctf-global-types/ust_tests_ctf_global_types.h b/tests/ctf-global-types/ust_tests_ctf_global_types.h
index 6fe2058..7b704d2 100644
--- a/tests/ctf-global-types/ust_tests_ctf_global_types.h
+++ b/tests/ctf-global-types/ust_tests_ctf_global_types.h
@@ -38,16 +38,36 @@ TRACEPOINT_ENUM(ust_tests_ctf_global_types, testenum,
 	)
 )
 
+TRACEPOINT_STRUCT(ust_tests_ctf_global_types, inner,
+	TP_ARGS(char *, text, size_t, textlen,
+		int, enumvalue),
+	TP_FIELDS(
+		ctf_array(char, arrfield, text, 10)
+		ctf_sequence(char, seqfield, text, size_t, textlen)
+		ctf_enum(ust_tests_ctf_global_types, testenum, enumfield, enumvalue)
+	)
+)
+
+TRACEPOINT_STRUCT(ust_tests_ctf_global_types, outer,
+	TP_ARGS(char *, text, size_t, textlen,
+		int, enumvalue),
+	TP_FIELDS(
+		ctf_string(stringfield, text)
+		ctf_struct(ust_tests_ctf_global_types, inner, structfield, text, textlen, enumvalue)
+	)
+)
+
 /*
  * Enumeration field is used twice to make sure the global type declaration
  * is entered only once in the metadata file.
  */
 TRACEPOINT_EVENT(ust_tests_ctf_global_types, tptest,
-	TP_ARGS(int, anint, int, enumval),
+	TP_ARGS(int, anint, int, enumval, char *, text, size_t, textlen),
 	TP_FIELDS(
 		ctf_integer(int, intfield, anint)
 		ctf_enum(ust_tests_ctf_global_types, testenum, enumfield, enumval)
 		ctf_enum(ust_tests_ctf_global_types, testenum, enumfield_bis, enumval)
+		ctf_struct(ust_tests_ctf_global_types, outer, structfield, text, textlen, enumval)
 	)
 )
 
-- 
1.9.1


_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

[RFC Patch Ust 5/5] Update the LTTng documentation with CTF global structures

[Geneviève Bastien]

Signed-off-by: Geneviève Bastien <gbastien+lttng@versatic.net>
---
 doc/man/lttng-ust.3 | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/doc/man/lttng-ust.3 b/doc/man/lttng-ust.3
index 40452df..45f8549 100644
--- a/doc/man/lttng-ust.3
+++ b/doc/man/lttng-ust.3
@@ -194,6 +194,17 @@ TRACEPOINT_EVENT(
 		 * previously with the TRACEPOINT_ENUM macro, described below.
 		 */
 		ctf_enum(sample_component, enumeration_name, enumfield, enumarg)
+
+		/*
+		 * ctf_struct: a field using a previously defined struct
+		 * args: (provider, struct name, field name, [argument expression]*)
+		 * The structure must have been previously defined with the
+		 * TRACEPOINT_STRUCT macro, described below.
+		 *
+		 * The [argument expression]* part must correspond to the arguments
+		 * defined in the TP_ARGS of the struct.
+		 */
+		ctf_struct(sample_component, structure_name, structfield, args...)
 	)
 )
 
@@ -250,6 +261,49 @@ TRACEPOINT_ENUM(
 	)
 )
 
+A structure allows to group fields together. They can be reused by different
+tracepoints:
+
+TRACEPOINT_STRUCT(
+	/*
+	 * The provider name, as described in the TRACEPOINT_EVENT macro
+	 */
+	sample_component,
+
+	/*
+	 * The name of this structure, that will be used when using this
+	 * metadata type in tracepoint fields
+	 */
+	structure_name,
+
+	/*
+	 * TP_ARGS macro contains the arguments passed for the structure
+	 * it is in the following format
+	 *	      TP_ARGS(type1, name1, type2, name2, ... type10,
+				 name10)
+	 * where there can be from zero to ten elements.
+	 * typeN is the datatype, such as int, struct or double **.
+	 * name is the variable name (in "int myInt" the name would be
+	 * myint)
+	 *	      TP_ARGS() is valid to mean no arguments
+	 *	      TP_ARGS(void) is valid too
+	 */
+	TP_ARGS(int, anint, int, netint, long *, values,
+		 char *, text, size_t, textlen,
+		 double, doublearg, float, floatarg),
+
+	/*
+	 * TP_FIELDS describes how to write the fields of the structure.
+	 * You can put expressions in the "argument expression" area,
+	 * typically using the input arguments from TP_ARGS.
+	 *
+	 * See TP_FIELDS macro in TRACEPOINT_EVENT for complete description
+	 * of possible fields. A structure may not contain itself and there cannot
+	 * be a cycle.
+	 */
+	TP_FIELDS(...)
+)
+
 .fi
 
 .SH "ASSIGNING LOGLEVEL TO EVENTS"
-- 
1.9.1


_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [RFC Patch Ust 1/5] Update data structures to support CTF global structures

[Mathieu Desnoyers]

----- Original Message -----
> From: "Geneviève Bastien" <gbastien+lttng@versatic.net>
> To: lttng-dev@lists.lttng.org
> Sent: Wednesday, March 26, 2014 10:47:08 AM
> Subject: [lttng-dev] [RFC Patch Ust 1/5] Update data structures to support	CTF global structures
> 
> The structures match those in LTTng-tools to support global structures.
> 
> The struct lttng_structure contains the global types required by the
> structure, but not ustctl_structure because global types will be flattened
> in the event description when serializing.
> 
> Signed-off-by: Geneviève Bastien <gbastien+lttng@versatic.net>
> ---
>  include/lttng/ust-ctl.h    | 14 ++++++++++++++
>  include/lttng/ust-events.h | 17 +++++++++++++++++
>  2 files changed, 31 insertions(+)
> 
> diff --git a/include/lttng/ust-ctl.h b/include/lttng/ust-ctl.h
> index ba3a271..708501d 100644
> --- a/include/lttng/ust-ctl.h
> +++ b/include/lttng/ust-ctl.h
> @@ -265,6 +265,7 @@ enum ustctl_abstract_types {
>  	ustctl_atype_sequence,
>  	ustctl_atype_string,
>  	ustctl_atype_float,
> +	ustctl_atype_structure,
>  	NR_USTCTL_ABSTRACT_TYPES,
>  };
>  
> @@ -335,6 +336,9 @@ struct ustctl_type {
>  			struct ustctl_basic_type length_type;
>  			struct ustctl_basic_type elem_type;
>  		} sequence;
> +		struct {
> +			char name[LTTNG_UST_SYM_NAME_LEN];
> +		} structure;
>  		char padding[USTCTL_UST_TYPE_PADDING];
>  	} u;
>  } LTTNG_PACKED;
> @@ -348,9 +352,18 @@ struct ustctl_enum {
>  	char padding[USTCTL_UST_ENUM_TYPE_PADDING];
>  } LTTNG_PACKED;
>  
> +#define USTCTL_UST_STRUCT_TYPE_PADDING	368

why 368 bytes of padding ? What's so large that we could have to
add to this structure ?

Moreover, my other worry is the "const char *" we're adding for
enums and structures. I'm concerned that when we eventually choose
to dynamically allocate those (when we support dynamic probes), we
will have to do ugly cast to remove the "const".

Thoughts ?

Thanks,

Mathieu

> +struct ustctl_structure {
> +	char name[LTTNG_UST_SYM_NAME_LEN];
> +	size_t nr_fields;
> +	struct ustctl_field *fields;
> +	char padding[USTCTL_UST_STRUCT_TYPE_PADDING];
> +};
> +
>  /* CTF categories for global types declared outside event descriptions */
>  enum ustctl_global_type_categories {
>  	ustctl_mtype_enum,
> +	ustctl_mtype_structure,
>  	NR_USTCTL_GLOBAL_TYPES,
>  };
>  
> @@ -359,6 +372,7 @@ struct ustctl_global_type_decl {
>  	uint32_t mtype;
>  	union {
>  		struct ustctl_enum ctf_enum;
> +		struct ustctl_structure ctf_structure;
>  		char padding[USTCTL_UST_GLOBAL_TYPE_DECL_PADDING];
>  	} u;
>  } LTTNG_PACKED;
> diff --git a/include/lttng/ust-events.h b/include/lttng/ust-events.h
> index 6f11e84..89155ae 100644
> --- a/include/lttng/ust-events.h
> +++ b/include/lttng/ust-events.h
> @@ -82,6 +82,7 @@ enum lttng_abstract_types {
>  	atype_sequence,
>  	atype_string,
>  	atype_float,
> +	atype_structure,
>  	NR_ABSTRACT_TYPES,
>  };
>  
> @@ -96,6 +97,7 @@ enum lttng_string_encodings {
>  /* CTF categories for global types declared outside event descriptions */
>  enum lttng_global_type_categories {
>  	mtype_enum,
> +	mtype_structure,
>  	NR_GLOBAL_TYPES,
>  };
>  
> @@ -207,6 +209,9 @@ struct lttng_type {
>  			struct lttng_basic_type length_type;
>  			struct lttng_basic_type elem_type;
>  		} sequence;
> +		struct {
> +			const char *name;
> +		} structure;
>  		char padding[LTTNG_UST_TYPE_PADDING];
>  	} u;
>  };
> @@ -220,12 +225,24 @@ struct lttng_enum {
>  	char padding[LTTNG_UST_ENUM_TYPE_PADDING];
>  };
>  
> +#define LTTNG_UST_STRUCT_TYPE_PADDING 20
> +struct lttng_structure {
> +	const char *name;
> +	size_t nr_fields;
> +	const struct lttng_event_field *fields;
> +	/* Global types required by this structure. */
> +	unsigned int nr_global_type_decl;
> +	const struct lttng_global_type_decl *global_type_decl;
> +	char padding[LTTNG_UST_STRUCT_TYPE_PADDING];
> +};
> +
>  #define LTTNG_UST_GLOBAL_TYPES_PADDING	60
>  struct lttng_global_type_decl {
>  	enum lttng_global_type_categories mtype;
>  	unsigned int nowrite; /* inherited from the field using it */
>  	union {
>  		const struct lttng_enum *ctf_enum;
> +		const struct lttng_structure *ctf_structure;
>  		char padding[LTTNG_UST_GLOBAL_TYPES_PADDING];
>  	} u;
>  };
> --
> 1.9.1
> 
> 
> _______________________________________________
> lttng-dev mailing list
> lttng-dev@lists.lttng.org
> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
> 

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [RFC Patch Ust 2/5] Serialize the CTF global structures for ust-comm

[Mathieu Desnoyers]

----- Original Message -----
> From: "Geneviève Bastien" <gbastien+lttng@versatic.net>
> To: lttng-dev@lists.lttng.org
> Sent: Wednesday, March 26, 2014 10:47:09 AM
> Subject: [lttng-dev] [RFC Patch Ust 2/5] Serialize the CTF global structures	for ust-comm
> 
> It flatten the global types so that global types used by other global types

flatten -> flattens

> all belong to the event at serialization. This avoids recursive calls to the
> send and receive functions and simplifies error handling (no recursive
> frees).

Is the structure and enum part of the event, or declared outside of
the event scope ?

Is there a downside to this approach ? For instance, if the same
structure is used in many events, do we have to send the structure info
many times ?

Thanks,

Mathieu

> 
> Signed-off-by: Geneviève Bastien <gbastien+lttng@versatic.net>
> ---
>  liblttng-ust-comm/lttng-ust-comm.c | 170
>  +++++++++++++++++++++++++++++++++----
>  liblttng-ust-ctl/ustctl.c          |  26 ++++++
>  2 files changed, 179 insertions(+), 17 deletions(-)
> 
> diff --git a/liblttng-ust-comm/lttng-ust-comm.c
> b/liblttng-ust-comm/lttng-ust-comm.c
> index bc22130..90274c4 100644
> --- a/liblttng-ust-comm/lttng-ust-comm.c
> +++ b/liblttng-ust-comm/lttng-ust-comm.c
> @@ -742,6 +742,7 @@ int serialize_basic_type(enum ustctl_abstract_types
> *uatype,
>  	}
>  	case atype_array:
>  	case atype_sequence:
> +	case atype_structure:
>  	default:
>  		return -EINVAL;
>  	}
> @@ -800,6 +801,14 @@ int serialize_one_type(struct ustctl_type *ut, const
> struct lttng_type *lt)
>  		ut->atype = ustctl_atype_sequence;
>  		break;
>  	}
> +	case atype_structure:
> +	{
> +		strncpy(ut->u.structure.name, lt->u.structure.name,
> +						LTTNG_UST_SYM_NAME_LEN);
> +		ut->u.structure.name[LTTNG_UST_SYM_NAME_LEN - 1] = '\0';
> +		ut->atype = ustctl_atype_structure;
> +		break;
> +	}
>  	default:
>  		return -EINVAL;
>  	}
> @@ -892,25 +901,82 @@ int serialize_enum(struct ustctl_enum *uenum,
>  }
>  
>  static
> -int serialize_global_type_decl(size_t *_nr_write_global_type_decl,
> -		struct ustctl_global_type_decl **ustctl_global_type,
> +int serialize_structure(struct ustctl_structure *ustruct,
> +		const struct lttng_structure *lstruct)
> +{
> +	int ret;
> +
> +	strncpy(ustruct->name, lstruct->name, LTTNG_UST_SYM_NAME_LEN);
> +	ustruct->name[LTTNG_UST_SYM_NAME_LEN - 1] = '\0';
> +
> +	/* Serialize the fields */
> +	if (lstruct->nr_fields > 0) {
> +		ret = serialize_fields(&ustruct->nr_fields, &ustruct->fields,
> +				lstruct->nr_fields,	lstruct->fields);
> +		if (ret) {
> +			return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static size_t get_child_global_type_count(size_t nr_global_type_decl,
> +		const struct lttng_global_type_decl *lttng_global_type)
> +{
> +	size_t total_global_types = 0;
> +	int i;
> +	const struct lttng_global_type_decl *lg;
> +
> +	for (i = 0; i < nr_global_type_decl; i++) {
> +		lg = &lttng_global_type[i];
> +
> +		if (lg->nowrite)
> +			continue;
> +
> +		/* Count the global types in children as well */
> +		switch (lg->mtype) {
> +		case mtype_structure:
> +		{
> +			const struct lttng_structure *ls;
> +
> +			ls = lg->u.ctf_structure;
> +			/* Add the number of global types it contains and the count of its own
> children */
> +			if (ls->nr_global_type_decl > 0) {
> +				total_global_types += ls->nr_global_type_decl;
> +				total_global_types +=
> get_child_global_type_count(ls->nr_global_type_decl,
> +								ls->global_type_decl);
> +			}
> +			break;
> +		}
> +		case mtype_enum:
> +		default:
> +			break;
> +		}
> +	}
> +
> +	return total_global_types;
> +}
> +
> +/*
> + * When this method is called, index is the next index to write to and the
> + * ustctl_global_type structure is already initialized and will not
> overflow.
> + *
> + * The global type declarations are flattened at serialization to avoid
> + * needing to do recursive frees on error.
> + */
> +static int _serialize_global_type_decl(size_t *index,
> +		struct ustctl_global_type_decl *global_type_decl,
>  		size_t nr_global_type_decl,
>  		const struct lttng_global_type_decl *lttng_global_type)
>  {
> -	struct ustctl_global_type_decl *global_type_decl;
>  	int i, ret;
> -	size_t nr_write_global_type_decl = 0;
> -
> -	global_type_decl = zmalloc(nr_global_type_decl
> -					* sizeof(*global_type_decl));
> -	if (!global_type_decl)
> -		return -ENOMEM;
>  
>  	for (i = 0; i < nr_global_type_decl; i++) {
>  		struct ustctl_global_type_decl *f;
>  		const struct lttng_global_type_decl *lf;
>  
> -		f = &global_type_decl[nr_write_global_type_decl];
> +		f = &global_type_decl[*index];
>  		lf = &lttng_global_type[i];
>  
>  		/* skip 'nowrite' fields */
> @@ -928,18 +994,66 @@ int serialize_global_type_decl(size_t
> *_nr_write_global_type_decl,
>  			le = lf->u.ctf_enum;
>  			ret = serialize_enum(ue, le);
>  			if (ret)
> -				goto error;
> +				return ret;
>  
>  			f->mtype = ustctl_mtype_enum;
>  			break;
>  		}
> +		case mtype_structure:
> +		{
> +			struct ustctl_structure *us;
> +			const struct lttng_structure *ls;
> +
> +			/* Serialize children global types first */
> +			ls = lf->u.ctf_structure;
> +			ret = _serialize_global_type_decl(index,
> +					global_type_decl,
> +					ls->nr_global_type_decl,
> +					ls->global_type_decl);
> +			if (ret)
> +				return ret;
> +
> +			/* Reinitialize f since the index may have changed */
> +			f = &global_type_decl[*index];
> +			us = &f->u.ctf_structure;
> +
> +			ret = serialize_structure(us, ls);
> +			if (ret)
> +				return ret;
> +
> +			f->mtype = ustctl_mtype_structure;
> +			break;
> +		}
>  		default:
> -			ret = -EINVAL;
> -			goto error;
> +			return -EINVAL;
>  		}
>  
> -		nr_write_global_type_decl++;
> +		*index = *index + 1;
>  	}
> +	return 0;
> +}
> +
> +static
> +int serialize_global_type_decl(size_t *_nr_write_global_type_decl,
> +		struct ustctl_global_type_decl **ustctl_global_type,
> +		size_t nr_global_type_decl,
> +		const struct lttng_global_type_decl *lttng_global_type)
> +{
> +	struct ustctl_global_type_decl *global_type_decl;
> +	int i, ret;
> +	size_t nr_write_global_type_decl = 0;
> +	size_t nr_child_global_type_cnt =
> get_child_global_type_count(nr_global_type_decl,
> +							lttng_global_type);
> +
> +	global_type_decl = zmalloc((nr_global_type_decl + nr_child_global_type_cnt)
> +					* sizeof(*global_type_decl));
> +	if (!global_type_decl)
> +		return -ENOMEM;
> +
> +	ret = _serialize_global_type_decl(&nr_write_global_type_decl,
> global_type_decl,
> +			nr_global_type_decl, lttng_global_type);
> +	if (ret)
> +		goto error;
>  
>  	*_nr_write_global_type_decl = nr_write_global_type_decl;
>  	*ustctl_global_type = global_type_decl;
> @@ -954,6 +1068,9 @@ error:
>  		case ustctl_mtype_enum:
>  			free(m->u.ctf_enum.entries);
>  			break;
> +		case ustctl_mtype_structure:
> +			free(m->u.ctf_structure.fields);
> +			break;
>  		default:
>  			break;
>  		}
> @@ -1158,15 +1275,31 @@ int ustcomm_register_event(int sock,
>  				}
>  				break;
>  			}
> +			case ustctl_mtype_structure:
> +			{
> +				int field_len = one_global_type->u.ctf_structure.nr_fields *
> sizeof(*one_global_type->u.ctf_structure.fields);
> +
> +				/* Send the fields */
> +				DBG("Sending fields for global type structure %s.\n",
> +						one_global_type->u.ctf_structure.name);
> +				len = ustcomm_send_unix_sock(sock,
> one_global_type->u.ctf_structure.fields, field_len);
> +				free(one_global_type->u.ctf_structure.fields);
> +				one_global_type->u.ctf_structure.fields = NULL;
> +				if (len > 0 && len != field_len) {
> +					goto error_global_type;
> +				}
> +				if (len < 0) {
> +					goto error_global_type;
> +				}
> +				break;
> +			}
>  			default:
>  				break;
>  			}
>  		}
> -		free(global_type_decl);
>  
> -	} else {
> -		free(global_type_decl);
>  	}
> +	free(global_type_decl);
>  
>  	/* receive reply */
>  	len = ustcomm_recv_unix_sock(sock, &reply, sizeof(reply));
> @@ -1212,6 +1345,9 @@ error_global_type:
>  		case ustctl_mtype_enum:
>  			free(one_global_type->u.ctf_enum.entries);
>  			break;
> +		case ustctl_mtype_structure:
> +			free(one_global_type->u.ctf_structure.fields);
> +			break;
>  		default:
>  			break;
>  		}
> diff --git a/liblttng-ust-ctl/ustctl.c b/liblttng-ust-ctl/ustctl.c
> index a181e62..3100f1b 100644
> --- a/liblttng-ust-ctl/ustctl.c
> +++ b/liblttng-ust-ctl/ustctl.c
> @@ -1896,6 +1896,32 @@ int ustctl_recv_register_event(int sock,
>  				}
>  				break;
>  			}
> +			case ustctl_mtype_structure:
> +			{
> +				int entry_len = one_global_type->u.ctf_structure.nr_fields *
> sizeof(*one_global_type->u.ctf_structure.fields);
> +				/* Receive the entries */
> +				one_global_type->u.ctf_structure.fields = zmalloc(entry_len);
> +				if (!one_global_type->u.ctf_structure.fields) {
> +					len = -ENOMEM;
> +					goto global_type_error;
> +				}
> +				len = ustcomm_recv_unix_sock(sock,
> +						one_global_type->u.ctf_structure.fields,
> +						entry_len);
> +				DBG("Received fields for struct %s.\n",
> one_global_type->u.ctf_structure.name);
> +				if (len > 0 && len != entry_len) {
> +					len = -EIO;
> +					goto global_type_error;
> +				}
> +				if (len == 0) {
> +					len = -EPIPE;
> +					goto global_type_error;
> +				}
> +				if (len < 0) {
> +					goto global_type_error;
> +				}
> +				break;
> +			}
>  			default:
>  				break;
>  			}
> --
> 1.9.1
> 
> 
> _______________________________________________
> lttng-dev mailing list
> lttng-dev@lists.lttng.org
> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
> 

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [RFC Patch Ust 1/5] Update data structures to support CTF global structures

[Geneviève Bastien]

On 04/09/2014 11:36 AM, Mathieu Desnoyers wrote:
> ----- Original Message -----
>> From: "Geneviève Bastien" <gbastien+lttng@versatic.net>
>> To: lttng-dev@lists.lttng.org
>> Sent: Wednesday, March 26, 2014 10:47:08 AM
>> Subject: [lttng-dev] [RFC Patch Ust 1/5] Update data structures to support	CTF global structures
>>
>> The structures match those in LTTng-tools to support global structures.
>>
>> The struct lttng_structure contains the global types required by the
>> structure, but not ustctl_structure because global types will be flattened
>> in the event description when serializing.
>>
>> Signed-off-by: Geneviève Bastien <gbastien+lttng@versatic.net>
>> ---
>>   include/lttng/ust-ctl.h    | 14 ++++++++++++++
>>   include/lttng/ust-events.h | 17 +++++++++++++++++
>>   2 files changed, 31 insertions(+)
>>
>> diff --git a/include/lttng/ust-ctl.h b/include/lttng/ust-ctl.h
>> index ba3a271..708501d 100644
>> --- a/include/lttng/ust-ctl.h
>> +++ b/include/lttng/ust-ctl.h
>> @@ -265,6 +265,7 @@ enum ustctl_abstract_types {
>>   	ustctl_atype_sequence,
>>   	ustctl_atype_string,
>>   	ustctl_atype_float,
>> +	ustctl_atype_structure,
>>   	NR_USTCTL_ABSTRACT_TYPES,
>>   };
>>   
>> @@ -335,6 +336,9 @@ struct ustctl_type {
>>   			struct ustctl_basic_type length_type;
>>   			struct ustctl_basic_type elem_type;
>>   		} sequence;
>> +		struct {
>> +			char name[LTTNG_UST_SYM_NAME_LEN];
>> +		} structure;
>>   		char padding[USTCTL_UST_TYPE_PADDING];
>>   	} u;
>>   } LTTNG_PACKED;
>> @@ -348,9 +352,18 @@ struct ustctl_enum {
>>   	char padding[USTCTL_UST_ENUM_TYPE_PADDING];
>>   } LTTNG_PACKED;
>>   
>> +#define USTCTL_UST_STRUCT_TYPE_PADDING	368
> why 368 bytes of padding ? What's so large that we could have to
> add to this structure ?
It's USTCTL_UST_GLOBAL_TYPE_DECL_PADDING (640) minus the size of the 
content of this structure. Could be smaller though if it is not always 
used inside the ustctl_global_type_decl. But it leaves enough space for 
at least another string if need be (but I don't know why we would need 
that for structures).
>
> Moreover, my other worry is the "const char *" we're adding for
> enums and structures. I'm concerned that when we eventually choose
> to dynamically allocate those (when we support dynamic probes), we
> will have to do ugly cast to remove the "const".
>
> Thoughts ?
No, I have no idea what to do with that piece of information... Sorry 
that's my limited knowledge of C. I've just seen const char * in many 
other places of the ust-events.h structures and did the same.

Thanks,
Geneviève
>
> Thanks,
>
> Mathieu
>
>> +struct ustctl_structure {
>> +	char name[LTTNG_UST_SYM_NAME_LEN];
>> +	size_t nr_fields;
>> +	struct ustctl_field *fields;
>> +	char padding[USTCTL_UST_STRUCT_TYPE_PADDING];
>> +};
>> +
>>   /* CTF categories for global types declared outside event descriptions */
>>   enum ustctl_global_type_categories {
>>   	ustctl_mtype_enum,
>> +	ustctl_mtype_structure,
>>   	NR_USTCTL_GLOBAL_TYPES,
>>   };
>>   
>> @@ -359,6 +372,7 @@ struct ustctl_global_type_decl {
>>   	uint32_t mtype;
>>   	union {
>>   		struct ustctl_enum ctf_enum;
>> +		struct ustctl_structure ctf_structure;
>>   		char padding[USTCTL_UST_GLOBAL_TYPE_DECL_PADDING];
>>   	} u;
>>   } LTTNG_PACKED;
>> diff --git a/include/lttng/ust-events.h b/include/lttng/ust-events.h
>> index 6f11e84..89155ae 100644
>> --- a/include/lttng/ust-events.h
>> +++ b/include/lttng/ust-events.h
>> @@ -82,6 +82,7 @@ enum lttng_abstract_types {
>>   	atype_sequence,
>>   	atype_string,
>>   	atype_float,
>> +	atype_structure,
>>   	NR_ABSTRACT_TYPES,
>>   };
>>   
>> @@ -96,6 +97,7 @@ enum lttng_string_encodings {
>>   /* CTF categories for global types declared outside event descriptions */
>>   enum lttng_global_type_categories {
>>   	mtype_enum,
>> +	mtype_structure,
>>   	NR_GLOBAL_TYPES,
>>   };
>>   
>> @@ -207,6 +209,9 @@ struct lttng_type {
>>   			struct lttng_basic_type length_type;
>>   			struct lttng_basic_type elem_type;
>>   		} sequence;
>> +		struct {
>> +			const char *name;
>> +		} structure;
>>   		char padding[LTTNG_UST_TYPE_PADDING];
>>   	} u;
>>   };
>> @@ -220,12 +225,24 @@ struct lttng_enum {
>>   	char padding[LTTNG_UST_ENUM_TYPE_PADDING];
>>   };
>>   
>> +#define LTTNG_UST_STRUCT_TYPE_PADDING 20
>> +struct lttng_structure {
>> +	const char *name;
>> +	size_t nr_fields;
>> +	const struct lttng_event_field *fields;
>> +	/* Global types required by this structure. */
>> +	unsigned int nr_global_type_decl;
>> +	const struct lttng_global_type_decl *global_type_decl;
>> +	char padding[LTTNG_UST_STRUCT_TYPE_PADDING];
>> +};
>> +
>>   #define LTTNG_UST_GLOBAL_TYPES_PADDING	60
>>   struct lttng_global_type_decl {
>>   	enum lttng_global_type_categories mtype;
>>   	unsigned int nowrite; /* inherited from the field using it */
>>   	union {
>>   		const struct lttng_enum *ctf_enum;
>> +		const struct lttng_structure *ctf_structure;
>>   		char padding[LTTNG_UST_GLOBAL_TYPES_PADDING];
>>   	} u;
>>   };
>> --
>> 1.9.1
>>
>>
>> _______________________________________________
>> lttng-dev mailing list
>> lttng-dev@lists.lttng.org
>> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
>>


_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [RFC Patch Ust 2/5] Serialize the CTF global structures for ust-comm

[Geneviève Bastien]

On 04/09/2014 11:42 AM, Mathieu Desnoyers wrote:
> ----- Original Message -----
>> From: "Geneviève Bastien" <gbastien+lttng@versatic.net>
>> To: lttng-dev@lists.lttng.org
>> Sent: Wednesday, March 26, 2014 10:47:09 AM
>> Subject: [lttng-dev] [RFC Patch Ust 2/5] Serialize the CTF global structures	for ust-comm
>>
>> It flatten the global types so that global types used by other global types
> flatten -> flattens
>
>> all belong to the event at serialization. This avoids recursive calls to the
>> send and receive functions and simplifies error handling (no recursive
>> frees).
> Is the structure and enum part of the event, or declared outside of
> the event scope ?
They are named structures so declared outside the scope of the event. 
But they are sent only if one event needs them.
>
> Is there a downside to this approach ? For instance, if the same
> structure is used in many events, do we have to send the structure info
> many times ?
Sending each structure (and enum) once would be enough, we could keep a 
hash table of global types that were sent, like we do before dumping the 
metadata of those global types, and that would avoid sending them many 
times. But it is not what is being done right now. We just send all the 
global types, but they are added to the metadata only once.

Thanks,
Geneviève
>
> Thanks,
>
> Mathieu
>
>> Signed-off-by: Geneviève Bastien <gbastien+lttng@versatic.net>
>> ---
>>   liblttng-ust-comm/lttng-ust-comm.c | 170
>>   +++++++++++++++++++++++++++++++++----
>>   liblttng-ust-ctl/ustctl.c          |  26 ++++++
>>   2 files changed, 179 insertions(+), 17 deletions(-)
>>
>> diff --git a/liblttng-ust-comm/lttng-ust-comm.c
>> b/liblttng-ust-comm/lttng-ust-comm.c
>> index bc22130..90274c4 100644
>> --- a/liblttng-ust-comm/lttng-ust-comm.c
>> +++ b/liblttng-ust-comm/lttng-ust-comm.c
>> @@ -742,6 +742,7 @@ int serialize_basic_type(enum ustctl_abstract_types
>> *uatype,
>>   	}
>>   	case atype_array:
>>   	case atype_sequence:
>> +	case atype_structure:
>>   	default:
>>   		return -EINVAL;
>>   	}
>> @@ -800,6 +801,14 @@ int serialize_one_type(struct ustctl_type *ut, const
>> struct lttng_type *lt)
>>   		ut->atype = ustctl_atype_sequence;
>>   		break;
>>   	}
>> +	case atype_structure:
>> +	{
>> +		strncpy(ut->u.structure.name, lt->u.structure.name,
>> +						LTTNG_UST_SYM_NAME_LEN);
>> +		ut->u.structure.name[LTTNG_UST_SYM_NAME_LEN - 1] = '\0';
>> +		ut->atype = ustctl_atype_structure;
>> +		break;
>> +	}
>>   	default:
>>   		return -EINVAL;
>>   	}
>> @@ -892,25 +901,82 @@ int serialize_enum(struct ustctl_enum *uenum,
>>   }
>>   
>>   static
>> -int serialize_global_type_decl(size_t *_nr_write_global_type_decl,
>> -		struct ustctl_global_type_decl **ustctl_global_type,
>> +int serialize_structure(struct ustctl_structure *ustruct,
>> +		const struct lttng_structure *lstruct)
>> +{
>> +	int ret;
>> +
>> +	strncpy(ustruct->name, lstruct->name, LTTNG_UST_SYM_NAME_LEN);
>> +	ustruct->name[LTTNG_UST_SYM_NAME_LEN - 1] = '\0';
>> +
>> +	/* Serialize the fields */
>> +	if (lstruct->nr_fields > 0) {
>> +		ret = serialize_fields(&ustruct->nr_fields, &ustruct->fields,
>> +				lstruct->nr_fields,	lstruct->fields);
>> +		if (ret) {
>> +			return ret;
>> +		}
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static size_t get_child_global_type_count(size_t nr_global_type_decl,
>> +		const struct lttng_global_type_decl *lttng_global_type)
>> +{
>> +	size_t total_global_types = 0;
>> +	int i;
>> +	const struct lttng_global_type_decl *lg;
>> +
>> +	for (i = 0; i < nr_global_type_decl; i++) {
>> +		lg = &lttng_global_type[i];
>> +
>> +		if (lg->nowrite)
>> +			continue;
>> +
>> +		/* Count the global types in children as well */
>> +		switch (lg->mtype) {
>> +		case mtype_structure:
>> +		{
>> +			const struct lttng_structure *ls;
>> +
>> +			ls = lg->u.ctf_structure;
>> +			/* Add the number of global types it contains and the count of its own
>> children */
>> +			if (ls->nr_global_type_decl > 0) {
>> +				total_global_types += ls->nr_global_type_decl;
>> +				total_global_types +=
>> get_child_global_type_count(ls->nr_global_type_decl,
>> +								ls->global_type_decl);
>> +			}
>> +			break;
>> +		}
>> +		case mtype_enum:
>> +		default:
>> +			break;
>> +		}
>> +	}
>> +
>> +	return total_global_types;
>> +}
>> +
>> +/*
>> + * When this method is called, index is the next index to write to and the
>> + * ustctl_global_type structure is already initialized and will not
>> overflow.
>> + *
>> + * The global type declarations are flattened at serialization to avoid
>> + * needing to do recursive frees on error.
>> + */
>> +static int _serialize_global_type_decl(size_t *index,
>> +		struct ustctl_global_type_decl *global_type_decl,
>>   		size_t nr_global_type_decl,
>>   		const struct lttng_global_type_decl *lttng_global_type)
>>   {
>> -	struct ustctl_global_type_decl *global_type_decl;
>>   	int i, ret;
>> -	size_t nr_write_global_type_decl = 0;
>> -
>> -	global_type_decl = zmalloc(nr_global_type_decl
>> -					* sizeof(*global_type_decl));
>> -	if (!global_type_decl)
>> -		return -ENOMEM;
>>   
>>   	for (i = 0; i < nr_global_type_decl; i++) {
>>   		struct ustctl_global_type_decl *f;
>>   		const struct lttng_global_type_decl *lf;
>>   
>> -		f = &global_type_decl[nr_write_global_type_decl];
>> +		f = &global_type_decl[*index];
>>   		lf = &lttng_global_type[i];
>>   
>>   		/* skip 'nowrite' fields */
>> @@ -928,18 +994,66 @@ int serialize_global_type_decl(size_t
>> *_nr_write_global_type_decl,
>>   			le = lf->u.ctf_enum;
>>   			ret = serialize_enum(ue, le);
>>   			if (ret)
>> -				goto error;
>> +				return ret;
>>   
>>   			f->mtype = ustctl_mtype_enum;
>>   			break;
>>   		}
>> +		case mtype_structure:
>> +		{
>> +			struct ustctl_structure *us;
>> +			const struct lttng_structure *ls;
>> +
>> +			/* Serialize children global types first */
>> +			ls = lf->u.ctf_structure;
>> +			ret = _serialize_global_type_decl(index,
>> +					global_type_decl,
>> +					ls->nr_global_type_decl,
>> +					ls->global_type_decl);
>> +			if (ret)
>> +				return ret;
>> +
>> +			/* Reinitialize f since the index may have changed */
>> +			f = &global_type_decl[*index];
>> +			us = &f->u.ctf_structure;
>> +
>> +			ret = serialize_structure(us, ls);
>> +			if (ret)
>> +				return ret;
>> +
>> +			f->mtype = ustctl_mtype_structure;
>> +			break;
>> +		}
>>   		default:
>> -			ret = -EINVAL;
>> -			goto error;
>> +			return -EINVAL;
>>   		}
>>   
>> -		nr_write_global_type_decl++;
>> +		*index = *index + 1;
>>   	}
>> +	return 0;
>> +}
>> +
>> +static
>> +int serialize_global_type_decl(size_t *_nr_write_global_type_decl,
>> +		struct ustctl_global_type_decl **ustctl_global_type,
>> +		size_t nr_global_type_decl,
>> +		const struct lttng_global_type_decl *lttng_global_type)
>> +{
>> +	struct ustctl_global_type_decl *global_type_decl;
>> +	int i, ret;
>> +	size_t nr_write_global_type_decl = 0;
>> +	size_t nr_child_global_type_cnt =
>> get_child_global_type_count(nr_global_type_decl,
>> +							lttng_global_type);
>> +
>> +	global_type_decl = zmalloc((nr_global_type_decl + nr_child_global_type_cnt)
>> +					* sizeof(*global_type_decl));
>> +	if (!global_type_decl)
>> +		return -ENOMEM;
>> +
>> +	ret = _serialize_global_type_decl(&nr_write_global_type_decl,
>> global_type_decl,
>> +			nr_global_type_decl, lttng_global_type);
>> +	if (ret)
>> +		goto error;
>>   
>>   	*_nr_write_global_type_decl = nr_write_global_type_decl;
>>   	*ustctl_global_type = global_type_decl;
>> @@ -954,6 +1068,9 @@ error:
>>   		case ustctl_mtype_enum:
>>   			free(m->u.ctf_enum.entries);
>>   			break;
>> +		case ustctl_mtype_structure:
>> +			free(m->u.ctf_structure.fields);
>> +			break;
>>   		default:
>>   			break;
>>   		}
>> @@ -1158,15 +1275,31 @@ int ustcomm_register_event(int sock,
>>   				}
>>   				break;
>>   			}
>> +			case ustctl_mtype_structure:
>> +			{
>> +				int field_len = one_global_type->u.ctf_structure.nr_fields *
>> sizeof(*one_global_type->u.ctf_structure.fields);
>> +
>> +				/* Send the fields */
>> +				DBG("Sending fields for global type structure %s.\n",
>> +						one_global_type->u.ctf_structure.name);
>> +				len = ustcomm_send_unix_sock(sock,
>> one_global_type->u.ctf_structure.fields, field_len);
>> +				free(one_global_type->u.ctf_structure.fields);
>> +				one_global_type->u.ctf_structure.fields = NULL;
>> +				if (len > 0 && len != field_len) {
>> +					goto error_global_type;
>> +				}
>> +				if (len < 0) {
>> +					goto error_global_type;
>> +				}
>> +				break;
>> +			}
>>   			default:
>>   				break;
>>   			}
>>   		}
>> -		free(global_type_decl);
>>   
>> -	} else {
>> -		free(global_type_decl);
>>   	}
>> +	free(global_type_decl);
>>   
>>   	/* receive reply */
>>   	len = ustcomm_recv_unix_sock(sock, &reply, sizeof(reply));
>> @@ -1212,6 +1345,9 @@ error_global_type:
>>   		case ustctl_mtype_enum:
>>   			free(one_global_type->u.ctf_enum.entries);
>>   			break;
>> +		case ustctl_mtype_structure:
>> +			free(one_global_type->u.ctf_structure.fields);
>> +			break;
>>   		default:
>>   			break;
>>   		}
>> diff --git a/liblttng-ust-ctl/ustctl.c b/liblttng-ust-ctl/ustctl.c
>> index a181e62..3100f1b 100644
>> --- a/liblttng-ust-ctl/ustctl.c
>> +++ b/liblttng-ust-ctl/ustctl.c
>> @@ -1896,6 +1896,32 @@ int ustctl_recv_register_event(int sock,
>>   				}
>>   				break;
>>   			}
>> +			case ustctl_mtype_structure:
>> +			{
>> +				int entry_len = one_global_type->u.ctf_structure.nr_fields *
>> sizeof(*one_global_type->u.ctf_structure.fields);
>> +				/* Receive the entries */
>> +				one_global_type->u.ctf_structure.fields = zmalloc(entry_len);
>> +				if (!one_global_type->u.ctf_structure.fields) {
>> +					len = -ENOMEM;
>> +					goto global_type_error;
>> +				}
>> +				len = ustcomm_recv_unix_sock(sock,
>> +						one_global_type->u.ctf_structure.fields,
>> +						entry_len);
>> +				DBG("Received fields for struct %s.\n",
>> one_global_type->u.ctf_structure.name);
>> +				if (len > 0 && len != entry_len) {
>> +					len = -EIO;
>> +					goto global_type_error;
>> +				}
>> +				if (len == 0) {
>> +					len = -EPIPE;
>> +					goto global_type_error;
>> +				}
>> +				if (len < 0) {
>> +					goto global_type_error;
>> +				}
>> +				break;
>> +			}
>>   			default:
>>   				break;
>>   			}
>> --
>> 1.9.1
>>
>>
>> _______________________________________________
>> lttng-dev mailing list
>> lttng-dev@lists.lttng.org
>> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
>>


_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [RFC Patch Ust 2/5] Serialize the CTF global structures for ust-comm

[Mathieu Desnoyers]

----- Original Message -----
> From: "Geneviève Bastien" <gbastien+lttng@versatic.net>
> To: "Mathieu Desnoyers" <mathieu.desnoyers@efficios.com>
> Cc: lttng-dev@lists.lttng.org
> Sent: Wednesday, April 9, 2014 7:33:06 PM
> Subject: Re: [lttng-dev] [RFC Patch Ust 2/5] Serialize the CTF global structures for ust-comm
> 
> On 04/09/2014 11:42 AM, Mathieu Desnoyers wrote:
> > ----- Original Message -----
> >> From: "Geneviève Bastien" <gbastien+lttng@versatic.net>
> >> To: lttng-dev@lists.lttng.org
> >> Sent: Wednesday, March 26, 2014 10:47:09 AM
> >> Subject: [lttng-dev] [RFC Patch Ust 2/5] Serialize the CTF global
> >> structures	for ust-comm
> >>
> >> It flatten the global types so that global types used by other global
> >> types
> > flatten -> flattens
> >
> >> all belong to the event at serialization. This avoids recursive calls to
> >> the
> >> send and receive functions and simplifies error handling (no recursive
> >> frees).
> > Is the structure and enum part of the event, or declared outside of
> > the event scope ?
> They are named structures so declared outside the scope of the event.
> But they are sent only if one event needs them.
> >
> > Is there a downside to this approach ? For instance, if the same
> > structure is used in many events, do we have to send the structure info
> > many times ?
> Sending each structure (and enum) once would be enough, we could keep a
> hash table of global types that were sent, like we do before dumping the
> metadata of those global types, and that would avoid sending them many
> times. But it is not what is being done right now. We just send all the
> global types, but they are added to the metadata only once.

Repeating the integers, strings, etc on the "wire" (unix socket) for
every field between UST and sessiond was not so bad, since they were
small enough descriptions.

However, I figure that enumerations, structures and variant types will
grow to be much larger than their basic types counterparts.

Therefore, sending the global type separately, and then sending a
reference to that type (a string) when encountering the event field
using this global type seems like a more scalable solution to me.

Thanks,

Mathieu

> 
> Thanks,
> Geneviève
> >
> > Thanks,
> >
> > Mathieu
> >
> >> Signed-off-by: Geneviève Bastien <gbastien+lttng@versatic.net>
> >> ---
> >>   liblttng-ust-comm/lttng-ust-comm.c | 170
> >>   +++++++++++++++++++++++++++++++++----
> >>   liblttng-ust-ctl/ustctl.c          |  26 ++++++
> >>   2 files changed, 179 insertions(+), 17 deletions(-)
> >>
> >> diff --git a/liblttng-ust-comm/lttng-ust-comm.c
> >> b/liblttng-ust-comm/lttng-ust-comm.c
> >> index bc22130..90274c4 100644
> >> --- a/liblttng-ust-comm/lttng-ust-comm.c
> >> +++ b/liblttng-ust-comm/lttng-ust-comm.c
> >> @@ -742,6 +742,7 @@ int serialize_basic_type(enum ustctl_abstract_types
> >> *uatype,
> >>   	}
> >>   	case atype_array:
> >>   	case atype_sequence:
> >> +	case atype_structure:
> >>   	default:
> >>   		return -EINVAL;
> >>   	}
> >> @@ -800,6 +801,14 @@ int serialize_one_type(struct ustctl_type *ut, const
> >> struct lttng_type *lt)
> >>   		ut->atype = ustctl_atype_sequence;
> >>   		break;
> >>   	}
> >> +	case atype_structure:
> >> +	{
> >> +		strncpy(ut->u.structure.name, lt->u.structure.name,
> >> +						LTTNG_UST_SYM_NAME_LEN);
> >> +		ut->u.structure.name[LTTNG_UST_SYM_NAME_LEN - 1] = '\0';
> >> +		ut->atype = ustctl_atype_structure;
> >> +		break;
> >> +	}
> >>   	default:
> >>   		return -EINVAL;
> >>   	}
> >> @@ -892,25 +901,82 @@ int serialize_enum(struct ustctl_enum *uenum,
> >>   }
> >>   
> >>   static
> >> -int serialize_global_type_decl(size_t *_nr_write_global_type_decl,
> >> -		struct ustctl_global_type_decl **ustctl_global_type,
> >> +int serialize_structure(struct ustctl_structure *ustruct,
> >> +		const struct lttng_structure *lstruct)
> >> +{
> >> +	int ret;
> >> +
> >> +	strncpy(ustruct->name, lstruct->name, LTTNG_UST_SYM_NAME_LEN);
> >> +	ustruct->name[LTTNG_UST_SYM_NAME_LEN - 1] = '\0';
> >> +
> >> +	/* Serialize the fields */
> >> +	if (lstruct->nr_fields > 0) {
> >> +		ret = serialize_fields(&ustruct->nr_fields, &ustruct->fields,
> >> +				lstruct->nr_fields,	lstruct->fields);
> >> +		if (ret) {
> >> +			return ret;
> >> +		}
> >> +	}
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static size_t get_child_global_type_count(size_t nr_global_type_decl,
> >> +		const struct lttng_global_type_decl *lttng_global_type)
> >> +{
> >> +	size_t total_global_types = 0;
> >> +	int i;
> >> +	const struct lttng_global_type_decl *lg;
> >> +
> >> +	for (i = 0; i < nr_global_type_decl; i++) {
> >> +		lg = &lttng_global_type[i];
> >> +
> >> +		if (lg->nowrite)
> >> +			continue;
> >> +
> >> +		/* Count the global types in children as well */
> >> +		switch (lg->mtype) {
> >> +		case mtype_structure:
> >> +		{
> >> +			const struct lttng_structure *ls;
> >> +
> >> +			ls = lg->u.ctf_structure;
> >> +			/* Add the number of global types it contains and the count of its own
> >> children */
> >> +			if (ls->nr_global_type_decl > 0) {
> >> +				total_global_types += ls->nr_global_type_decl;
> >> +				total_global_types +=
> >> get_child_global_type_count(ls->nr_global_type_decl,
> >> +								ls->global_type_decl);
> >> +			}
> >> +			break;
> >> +		}
> >> +		case mtype_enum:
> >> +		default:
> >> +			break;
> >> +		}
> >> +	}
> >> +
> >> +	return total_global_types;
> >> +}
> >> +
> >> +/*
> >> + * When this method is called, index is the next index to write to and
> >> the
> >> + * ustctl_global_type structure is already initialized and will not
> >> overflow.
> >> + *
> >> + * The global type declarations are flattened at serialization to avoid
> >> + * needing to do recursive frees on error.
> >> + */
> >> +static int _serialize_global_type_decl(size_t *index,
> >> +		struct ustctl_global_type_decl *global_type_decl,
> >>   		size_t nr_global_type_decl,
> >>   		const struct lttng_global_type_decl *lttng_global_type)
> >>   {
> >> -	struct ustctl_global_type_decl *global_type_decl;
> >>   	int i, ret;
> >> -	size_t nr_write_global_type_decl = 0;
> >> -
> >> -	global_type_decl = zmalloc(nr_global_type_decl
> >> -					* sizeof(*global_type_decl));
> >> -	if (!global_type_decl)
> >> -		return -ENOMEM;
> >>   
> >>   	for (i = 0; i < nr_global_type_decl; i++) {
> >>   		struct ustctl_global_type_decl *f;
> >>   		const struct lttng_global_type_decl *lf;
> >>   
> >> -		f = &global_type_decl[nr_write_global_type_decl];
> >> +		f = &global_type_decl[*index];
> >>   		lf = &lttng_global_type[i];
> >>   
> >>   		/* skip 'nowrite' fields */
> >> @@ -928,18 +994,66 @@ int serialize_global_type_decl(size_t
> >> *_nr_write_global_type_decl,
> >>   			le = lf->u.ctf_enum;
> >>   			ret = serialize_enum(ue, le);
> >>   			if (ret)
> >> -				goto error;
> >> +				return ret;
> >>   
> >>   			f->mtype = ustctl_mtype_enum;
> >>   			break;
> >>   		}
> >> +		case mtype_structure:
> >> +		{
> >> +			struct ustctl_structure *us;
> >> +			const struct lttng_structure *ls;
> >> +
> >> +			/* Serialize children global types first */
> >> +			ls = lf->u.ctf_structure;
> >> +			ret = _serialize_global_type_decl(index,
> >> +					global_type_decl,
> >> +					ls->nr_global_type_decl,
> >> +					ls->global_type_decl);
> >> +			if (ret)
> >> +				return ret;
> >> +
> >> +			/* Reinitialize f since the index may have changed */
> >> +			f = &global_type_decl[*index];
> >> +			us = &f->u.ctf_structure;
> >> +
> >> +			ret = serialize_structure(us, ls);
> >> +			if (ret)
> >> +				return ret;
> >> +
> >> +			f->mtype = ustctl_mtype_structure;
> >> +			break;
> >> +		}
> >>   		default:
> >> -			ret = -EINVAL;
> >> -			goto error;
> >> +			return -EINVAL;
> >>   		}
> >>   
> >> -		nr_write_global_type_decl++;
> >> +		*index = *index + 1;
> >>   	}
> >> +	return 0;
> >> +}
> >> +
> >> +static
> >> +int serialize_global_type_decl(size_t *_nr_write_global_type_decl,
> >> +		struct ustctl_global_type_decl **ustctl_global_type,
> >> +		size_t nr_global_type_decl,
> >> +		const struct lttng_global_type_decl *lttng_global_type)
> >> +{
> >> +	struct ustctl_global_type_decl *global_type_decl;
> >> +	int i, ret;
> >> +	size_t nr_write_global_type_decl = 0;
> >> +	size_t nr_child_global_type_cnt =
> >> get_child_global_type_count(nr_global_type_decl,
> >> +							lttng_global_type);
> >> +
> >> +	global_type_decl = zmalloc((nr_global_type_decl +
> >> nr_child_global_type_cnt)
> >> +					* sizeof(*global_type_decl));
> >> +	if (!global_type_decl)
> >> +		return -ENOMEM;
> >> +
> >> +	ret = _serialize_global_type_decl(&nr_write_global_type_decl,
> >> global_type_decl,
> >> +			nr_global_type_decl, lttng_global_type);
> >> +	if (ret)
> >> +		goto error;
> >>   
> >>   	*_nr_write_global_type_decl = nr_write_global_type_decl;
> >>   	*ustctl_global_type = global_type_decl;
> >> @@ -954,6 +1068,9 @@ error:
> >>   		case ustctl_mtype_enum:
> >>   			free(m->u.ctf_enum.entries);
> >>   			break;
> >> +		case ustctl_mtype_structure:
> >> +			free(m->u.ctf_structure.fields);
> >> +			break;
> >>   		default:
> >>   			break;
> >>   		}
> >> @@ -1158,15 +1275,31 @@ int ustcomm_register_event(int sock,
> >>   				}
> >>   				break;
> >>   			}
> >> +			case ustctl_mtype_structure:
> >> +			{
> >> +				int field_len = one_global_type->u.ctf_structure.nr_fields *
> >> sizeof(*one_global_type->u.ctf_structure.fields);
> >> +
> >> +				/* Send the fields */
> >> +				DBG("Sending fields for global type structure %s.\n",
> >> +						one_global_type->u.ctf_structure.name);
> >> +				len = ustcomm_send_unix_sock(sock,
> >> one_global_type->u.ctf_structure.fields, field_len);
> >> +				free(one_global_type->u.ctf_structure.fields);
> >> +				one_global_type->u.ctf_structure.fields = NULL;
> >> +				if (len > 0 && len != field_len) {
> >> +					goto error_global_type;
> >> +				}
> >> +				if (len < 0) {
> >> +					goto error_global_type;
> >> +				}
> >> +				break;
> >> +			}
> >>   			default:
> >>   				break;
> >>   			}
> >>   		}
> >> -		free(global_type_decl);
> >>   
> >> -	} else {
> >> -		free(global_type_decl);
> >>   	}
> >> +	free(global_type_decl);
> >>   
> >>   	/* receive reply */
> >>   	len = ustcomm_recv_unix_sock(sock, &reply, sizeof(reply));
> >> @@ -1212,6 +1345,9 @@ error_global_type:
> >>   		case ustctl_mtype_enum:
> >>   			free(one_global_type->u.ctf_enum.entries);
> >>   			break;
> >> +		case ustctl_mtype_structure:
> >> +			free(one_global_type->u.ctf_structure.fields);
> >> +			break;
> >>   		default:
> >>   			break;
> >>   		}
> >> diff --git a/liblttng-ust-ctl/ustctl.c b/liblttng-ust-ctl/ustctl.c
> >> index a181e62..3100f1b 100644
> >> --- a/liblttng-ust-ctl/ustctl.c
> >> +++ b/liblttng-ust-ctl/ustctl.c
> >> @@ -1896,6 +1896,32 @@ int ustctl_recv_register_event(int sock,
> >>   				}
> >>   				break;
> >>   			}
> >> +			case ustctl_mtype_structure:
> >> +			{
> >> +				int entry_len = one_global_type->u.ctf_structure.nr_fields *
> >> sizeof(*one_global_type->u.ctf_structure.fields);
> >> +				/* Receive the entries */
> >> +				one_global_type->u.ctf_structure.fields = zmalloc(entry_len);
> >> +				if (!one_global_type->u.ctf_structure.fields) {
> >> +					len = -ENOMEM;
> >> +					goto global_type_error;
> >> +				}
> >> +				len = ustcomm_recv_unix_sock(sock,
> >> +						one_global_type->u.ctf_structure.fields,
> >> +						entry_len);
> >> +				DBG("Received fields for struct %s.\n",
> >> one_global_type->u.ctf_structure.name);
> >> +				if (len > 0 && len != entry_len) {
> >> +					len = -EIO;
> >> +					goto global_type_error;
> >> +				}
> >> +				if (len == 0) {
> >> +					len = -EPIPE;
> >> +					goto global_type_error;
> >> +				}
> >> +				if (len < 0) {
> >> +					goto global_type_error;
> >> +				}
> >> +				break;
> >> +			}
> >>   			default:
> >>   				break;
> >>   			}
> >> --
> >> 1.9.1
> >>
> >>
> >> _______________________________________________
> >> lttng-dev mailing list
> >> lttng-dev@lists.lttng.org
> >> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
> >>
> 
> 

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [RFC Patch Ust 1/5] Update data structures to support CTF global structures

[Mathieu Desnoyers]

----- Original Message -----
> From: "Geneviève Bastien" <gbastien+lttng@versatic.net>
> To: "Mathieu Desnoyers" <mathieu.desnoyers@efficios.com>
> Cc: lttng-dev@lists.lttng.org
> Sent: Wednesday, April 9, 2014 7:29:14 PM
> Subject: Re: [lttng-dev] [RFC Patch Ust 1/5] Update data structures to support CTF global structures
> 
> On 04/09/2014 11:36 AM, Mathieu Desnoyers wrote:
> > ----- Original Message -----
> >> From: "Geneviève Bastien" <gbastien+lttng@versatic.net>
> >> To: lttng-dev@lists.lttng.org
> >> Sent: Wednesday, March 26, 2014 10:47:08 AM
> >> Subject: [lttng-dev] [RFC Patch Ust 1/5] Update data structures to support
> >> 	CTF global structures
> >>
> >> The structures match those in LTTng-tools to support global structures.
> >>
> >> The struct lttng_structure contains the global types required by the
> >> structure, but not ustctl_structure because global types will be flattened
> >> in the event description when serializing.
> >>
> >> Signed-off-by: Geneviève Bastien <gbastien+lttng@versatic.net>
> >> ---
> >>   include/lttng/ust-ctl.h    | 14 ++++++++++++++
> >>   include/lttng/ust-events.h | 17 +++++++++++++++++
> >>   2 files changed, 31 insertions(+)
> >>
> >> diff --git a/include/lttng/ust-ctl.h b/include/lttng/ust-ctl.h
> >> index ba3a271..708501d 100644
> >> --- a/include/lttng/ust-ctl.h
> >> +++ b/include/lttng/ust-ctl.h
> >> @@ -265,6 +265,7 @@ enum ustctl_abstract_types {
> >>   	ustctl_atype_sequence,
> >>   	ustctl_atype_string,
> >>   	ustctl_atype_float,
> >> +	ustctl_atype_structure,
> >>   	NR_USTCTL_ABSTRACT_TYPES,
> >>   };
> >>   
> >> @@ -335,6 +336,9 @@ struct ustctl_type {
> >>   			struct ustctl_basic_type length_type;
> >>   			struct ustctl_basic_type elem_type;
> >>   		} sequence;
> >> +		struct {
> >> +			char name[LTTNG_UST_SYM_NAME_LEN];
> >> +		} structure;
> >>   		char padding[USTCTL_UST_TYPE_PADDING];
> >>   	} u;
> >>   } LTTNG_PACKED;
> >> @@ -348,9 +352,18 @@ struct ustctl_enum {
> >>   	char padding[USTCTL_UST_ENUM_TYPE_PADDING];
> >>   } LTTNG_PACKED;
> >>   
> >> +#define USTCTL_UST_STRUCT_TYPE_PADDING	368
> > why 368 bytes of padding ? What's so large that we could have to
> > add to this structure ?
> It's USTCTL_UST_GLOBAL_TYPE_DECL_PADDING (640) minus the size of the
> content of this structure. Could be smaller though if it is not always
> used inside the ustctl_global_type_decl. But it leaves enough space for
> at least another string if need be (but I don't know why we would need
> that for structures).

OK, it makes sense.

> >
> > Moreover, my other worry is the "const char *" we're adding for
> > enums and structures. I'm concerned that when we eventually choose
> > to dynamically allocate those (when we support dynamic probes), we
> > will have to do ugly cast to remove the "const".
> >
> > Thoughts ?
> No, I have no idea what to do with that piece of information... Sorry
> that's my limited knowledge of C. I've just seen const char * in many
> other places of the ust-events.h structures and did the same.

I just tried moving the "const char *" to "char *", and the compiler
complains. So let's continue using "const char *" in there, and we'll
cast to "char *" whenever we eventually allocate those structures
dynamically and need to free them. Allocating those objects in an object
stack could also do the trick.

Thanks,

Mathieu

> 
> Thanks,
> Geneviève
> >
> > Thanks,
> >
> > Mathieu
> >
> >> +struct ustctl_structure {
> >> +	char name[LTTNG_UST_SYM_NAME_LEN];
> >> +	size_t nr_fields;
> >> +	struct ustctl_field *fields;
> >> +	char padding[USTCTL_UST_STRUCT_TYPE_PADDING];
> >> +};
> >> +
> >>   /* CTF categories for global types declared outside event descriptions
> >>   */
> >>   enum ustctl_global_type_categories {
> >>   	ustctl_mtype_enum,
> >> +	ustctl_mtype_structure,
> >>   	NR_USTCTL_GLOBAL_TYPES,
> >>   };
> >>   
> >> @@ -359,6 +372,7 @@ struct ustctl_global_type_decl {
> >>   	uint32_t mtype;
> >>   	union {
> >>   		struct ustctl_enum ctf_enum;
> >> +		struct ustctl_structure ctf_structure;
> >>   		char padding[USTCTL_UST_GLOBAL_TYPE_DECL_PADDING];
> >>   	} u;
> >>   } LTTNG_PACKED;
> >> diff --git a/include/lttng/ust-events.h b/include/lttng/ust-events.h
> >> index 6f11e84..89155ae 100644
> >> --- a/include/lttng/ust-events.h
> >> +++ b/include/lttng/ust-events.h
> >> @@ -82,6 +82,7 @@ enum lttng_abstract_types {
> >>   	atype_sequence,
> >>   	atype_string,
> >>   	atype_float,
> >> +	atype_structure,
> >>   	NR_ABSTRACT_TYPES,
> >>   };
> >>   
> >> @@ -96,6 +97,7 @@ enum lttng_string_encodings {
> >>   /* CTF categories for global types declared outside event descriptions
> >>   */
> >>   enum lttng_global_type_categories {
> >>   	mtype_enum,
> >> +	mtype_structure,
> >>   	NR_GLOBAL_TYPES,
> >>   };
> >>   
> >> @@ -207,6 +209,9 @@ struct lttng_type {
> >>   			struct lttng_basic_type length_type;
> >>   			struct lttng_basic_type elem_type;
> >>   		} sequence;
> >> +		struct {
> >> +			const char *name;
> >> +		} structure;
> >>   		char padding[LTTNG_UST_TYPE_PADDING];
> >>   	} u;
> >>   };
> >> @@ -220,12 +225,24 @@ struct lttng_enum {
> >>   	char padding[LTTNG_UST_ENUM_TYPE_PADDING];
> >>   };
> >>   
> >> +#define LTTNG_UST_STRUCT_TYPE_PADDING 20
> >> +struct lttng_structure {
> >> +	const char *name;
> >> +	size_t nr_fields;
> >> +	const struct lttng_event_field *fields;
> >> +	/* Global types required by this structure. */
> >> +	unsigned int nr_global_type_decl;
> >> +	const struct lttng_global_type_decl *global_type_decl;
> >> +	char padding[LTTNG_UST_STRUCT_TYPE_PADDING];
> >> +};
> >> +
> >>   #define LTTNG_UST_GLOBAL_TYPES_PADDING	60
> >>   struct lttng_global_type_decl {
> >>   	enum lttng_global_type_categories mtype;
> >>   	unsigned int nowrite; /* inherited from the field using it */
> >>   	union {
> >>   		const struct lttng_enum *ctf_enum;
> >> +		const struct lttng_structure *ctf_structure;
> >>   		char padding[LTTNG_UST_GLOBAL_TYPES_PADDING];
> >>   	} u;
> >>   };
> >> --
> >> 1.9.1
> >>
> >>
> >> _______________________________________________
> >> lttng-dev mailing list
> >> lttng-dev@lists.lttng.org
> >> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
> >>
> 
> 

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [RFC Patch Ust 2/5] Serialize the CTF global structures for ust-comm

[Geneviève Bastien]

On 14-04-27 04:34 AM, Mathieu Desnoyers wrote:
> ----- Original Message -----
>> From: "Geneviève Bastien" <gbastien+lttng@versatic.net>
>> To: "Mathieu Desnoyers" <mathieu.desnoyers@efficios.com>
>> Cc: lttng-dev@lists.lttng.org
>> Sent: Wednesday, April 9, 2014 7:33:06 PM
>> Subject: Re: [lttng-dev] [RFC Patch Ust 2/5] Serialize the CTF global structures for ust-comm
>>
>> On 04/09/2014 11:42 AM, Mathieu Desnoyers wrote:
>>> ----- Original Message -----
>>>> From: "Geneviève Bastien" <gbastien+lttng@versatic.net>
>>>> To: lttng-dev@lists.lttng.org
>>>> Sent: Wednesday, March 26, 2014 10:47:09 AM
>>>> Subject: [lttng-dev] [RFC Patch Ust 2/5] Serialize the CTF global
>>>> structures	for ust-comm
>>>>
>>>> It flatten the global types so that global types used by other global
>>>> types
>>> flatten -> flattens
>>>
>>>> all belong to the event at serialization. This avoids recursive calls to
>>>> the
>>>> send and receive functions and simplifies error handling (no recursive
>>>> frees).
>>> Is the structure and enum part of the event, or declared outside of
>>> the event scope ?
>> They are named structures so declared outside the scope of the event.
>> But they are sent only if one event needs them.
>>> Is there a downside to this approach ? For instance, if the same
>>> structure is used in many events, do we have to send the structure info
>>> many times ?
>> Sending each structure (and enum) once would be enough, we could keep a
>> hash table of global types that were sent, like we do before dumping the
>> metadata of those global types, and that would avoid sending them many
>> times. But it is not what is being done right now. We just send all the
>> global types, but they are added to the metadata only once.
> Repeating the integers, strings, etc on the "wire" (unix socket) for
> every field between UST and sessiond was not so bad, since they were
> small enough descriptions.
>
> However, I figure that enumerations, structures and variant types will
> grow to be much larger than their basic types counterparts.
>
> Therefore, sending the global type separately, and then sending a
> reference to that type (a string) when encountering the event field
> using this global type seems like a more scalable solution to me.
Just to be clear. Your comment is about the number of times a global 
type is sent across the wire, not the way it is sent in the current 
implementation (an event is followed by the global types it uses)? So 
that event1 which uses struct1 will have 1 global type following it and 
later on, when event2 using also struct1 is sent, then it will have 0 
global types, right?

I'll see what I can do, probably something similar to what is done for 
the events.

Thanks,

Geneviève
>
> Thanks,
>
> Mathieu
>
>> Thanks,
>> Geneviève
>>> Thanks,
>>>
>>> Mathieu
>>>
>>>> Signed-off-by: Geneviève Bastien <gbastien+lttng@versatic.net>
>>>> ---
>>>>    liblttng-ust-comm/lttng-ust-comm.c | 170
>>>>    +++++++++++++++++++++++++++++++++----
>>>>    liblttng-ust-ctl/ustctl.c          |  26 ++++++
>>>>    2 files changed, 179 insertions(+), 17 deletions(-)
>>>>
>>>> diff --git a/liblttng-ust-comm/lttng-ust-comm.c
>>>> b/liblttng-ust-comm/lttng-ust-comm.c
>>>> index bc22130..90274c4 100644
>>>> --- a/liblttng-ust-comm/lttng-ust-comm.c
>>>> +++ b/liblttng-ust-comm/lttng-ust-comm.c
>>>> @@ -742,6 +742,7 @@ int serialize_basic_type(enum ustctl_abstract_types
>>>> *uatype,
>>>>    	}
>>>>    	case atype_array:
>>>>    	case atype_sequence:
>>>> +	case atype_structure:
>>>>    	default:
>>>>    		return -EINVAL;
>>>>    	}
>>>> @@ -800,6 +801,14 @@ int serialize_one_type(struct ustctl_type *ut, const
>>>> struct lttng_type *lt)
>>>>    		ut->atype = ustctl_atype_sequence;
>>>>    		break;
>>>>    	}
>>>> +	case atype_structure:
>>>> +	{
>>>> +		strncpy(ut->u.structure.name, lt->u.structure.name,
>>>> +						LTTNG_UST_SYM_NAME_LEN);
>>>> +		ut->u.structure.name[LTTNG_UST_SYM_NAME_LEN - 1] = '\0';
>>>> +		ut->atype = ustctl_atype_structure;
>>>> +		break;
>>>> +	}
>>>>    	default:
>>>>    		return -EINVAL;
>>>>    	}
>>>> @@ -892,25 +901,82 @@ int serialize_enum(struct ustctl_enum *uenum,
>>>>    }
>>>>    
>>>>    static
>>>> -int serialize_global_type_decl(size_t *_nr_write_global_type_decl,
>>>> -		struct ustctl_global_type_decl **ustctl_global_type,
>>>> +int serialize_structure(struct ustctl_structure *ustruct,
>>>> +		const struct lttng_structure *lstruct)
>>>> +{
>>>> +	int ret;
>>>> +
>>>> +	strncpy(ustruct->name, lstruct->name, LTTNG_UST_SYM_NAME_LEN);
>>>> +	ustruct->name[LTTNG_UST_SYM_NAME_LEN - 1] = '\0';
>>>> +
>>>> +	/* Serialize the fields */
>>>> +	if (lstruct->nr_fields > 0) {
>>>> +		ret = serialize_fields(&ustruct->nr_fields, &ustruct->fields,
>>>> +				lstruct->nr_fields,	lstruct->fields);
>>>> +		if (ret) {
>>>> +			return ret;
>>>> +		}
>>>> +	}
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static size_t get_child_global_type_count(size_t nr_global_type_decl,
>>>> +		const struct lttng_global_type_decl *lttng_global_type)
>>>> +{
>>>> +	size_t total_global_types = 0;
>>>> +	int i;
>>>> +	const struct lttng_global_type_decl *lg;
>>>> +
>>>> +	for (i = 0; i < nr_global_type_decl; i++) {
>>>> +		lg = &lttng_global_type[i];
>>>> +
>>>> +		if (lg->nowrite)
>>>> +			continue;
>>>> +
>>>> +		/* Count the global types in children as well */
>>>> +		switch (lg->mtype) {
>>>> +		case mtype_structure:
>>>> +		{
>>>> +			const struct lttng_structure *ls;
>>>> +
>>>> +			ls = lg->u.ctf_structure;
>>>> +			/* Add the number of global types it contains and the count of its own
>>>> children */
>>>> +			if (ls->nr_global_type_decl > 0) {
>>>> +				total_global_types += ls->nr_global_type_decl;
>>>> +				total_global_types +=
>>>> get_child_global_type_count(ls->nr_global_type_decl,
>>>> +								ls->global_type_decl);
>>>> +			}
>>>> +			break;
>>>> +		}
>>>> +		case mtype_enum:
>>>> +		default:
>>>> +			break;
>>>> +		}
>>>> +	}
>>>> +
>>>> +	return total_global_types;
>>>> +}
>>>> +
>>>> +/*
>>>> + * When this method is called, index is the next index to write to and
>>>> the
>>>> + * ustctl_global_type structure is already initialized and will not
>>>> overflow.
>>>> + *
>>>> + * The global type declarations are flattened at serialization to avoid
>>>> + * needing to do recursive frees on error.
>>>> + */
>>>> +static int _serialize_global_type_decl(size_t *index,
>>>> +		struct ustctl_global_type_decl *global_type_decl,
>>>>    		size_t nr_global_type_decl,
>>>>    		const struct lttng_global_type_decl *lttng_global_type)
>>>>    {
>>>> -	struct ustctl_global_type_decl *global_type_decl;
>>>>    	int i, ret;
>>>> -	size_t nr_write_global_type_decl = 0;
>>>> -
>>>> -	global_type_decl = zmalloc(nr_global_type_decl
>>>> -					* sizeof(*global_type_decl));
>>>> -	if (!global_type_decl)
>>>> -		return -ENOMEM;
>>>>    
>>>>    	for (i = 0; i < nr_global_type_decl; i++) {
>>>>    		struct ustctl_global_type_decl *f;
>>>>    		const struct lttng_global_type_decl *lf;
>>>>    
>>>> -		f = &global_type_decl[nr_write_global_type_decl];
>>>> +		f = &global_type_decl[*index];
>>>>    		lf = &lttng_global_type[i];
>>>>    
>>>>    		/* skip 'nowrite' fields */
>>>> @@ -928,18 +994,66 @@ int serialize_global_type_decl(size_t
>>>> *_nr_write_global_type_decl,
>>>>    			le = lf->u.ctf_enum;
>>>>    			ret = serialize_enum(ue, le);
>>>>    			if (ret)
>>>> -				goto error;
>>>> +				return ret;
>>>>    
>>>>    			f->mtype = ustctl_mtype_enum;
>>>>    			break;
>>>>    		}
>>>> +		case mtype_structure:
>>>> +		{
>>>> +			struct ustctl_structure *us;
>>>> +			const struct lttng_structure *ls;
>>>> +
>>>> +			/* Serialize children global types first */
>>>> +			ls = lf->u.ctf_structure;
>>>> +			ret = _serialize_global_type_decl(index,
>>>> +					global_type_decl,
>>>> +					ls->nr_global_type_decl,
>>>> +					ls->global_type_decl);
>>>> +			if (ret)
>>>> +				return ret;
>>>> +
>>>> +			/* Reinitialize f since the index may have changed */
>>>> +			f = &global_type_decl[*index];
>>>> +			us = &f->u.ctf_structure;
>>>> +
>>>> +			ret = serialize_structure(us, ls);
>>>> +			if (ret)
>>>> +				return ret;
>>>> +
>>>> +			f->mtype = ustctl_mtype_structure;
>>>> +			break;
>>>> +		}
>>>>    		default:
>>>> -			ret = -EINVAL;
>>>> -			goto error;
>>>> +			return -EINVAL;
>>>>    		}
>>>>    
>>>> -		nr_write_global_type_decl++;
>>>> +		*index = *index + 1;
>>>>    	}
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static
>>>> +int serialize_global_type_decl(size_t *_nr_write_global_type_decl,
>>>> +		struct ustctl_global_type_decl **ustctl_global_type,
>>>> +		size_t nr_global_type_decl,
>>>> +		const struct lttng_global_type_decl *lttng_global_type)
>>>> +{
>>>> +	struct ustctl_global_type_decl *global_type_decl;
>>>> +	int i, ret;
>>>> +	size_t nr_write_global_type_decl = 0;
>>>> +	size_t nr_child_global_type_cnt =
>>>> get_child_global_type_count(nr_global_type_decl,
>>>> +							lttng_global_type);
>>>> +
>>>> +	global_type_decl = zmalloc((nr_global_type_decl +
>>>> nr_child_global_type_cnt)
>>>> +					* sizeof(*global_type_decl));
>>>> +	if (!global_type_decl)
>>>> +		return -ENOMEM;
>>>> +
>>>> +	ret = _serialize_global_type_decl(&nr_write_global_type_decl,
>>>> global_type_decl,
>>>> +			nr_global_type_decl, lttng_global_type);
>>>> +	if (ret)
>>>> +		goto error;
>>>>    
>>>>    	*_nr_write_global_type_decl = nr_write_global_type_decl;
>>>>    	*ustctl_global_type = global_type_decl;
>>>> @@ -954,6 +1068,9 @@ error:
>>>>    		case ustctl_mtype_enum:
>>>>    			free(m->u.ctf_enum.entries);
>>>>    			break;
>>>> +		case ustctl_mtype_structure:
>>>> +			free(m->u.ctf_structure.fields);
>>>> +			break;
>>>>    		default:
>>>>    			break;
>>>>    		}
>>>> @@ -1158,15 +1275,31 @@ int ustcomm_register_event(int sock,
>>>>    				}
>>>>    				break;
>>>>    			}
>>>> +			case ustctl_mtype_structure:
>>>> +			{
>>>> +				int field_len = one_global_type->u.ctf_structure.nr_fields *
>>>> sizeof(*one_global_type->u.ctf_structure.fields);
>>>> +
>>>> +				/* Send the fields */
>>>> +				DBG("Sending fields for global type structure %s.\n",
>>>> +						one_global_type->u.ctf_structure.name);
>>>> +				len = ustcomm_send_unix_sock(sock,
>>>> one_global_type->u.ctf_structure.fields, field_len);
>>>> +				free(one_global_type->u.ctf_structure.fields);
>>>> +				one_global_type->u.ctf_structure.fields = NULL;
>>>> +				if (len > 0 && len != field_len) {
>>>> +					goto error_global_type;
>>>> +				}
>>>> +				if (len < 0) {
>>>> +					goto error_global_type;
>>>> +				}
>>>> +				break;
>>>> +			}
>>>>    			default:
>>>>    				break;
>>>>    			}
>>>>    		}
>>>> -		free(global_type_decl);
>>>>    
>>>> -	} else {
>>>> -		free(global_type_decl);
>>>>    	}
>>>> +	free(global_type_decl);
>>>>    
>>>>    	/* receive reply */
>>>>    	len = ustcomm_recv_unix_sock(sock, &reply, sizeof(reply));
>>>> @@ -1212,6 +1345,9 @@ error_global_type:
>>>>    		case ustctl_mtype_enum:
>>>>    			free(one_global_type->u.ctf_enum.entries);
>>>>    			break;
>>>> +		case ustctl_mtype_structure:
>>>> +			free(one_global_type->u.ctf_structure.fields);
>>>> +			break;
>>>>    		default:
>>>>    			break;
>>>>    		}
>>>> diff --git a/liblttng-ust-ctl/ustctl.c b/liblttng-ust-ctl/ustctl.c
>>>> index a181e62..3100f1b 100644
>>>> --- a/liblttng-ust-ctl/ustctl.c
>>>> +++ b/liblttng-ust-ctl/ustctl.c
>>>> @@ -1896,6 +1896,32 @@ int ustctl_recv_register_event(int sock,
>>>>    				}
>>>>    				break;
>>>>    			}
>>>> +			case ustctl_mtype_structure:
>>>> +			{
>>>> +				int entry_len = one_global_type->u.ctf_structure.nr_fields *
>>>> sizeof(*one_global_type->u.ctf_structure.fields);
>>>> +				/* Receive the entries */
>>>> +				one_global_type->u.ctf_structure.fields = zmalloc(entry_len);
>>>> +				if (!one_global_type->u.ctf_structure.fields) {
>>>> +					len = -ENOMEM;
>>>> +					goto global_type_error;
>>>> +				}
>>>> +				len = ustcomm_recv_unix_sock(sock,
>>>> +						one_global_type->u.ctf_structure.fields,
>>>> +						entry_len);
>>>> +				DBG("Received fields for struct %s.\n",
>>>> one_global_type->u.ctf_structure.name);
>>>> +				if (len > 0 && len != entry_len) {
>>>> +					len = -EIO;
>>>> +					goto global_type_error;
>>>> +				}
>>>> +				if (len == 0) {
>>>> +					len = -EPIPE;
>>>> +					goto global_type_error;
>>>> +				}
>>>> +				if (len < 0) {
>>>> +					goto global_type_error;
>>>> +				}
>>>> +				break;
>>>> +			}
>>>>    			default:
>>>>    				break;
>>>>    			}
>>>> --
>>>> 1.9.1
>>>>
>>>>
>>>> _______________________________________________
>>>> lttng-dev mailing list
>>>> lttng-dev@lists.lttng.org
>>>> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
>>>>
>>


_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [RFC Patch Ust 2/5] Serialize the CTF global structures for ust-comm

[Mathieu Desnoyers]

----- Original Message -----
> From: "Geneviève Bastien" <gbastien+lttng@versatic.net>
> To: "Mathieu Desnoyers" <mathieu.desnoyers@efficios.com>
> Cc: lttng-dev@lists.lttng.org
> Sent: Wednesday, April 30, 2014 9:05:35 PM
> Subject: Re: [lttng-dev] [RFC Patch Ust 2/5] Serialize the CTF global structures for ust-comm
> 
> On 14-04-27 04:34 AM, Mathieu Desnoyers wrote:
> > ----- Original Message -----
> >> From: "Geneviève Bastien" <gbastien+lttng@versatic.net>
> >> To: "Mathieu Desnoyers" <mathieu.desnoyers@efficios.com>
> >> Cc: lttng-dev@lists.lttng.org
> >> Sent: Wednesday, April 9, 2014 7:33:06 PM
> >> Subject: Re: [lttng-dev] [RFC Patch Ust 2/5] Serialize the CTF global
> >> structures for ust-comm
> >>
> >> On 04/09/2014 11:42 AM, Mathieu Desnoyers wrote:
> >>> ----- Original Message -----
> >>>> From: "Geneviève Bastien" <gbastien+lttng@versatic.net>
> >>>> To: lttng-dev@lists.lttng.org
> >>>> Sent: Wednesday, March 26, 2014 10:47:09 AM
> >>>> Subject: [lttng-dev] [RFC Patch Ust 2/5] Serialize the CTF global
> >>>> structures	for ust-comm
> >>>>
> >>>> It flatten the global types so that global types used by other global
> >>>> types
> >>> flatten -> flattens
> >>>
> >>>> all belong to the event at serialization. This avoids recursive calls to
> >>>> the
> >>>> send and receive functions and simplifies error handling (no recursive
> >>>> frees).
> >>> Is the structure and enum part of the event, or declared outside of
> >>> the event scope ?
> >> They are named structures so declared outside the scope of the event.
> >> But they are sent only if one event needs them.
> >>> Is there a downside to this approach ? For instance, if the same
> >>> structure is used in many events, do we have to send the structure info
> >>> many times ?
> >> Sending each structure (and enum) once would be enough, we could keep a
> >> hash table of global types that were sent, like we do before dumping the
> >> metadata of those global types, and that would avoid sending them many
> >> times. But it is not what is being done right now. We just send all the
> >> global types, but they are added to the metadata only once.
> > Repeating the integers, strings, etc on the "wire" (unix socket) for
> > every field between UST and sessiond was not so bad, since they were
> > small enough descriptions.
> >
> > However, I figure that enumerations, structures and variant types will
> > grow to be much larger than their basic types counterparts.
> >
> > Therefore, sending the global type separately, and then sending a
> > reference to that type (a string) when encountering the event field
> > using this global type seems like a more scalable solution to me.
> Just to be clear. Your comment is about the number of times a global
> type is sent across the wire, not the way it is sent in the current
> implementation (an event is followed by the global types it uses)? So
> that event1 which uses struct1 will have 1 global type following it and
> later on, when event2 using also struct1 is sent, then it will have 0
> global types, right?

Yes, all this is about not repeating global type descriptions over the
unix socket between applications and session daemon.

The issue here is that with the current approach, we have to repeat
possibly very long descriptions of structures, variants, and enums
because they need to be bundled after every event description.

Here is what I would envision instead:

- We add a "global type" notifier to the UST notfiers (there is currently
  only "event" and "channel"). Those notifier are sending the descriptions
  of channels and events as seen from within the application to
  lttng-sessiond, so it can put it in its ust registry to later generate
  the metadata,
- Those notifiers can be used to send descriptions of enums, structs, and
  variants,
- We'll probably want to have some way of verifying if the global
  type is exactly the same as global types with the same provider and
  global type names registered previously, thus dealing with different
  provider .so versions, each containing different versions of the global
  type. We'll have to find out what's the behavior when this check does
  not succeed (failure or versioning of the global type within sessiond ?).
  This verification will be needed once per global type, per application
  process. This verification needs to be done by sending the provider and
  global type name, along with the global type details (to see if it
  matches).
- Per global type, per application, we can keep a flag that knows whether
  the global type description has been successfully sent to the session
  daemon for each session.

Thanks,

Mathieu

> 
> I'll see what I can do, probably something similar to what is done for
> the events.
> 
> Thanks,
> 
> Geneviève
> >
> > Thanks,
> >
> > Mathieu
> >
> >> Thanks,
> >> Geneviève
> >>> Thanks,
> >>>
> >>> Mathieu
> >>>
> >>>> Signed-off-by: Geneviève Bastien <gbastien+lttng@versatic.net>
> >>>> ---
> >>>>    liblttng-ust-comm/lttng-ust-comm.c | 170
> >>>>    +++++++++++++++++++++++++++++++++----
> >>>>    liblttng-ust-ctl/ustctl.c          |  26 ++++++
> >>>>    2 files changed, 179 insertions(+), 17 deletions(-)
> >>>>
> >>>> diff --git a/liblttng-ust-comm/lttng-ust-comm.c
> >>>> b/liblttng-ust-comm/lttng-ust-comm.c
> >>>> index bc22130..90274c4 100644
> >>>> --- a/liblttng-ust-comm/lttng-ust-comm.c
> >>>> +++ b/liblttng-ust-comm/lttng-ust-comm.c
> >>>> @@ -742,6 +742,7 @@ int serialize_basic_type(enum ustctl_abstract_types
> >>>> *uatype,
> >>>>    	}
> >>>>    	case atype_array:
> >>>>    	case atype_sequence:
> >>>> +	case atype_structure:
> >>>>    	default:
> >>>>    		return -EINVAL;
> >>>>    	}
> >>>> @@ -800,6 +801,14 @@ int serialize_one_type(struct ustctl_type *ut,
> >>>> const
> >>>> struct lttng_type *lt)
> >>>>    		ut->atype = ustctl_atype_sequence;
> >>>>    		break;
> >>>>    	}
> >>>> +	case atype_structure:
> >>>> +	{
> >>>> +		strncpy(ut->u.structure.name, lt->u.structure.name,
> >>>> +						LTTNG_UST_SYM_NAME_LEN);
> >>>> +		ut->u.structure.name[LTTNG_UST_SYM_NAME_LEN - 1] = '\0';
> >>>> +		ut->atype = ustctl_atype_structure;
> >>>> +		break;
> >>>> +	}
> >>>>    	default:
> >>>>    		return -EINVAL;
> >>>>    	}
> >>>> @@ -892,25 +901,82 @@ int serialize_enum(struct ustctl_enum *uenum,
> >>>>    }
> >>>>    
> >>>>    static
> >>>> -int serialize_global_type_decl(size_t *_nr_write_global_type_decl,
> >>>> -		struct ustctl_global_type_decl **ustctl_global_type,
> >>>> +int serialize_structure(struct ustctl_structure *ustruct,
> >>>> +		const struct lttng_structure *lstruct)
> >>>> +{
> >>>> +	int ret;
> >>>> +
> >>>> +	strncpy(ustruct->name, lstruct->name, LTTNG_UST_SYM_NAME_LEN);
> >>>> +	ustruct->name[LTTNG_UST_SYM_NAME_LEN - 1] = '\0';
> >>>> +
> >>>> +	/* Serialize the fields */
> >>>> +	if (lstruct->nr_fields > 0) {
> >>>> +		ret = serialize_fields(&ustruct->nr_fields, &ustruct->fields,
> >>>> +				lstruct->nr_fields,	lstruct->fields);
> >>>> +		if (ret) {
> >>>> +			return ret;
> >>>> +		}
> >>>> +	}
> >>>> +
> >>>> +	return 0;
> >>>> +}
> >>>> +
> >>>> +static size_t get_child_global_type_count(size_t nr_global_type_decl,
> >>>> +		const struct lttng_global_type_decl *lttng_global_type)
> >>>> +{
> >>>> +	size_t total_global_types = 0;
> >>>> +	int i;
> >>>> +	const struct lttng_global_type_decl *lg;
> >>>> +
> >>>> +	for (i = 0; i < nr_global_type_decl; i++) {
> >>>> +		lg = &lttng_global_type[i];
> >>>> +
> >>>> +		if (lg->nowrite)
> >>>> +			continue;
> >>>> +
> >>>> +		/* Count the global types in children as well */
> >>>> +		switch (lg->mtype) {
> >>>> +		case mtype_structure:
> >>>> +		{
> >>>> +			const struct lttng_structure *ls;
> >>>> +
> >>>> +			ls = lg->u.ctf_structure;
> >>>> +			/* Add the number of global types it contains and the count of its
> >>>> own
> >>>> children */
> >>>> +			if (ls->nr_global_type_decl > 0) {
> >>>> +				total_global_types += ls->nr_global_type_decl;
> >>>> +				total_global_types +=
> >>>> get_child_global_type_count(ls->nr_global_type_decl,
> >>>> +								ls->global_type_decl);
> >>>> +			}
> >>>> +			break;
> >>>> +		}
> >>>> +		case mtype_enum:
> >>>> +		default:
> >>>> +			break;
> >>>> +		}
> >>>> +	}
> >>>> +
> >>>> +	return total_global_types;
> >>>> +}
> >>>> +
> >>>> +/*
> >>>> + * When this method is called, index is the next index to write to and
> >>>> the
> >>>> + * ustctl_global_type structure is already initialized and will not
> >>>> overflow.
> >>>> + *
> >>>> + * The global type declarations are flattened at serialization to avoid
> >>>> + * needing to do recursive frees on error.
> >>>> + */
> >>>> +static int _serialize_global_type_decl(size_t *index,
> >>>> +		struct ustctl_global_type_decl *global_type_decl,
> >>>>    		size_t nr_global_type_decl,
> >>>>    		const struct lttng_global_type_decl *lttng_global_type)
> >>>>    {
> >>>> -	struct ustctl_global_type_decl *global_type_decl;
> >>>>    	int i, ret;
> >>>> -	size_t nr_write_global_type_decl = 0;
> >>>> -
> >>>> -	global_type_decl = zmalloc(nr_global_type_decl
> >>>> -					* sizeof(*global_type_decl));
> >>>> -	if (!global_type_decl)
> >>>> -		return -ENOMEM;
> >>>>    
> >>>>    	for (i = 0; i < nr_global_type_decl; i++) {
> >>>>    		struct ustctl_global_type_decl *f;
> >>>>    		const struct lttng_global_type_decl *lf;
> >>>>    
> >>>> -		f = &global_type_decl[nr_write_global_type_decl];
> >>>> +		f = &global_type_decl[*index];
> >>>>    		lf = &lttng_global_type[i];
> >>>>    
> >>>>    		/* skip 'nowrite' fields */
> >>>> @@ -928,18 +994,66 @@ int serialize_global_type_decl(size_t
> >>>> *_nr_write_global_type_decl,
> >>>>    			le = lf->u.ctf_enum;
> >>>>    			ret = serialize_enum(ue, le);
> >>>>    			if (ret)
> >>>> -				goto error;
> >>>> +				return ret;
> >>>>    
> >>>>    			f->mtype = ustctl_mtype_enum;
> >>>>    			break;
> >>>>    		}
> >>>> +		case mtype_structure:
> >>>> +		{
> >>>> +			struct ustctl_structure *us;
> >>>> +			const struct lttng_structure *ls;
> >>>> +
> >>>> +			/* Serialize children global types first */
> >>>> +			ls = lf->u.ctf_structure;
> >>>> +			ret = _serialize_global_type_decl(index,
> >>>> +					global_type_decl,
> >>>> +					ls->nr_global_type_decl,
> >>>> +					ls->global_type_decl);
> >>>> +			if (ret)
> >>>> +				return ret;
> >>>> +
> >>>> +			/* Reinitialize f since the index may have changed */
> >>>> +			f = &global_type_decl[*index];
> >>>> +			us = &f->u.ctf_structure;
> >>>> +
> >>>> +			ret = serialize_structure(us, ls);
> >>>> +			if (ret)
> >>>> +				return ret;
> >>>> +
> >>>> +			f->mtype = ustctl_mtype_structure;
> >>>> +			break;
> >>>> +		}
> >>>>    		default:
> >>>> -			ret = -EINVAL;
> >>>> -			goto error;
> >>>> +			return -EINVAL;
> >>>>    		}
> >>>>    
> >>>> -		nr_write_global_type_decl++;
> >>>> +		*index = *index + 1;
> >>>>    	}
> >>>> +	return 0;
> >>>> +}
> >>>> +
> >>>> +static
> >>>> +int serialize_global_type_decl(size_t *_nr_write_global_type_decl,
> >>>> +		struct ustctl_global_type_decl **ustctl_global_type,
> >>>> +		size_t nr_global_type_decl,
> >>>> +		const struct lttng_global_type_decl *lttng_global_type)
> >>>> +{
> >>>> +	struct ustctl_global_type_decl *global_type_decl;
> >>>> +	int i, ret;
> >>>> +	size_t nr_write_global_type_decl = 0;
> >>>> +	size_t nr_child_global_type_cnt =
> >>>> get_child_global_type_count(nr_global_type_decl,
> >>>> +							lttng_global_type);
> >>>> +
> >>>> +	global_type_decl = zmalloc((nr_global_type_decl +
> >>>> nr_child_global_type_cnt)
> >>>> +					* sizeof(*global_type_decl));
> >>>> +	if (!global_type_decl)
> >>>> +		return -ENOMEM;
> >>>> +
> >>>> +	ret = _serialize_global_type_decl(&nr_write_global_type_decl,
> >>>> global_type_decl,
> >>>> +			nr_global_type_decl, lttng_global_type);
> >>>> +	if (ret)
> >>>> +		goto error;
> >>>>    
> >>>>    	*_nr_write_global_type_decl = nr_write_global_type_decl;
> >>>>    	*ustctl_global_type = global_type_decl;
> >>>> @@ -954,6 +1068,9 @@ error:
> >>>>    		case ustctl_mtype_enum:
> >>>>    			free(m->u.ctf_enum.entries);
> >>>>    			break;
> >>>> +		case ustctl_mtype_structure:
> >>>> +			free(m->u.ctf_structure.fields);
> >>>> +			break;
> >>>>    		default:
> >>>>    			break;
> >>>>    		}
> >>>> @@ -1158,15 +1275,31 @@ int ustcomm_register_event(int sock,
> >>>>    				}
> >>>>    				break;
> >>>>    			}
> >>>> +			case ustctl_mtype_structure:
> >>>> +			{
> >>>> +				int field_len = one_global_type->u.ctf_structure.nr_fields *
> >>>> sizeof(*one_global_type->u.ctf_structure.fields);
> >>>> +
> >>>> +				/* Send the fields */
> >>>> +				DBG("Sending fields for global type structure %s.\n",
> >>>> +						one_global_type->u.ctf_structure.name);
> >>>> +				len = ustcomm_send_unix_sock(sock,
> >>>> one_global_type->u.ctf_structure.fields, field_len);
> >>>> +				free(one_global_type->u.ctf_structure.fields);
> >>>> +				one_global_type->u.ctf_structure.fields = NULL;
> >>>> +				if (len > 0 && len != field_len) {
> >>>> +					goto error_global_type;
> >>>> +				}
> >>>> +				if (len < 0) {
> >>>> +					goto error_global_type;
> >>>> +				}
> >>>> +				break;
> >>>> +			}
> >>>>    			default:
> >>>>    				break;
> >>>>    			}
> >>>>    		}
> >>>> -		free(global_type_decl);
> >>>>    
> >>>> -	} else {
> >>>> -		free(global_type_decl);
> >>>>    	}
> >>>> +	free(global_type_decl);
> >>>>    
> >>>>    	/* receive reply */
> >>>>    	len = ustcomm_recv_unix_sock(sock, &reply, sizeof(reply));
> >>>> @@ -1212,6 +1345,9 @@ error_global_type:
> >>>>    		case ustctl_mtype_enum:
> >>>>    			free(one_global_type->u.ctf_enum.entries);
> >>>>    			break;
> >>>> +		case ustctl_mtype_structure:
> >>>> +			free(one_global_type->u.ctf_structure.fields);
> >>>> +			break;
> >>>>    		default:
> >>>>    			break;
> >>>>    		}
> >>>> diff --git a/liblttng-ust-ctl/ustctl.c b/liblttng-ust-ctl/ustctl.c
> >>>> index a181e62..3100f1b 100644
> >>>> --- a/liblttng-ust-ctl/ustctl.c
> >>>> +++ b/liblttng-ust-ctl/ustctl.c
> >>>> @@ -1896,6 +1896,32 @@ int ustctl_recv_register_event(int sock,
> >>>>    				}
> >>>>    				break;
> >>>>    			}
> >>>> +			case ustctl_mtype_structure:
> >>>> +			{
> >>>> +				int entry_len = one_global_type->u.ctf_structure.nr_fields *
> >>>> sizeof(*one_global_type->u.ctf_structure.fields);
> >>>> +				/* Receive the entries */
> >>>> +				one_global_type->u.ctf_structure.fields = zmalloc(entry_len);
> >>>> +				if (!one_global_type->u.ctf_structure.fields) {
> >>>> +					len = -ENOMEM;
> >>>> +					goto global_type_error;
> >>>> +				}
> >>>> +				len = ustcomm_recv_unix_sock(sock,
> >>>> +						one_global_type->u.ctf_structure.fields,
> >>>> +						entry_len);
> >>>> +				DBG("Received fields for struct %s.\n",
> >>>> one_global_type->u.ctf_structure.name);
> >>>> +				if (len > 0 && len != entry_len) {
> >>>> +					len = -EIO;
> >>>> +					goto global_type_error;
> >>>> +				}
> >>>> +				if (len == 0) {
> >>>> +					len = -EPIPE;
> >>>> +					goto global_type_error;
> >>>> +				}
> >>>> +				if (len < 0) {
> >>>> +					goto global_type_error;
> >>>> +				}
> >>>> +				break;
> >>>> +			}
> >>>>    			default:
> >>>>    				break;
> >>>>    			}
> >>>> --
> >>>> 1.9.1
> >>>>
> >>>>
> >>>> _______________________________________________
> >>>> lttng-dev mailing list
> >>>> lttng-dev@lists.lttng.org
> >>>> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
> >>>>
> >>
> 
> 

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Re: [RFC Patch Ust 2/5] Serialize the CTF global structures for ust-comm

[Thibault, Daniel]

----------------------------------------------------------------------
> Date: Fri, 2 May 2014 09:29:09 +0000 (UTC)
> From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> To: Geneviève Bastien <gbastien+lttng@versatic.net>

> Yes, all this is about not repeating global type descriptions over the Unix socket between applications and session daemon.
>
> The issue here is that with the current approach, we have to repeat possibly very long descriptions of structures, variants, and enums because they need to be bundled after every event description.
>
> Here is what I would envision instead:
>
> - We add a "global type" notifier to the UST notifiers (there is currently only "event" and "channel"). Those notifiers are sending the descriptions
>   of channels and events as seen from within the application to lttng-sessiond, so it can put it in its ust registry to later generate
>   the metadata.
> - Those notifiers can be used to send descriptions of enums, structs, and variants.
> - We'll probably want to have some way of verifying if the global type is exactly the same as global types with the same provider and
>   global type names registered previously, thus dealing with different provider .so versions, each containing different versions of the global
>   type. We'll have to find out what's the behaviour when this check does not succeed (failure or versioning of the global type within sessiond ?).
>   This verification will be needed once per global type, per application process. This verification needs to be done by sending the provider and
>   global type name, along with the global type details (to see if it matches).
> - Per global type, per application, we can keep a flag that knows whether the global type description has been successfully sent to the session
>   daemon for each session.

   I'd strongly advise to go the versioning way rather than the failure way (although failure is easier to implement initially).  The verification needs to be done more often than "once per global type, per application process": one use case is a process that dynamically loads (dlopen) an instrumented .so, generates events with it for a while, then unloads it (dlclose) and loads another .so that happens to include a provider with the same signature but different event payloads.  The process may or may not wholly unregister from and re-register with the session manager, depending on whether or not it has other providers loaded.

   See the closely related bugs #561, #698, #720.

   If the bandwidth used in sending lengthy type descriptions for mere verification is a concern, maybe the conversation between the app tracepoint provider and session manager could start by sending an MD5 or SHA hash of the type description.  Only when those mismatch would the full type description need to be sent.

   Also note that event type verification can be "lazy" (or "delayed") and occur only when the event type in question is actually enabled or emitted.  There isn't much point to updating the trace metadata with changing event types if the events in question are not enabled (at the channel or event level).  On the other hand, sending a filter specification to a provider pretty much requires event type verification, if only to warn the user that the filter he specified for the event won't be understood by the provider.

Daniel U. Thibault
Protection des systèmes et contremesures (PSC) | Systems Protection & Countermeasures (SPC)
Cyber sécurité pour les missions essentielles (CME) | Mission Critical Cyber Security (MCCS)
R & D pour la défense Canada - Valcartier (RDDC Valcartier) | Defence R&D Canada - Valcartier (DRDC Valcartier)
2459 route de la Bravoure
Québec QC  G3J 1X5
CANADA
Vox : (418) 844-4000 x4245
Fax : (418) 844-4538
NAC : 918V QSDJ <http://www.travelgis.com/map.asp?addr=918V%20QSDJ>
Gouvernement du Canada | Government of Canada
<http://www.valcartier.drdc-rddc.gc.ca/>