Mirror of the lttng-dev mailing list
 help / color / mirror / Atom feed
* [lttng-dev] [PATCH] Expose kernel tracer to user-space
@ 2012-06-05 23:24 Francis Giraldeau
  2012-06-05 23:40 ` Francis Giraldeau
  2012-06-26  6:40 ` Mathieu Desnoyers
  0 siblings, 2 replies; 5+ messages in thread
From: Francis Giraldeau @ 2012-06-05 23:24 UTC (permalink / raw)


By writing to the file /proc/lttng, a user-space application creates a
kernel event. The event's payload is by default UTF-8 text, but any data
can be written, up to 1024 bytes. Null-character is optional and is not
enforced. The event uses sequence for space efficiency and to store any
data as payload.

Update: split the probe code into it's own module and make it an optional
feature of lttng-abi. The feature is enabled when the module is loaded. The
lttng-abi module exports a register function and includes a wrapper for
lttng_fops write. This is required since struct file_operations must be const.
Since the module dependency is reversed, unloading the lttng-uevent module is
done only when it's not used anymore. This is done with rwlock synchronisation.
The synchronisation doesn't prevent starvation, but this situation is unlikely
and can be prevented by stop active tracing sessions.

Signed-off-by: Francis Giraldeau <francis.giraldeau at gmail.com>
---
 instrumentation/events/lttng-module/uevent.h |   33 +++++++++++++++
 lttng-abi.c                                  |   42 +++++++++++++++++++
 lttng-abi.h                                  |    3 ++
 probes/Makefile                              |    2 +
 probes/lttng-probe-uevent.c                  |   36 ++++++++++++++++
 probes/lttng-uevent.c                        |   58 ++++++++++++++++++++++++++
 6 files changed, 174 insertions(+)
 create mode 100644 instrumentation/events/lttng-module/uevent.h
 create mode 100644 probes/lttng-probe-uevent.c
 create mode 100644 probes/lttng-uevent.c

diff --git a/instrumentation/events/lttng-module/uevent.h b/instrumentation/events/lttng-module/uevent.h
new file mode 100644
index 0000000..f67d901
--- /dev/null
+++ b/instrumentation/events/lttng-module/uevent.h
@@ -0,0 +1,33 @@
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM uevent
+
+#if !defined(UEVENT_H_) || defined(TRACE_HEADER_MULTI_READ)
+#define UEVENT_H_
+
+#include <linux/tracepoint.h>
+
+TRACE_EVENT(lttng_uevent,
+
+	TP_PROTO(const char *str, size_t len),
+
+	TP_ARGS(str, len),
+
+	/*
+	 * Uses sequence to hold variable size data, by default considered
+	 * as text. Null-terminal character is optional and is not enforced.
+	 */
+	TP_STRUCT__entry(
+		__dynamic_array_text(char, text, len)
+	),
+
+	TP_fast_assign(
+		tp_memcpy_dyn_from_user(text, str)
+	),
+
+	TP_printk("")
+)
+
+#endif /* UEVENT_H_ */
+
+/* This part must be outside protection */
+#include "../../../probes/define_trace.h"
diff --git a/lttng-abi.c b/lttng-abi.c
index 26a02ed..b8e6b57 100644
--- a/lttng-abi.c
+++ b/lttng-abi.c
@@ -51,6 +51,12 @@
 #include "lttng-tracer.h"
 
 /*
+ * Required data structures to support lttng-probe-uevent
+ */
+DEFINE_RWLOCK(uevent_rwlock);
+write_ops_t lttng_uevent_handler;
+
+/*
  * This is LTTng's own personal way to create a system call as an external
  * module. We use ioctl() on /proc/lttng.
  */
@@ -252,9 +258,45 @@ long lttng_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	}
 }
 
+/*
+ * lttng_uevent_set_handler - set handler functions for uevent
+ *
+ * Access to handler code is protected with rwlock in order to
+ * prevent the optional module to be removed while in use.
+ */
+
+void lttng_uevent_set_handler(write_ops_t handler)
+{
+	write_lock(&uevent_rwlock);
+	lttng_uevent_handler = handler;
+	write_unlock(&uevent_rwlock);
+}
+EXPORT_SYMBOL_GPL(lttng_uevent_set_handler);
+
+/*
+ * lttng_write_uevent - expose kernel tracer to user-space
+ */
+
+static
+ssize_t lttng_write_uevent(struct file *file, const char __user *ubuf,
+		size_t count, loff_t *fpos)
+{
+	int ret;
+
+	read_lock(&uevent_rwlock);
+	if (unlikely(lttng_uevent_handler == NULL)) {
+		read_unlock(&uevent_rwlock);
+		return -ENOSYS;
+	}
+	ret = (*lttng_uevent_handler)(file, ubuf, count, fpos);
+	read_unlock(&uevent_rwlock);
+	return ret;
+}
+
 static const struct file_operations lttng_fops = {
 	.owner = THIS_MODULE,
 	.unlocked_ioctl = lttng_ioctl,
+	.write = lttng_write_uevent,
 #ifdef CONFIG_COMPAT
 	.compat_ioctl = lttng_ioctl,
 #endif
diff --git a/lttng-abi.h b/lttng-abi.h
index dc230d8..f4a8c0c 100644
--- a/lttng-abi.h
+++ b/lttng-abi.h
@@ -27,6 +27,9 @@
 
 #define LTTNG_KERNEL_SYM_NAME_LEN	256
 
+typedef	ssize_t (*write_ops_t) (struct file *, const char __user *, size_t, loff_t *);
+void lttng_uevent_set_handler(write_ops_t handler);
+
 enum lttng_kernel_instrumentation {
 	LTTNG_KERNEL_TRACEPOINT	= 0,
 	LTTNG_KERNEL_KPROBE	= 1,
diff --git a/probes/Makefile b/probes/Makefile
index 698a9c9..a895e60 100644
--- a/probes/Makefile
+++ b/probes/Makefile
@@ -14,6 +14,8 @@ obj-m += lttng-probe-sched.o
 obj-m += lttng-probe-irq.o
 obj-m += lttng-probe-signal.o
 obj-m += lttng-probe-timer.o
+obj-m += lttng-probe-uevent.o
+obj-m += lttng-uevent.o
 
 obj-m += lttng-probe-statedump.o
 
diff --git a/probes/lttng-probe-uevent.c b/probes/lttng-probe-uevent.c
new file mode 100644
index 0000000..90abb5e
--- /dev/null
+++ b/probes/lttng-probe-uevent.c
@@ -0,0 +1,36 @@
+/*
+ * probes/lttng-probe-uevent.c
+ *
+ * Expose kernel tracer to user-space through /proc/lttng
+ *
+ * Copyright (C) 2009-2012 Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; only
+ * version 2.1 of the License.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include <linux/module.h>
+
+/*
+ * Create lttng_uevent tracepoint probes.
+ */
+#define LTTNG_PACKAGE_BUILD
+#define CREATE_TRACE_POINTS
+#define TRACE_INCLUDE_PATH ../instrumentation/events/lttng-module
+
+#include "../instrumentation/events/lttng-module/uevent.h"
+
+MODULE_LICENSE("GPL and additional rights");
+MODULE_AUTHOR("Mathieu Desnoyers <mathieu.desnoyers at efficios.com>");
+MODULE_DESCRIPTION("LTTng uevent probes");
diff --git a/probes/lttng-uevent.c b/probes/lttng-uevent.c
new file mode 100644
index 0000000..7b4bffc
--- /dev/null
+++ b/probes/lttng-uevent.c
@@ -0,0 +1,58 @@
+/*
+ * probes/lttng-uevent.c
+ *
+ * Expose kernel tracer to user-space through /proc/lttng
+ *
+ * Copyright (C) 2012 Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; only
+ * version 2.1 of the License.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include <linux/module.h>
+#include "../lttng-abi.h"
+
+/* include our own uevent tracepoint */
+#include "../instrumentation/events/lttng-module/uevent.h"
+DEFINE_TRACE(lttng_uevent);
+
+#define LTTNG_UEVENT_SIZE 1024
+
+ssize_t uevent_write_handler(struct file *file, const char __user *ubuf,
+		size_t count, loff_t *fpos)
+{
+	if (count > LTTNG_UEVENT_SIZE)
+		count = LTTNG_UEVENT_SIZE;
+
+	trace_lttng_uevent(ubuf, count);
+	return count;
+}
+
+static int __init lttng_probe_uevent_init(void)
+{
+	lttng_uevent_set_handler(uevent_write_handler);
+	return 0;
+}
+
+static void __exit lttng_probe_uevent_exit(void)
+{
+	lttng_uevent_set_handler(NULL);
+}
+
+module_init(lttng_probe_uevent_init);
+module_exit(lttng_probe_uevent_exit);
+
+MODULE_LICENSE("GPL and additional rights");
+MODULE_AUTHOR("Mathieu Desnoyers <mathieu.desnoyers at efficios.com>");
+MODULE_DESCRIPTION("LTTng kernel event from user-space");
-- 
1.7.9.5




^ permalink raw reply	[flat|nested] 5+ messages in thread

* [lttng-dev] [PATCH] Expose kernel tracer to user-space
  2012-06-05 23:24 [lttng-dev] [PATCH] Expose kernel tracer to user-space Francis Giraldeau
@ 2012-06-05 23:40 ` Francis Giraldeau
  2012-06-26  6:40 ` Mathieu Desnoyers
  1 sibling, 0 replies; 5+ messages in thread
From: Francis Giraldeau @ 2012-06-05 23:40 UTC (permalink / raw)


Le 2012-06-06 01:24, Francis Giraldeau a ?crit :
> By writing to the file /proc/lttng, a user-space application creates a
> kernel event. The event's payload is by default UTF-8 text, but any data
> can be written, up to 1024 bytes. Null-character is optional and is not
> enforced. The event uses sequence for space efficiency and to store any
> data as payload.
>
> Update: split the probe code into it's own module and make it an optional
> feature of lttng-abi. The feature is enabled when the module is loaded. The
> lttng-abi module exports a register function and includes a wrapper for
> lttng_fops write. This is required since struct file_operations must be const.
> Since the module dependency is reversed, unloading the lttng-uevent module is
> done only when it's not used anymore. This is done with rwlock synchronisation.
> The synchronisation doesn't prevent starvation, but this situation is unlikely
> and can be prevented by stop active tracing sessions.

One little gotcha: after modprobe lttng-uevent, the module
lttng-probe-uevent must be loaded manually, otherwise lttng_uevent is
not listed with "lttng list -k". Is there a way to make the probe load
automatically when lttng-uevent is loaded?

Cheers,

Francis


-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 4476 bytes
Desc: Signature cryptographique S/MIME
URL: <http://lists.lttng.org/pipermail/lttng-dev/attachments/20120606/199bca17/attachment-0001.bin>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [lttng-dev] [PATCH] Expose kernel tracer to user-space
  2012-06-05 23:24 [lttng-dev] [PATCH] Expose kernel tracer to user-space Francis Giraldeau
  2012-06-05 23:40 ` Francis Giraldeau
@ 2012-06-26  6:40 ` Mathieu Desnoyers
  2012-06-26 14:52   ` Francis Giraldeau
  1 sibling, 1 reply; 5+ messages in thread
From: Mathieu Desnoyers @ 2012-06-26  6:40 UTC (permalink / raw)


* Francis Giraldeau (francis.giraldeau at gmail.com) wrote:
> By writing to the file /proc/lttng, a user-space application creates a
> kernel event. The event's payload is by default UTF-8 text, but any data
> can be written, up to 1024 bytes. Null-character is optional and is not
> enforced. The event uses sequence for space efficiency and to store any
> data as payload.
> 
> Update: split the probe code into it's own module and make it an optional
> feature of lttng-abi. The feature is enabled when the module is loaded. The
> lttng-abi module exports a register function and includes a wrapper for
> lttng_fops write. This is required since struct file_operations must be const.
> Since the module dependency is reversed, unloading the lttng-uevent module is
> done only when it's not used anymore. This is done with rwlock synchronisation.
> The synchronisation doesn't prevent starvation, but this situation is unlikely
> and can be prevented by stop active tracing sessions.
> 
> Signed-off-by: Francis Giraldeau <francis.giraldeau at gmail.com>
> ---
>  instrumentation/events/lttng-module/uevent.h |   33 +++++++++++++++
>  lttng-abi.c                                  |   42 +++++++++++++++++++
>  lttng-abi.h                                  |    3 ++
>  probes/Makefile                              |    2 +
>  probes/lttng-probe-uevent.c                  |   36 ++++++++++++++++
>  probes/lttng-uevent.c                        |   58 ++++++++++++++++++++++++++
>  6 files changed, 174 insertions(+)
>  create mode 100644 instrumentation/events/lttng-module/uevent.h
>  create mode 100644 probes/lttng-probe-uevent.c
>  create mode 100644 probes/lttng-uevent.c
> 
> diff --git a/instrumentation/events/lttng-module/uevent.h b/instrumentation/events/lttng-module/uevent.h
> new file mode 100644
> index 0000000..f67d901
> --- /dev/null
> +++ b/instrumentation/events/lttng-module/uevent.h
> @@ -0,0 +1,33 @@
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM uevent
> +
> +#if !defined(UEVENT_H_) || defined(TRACE_HEADER_MULTI_READ)
> +#define UEVENT_H_
> +
> +#include <linux/tracepoint.h>
> +
> +TRACE_EVENT(lttng_uevent,
> +
> +	TP_PROTO(const char *str, size_t len),
> +
> +	TP_ARGS(str, len),
> +
> +	/*
> +	 * Uses sequence to hold variable size data, by default considered
> +	 * as text. Null-terminal character is optional and is not enforced.
> +	 */
> +	TP_STRUCT__entry(
> +		__dynamic_array_text(char, text, len)
> +	),
> +
> +	TP_fast_assign(
> +		tp_memcpy_dyn_from_user(text, str)
> +	),
> +
> +	TP_printk("")
> +)
> +
> +#endif /* UEVENT_H_ */
> +
> +/* This part must be outside protection */
> +#include "../../../probes/define_trace.h"
> diff --git a/lttng-abi.c b/lttng-abi.c
> index 26a02ed..b8e6b57 100644
> --- a/lttng-abi.c
> +++ b/lttng-abi.c
> @@ -51,6 +51,12 @@
>  #include "lttng-tracer.h"
>  
>  /*
> + * Required data structures to support lttng-probe-uevent
> + */
> +DEFINE_RWLOCK(uevent_rwlock);

Not required.

> +write_ops_t lttng_uevent_handler;

this should be static.

> +
> +/*
>   * This is LTTng's own personal way to create a system call as an external
>   * module. We use ioctl() on /proc/lttng.
>   */
> @@ -252,9 +258,45 @@ long lttng_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
>  	}
>  }
>  
> +/*
> + * lttng_uevent_set_handler - set handler functions for uevent
> + *
> + * Access to handler code is protected with rwlock in order to
> + * prevent the optional module to be removed while in use.
> + */
> +
> +void lttng_uevent_set_handler(write_ops_t handler)
> +{
> +	write_lock(&uevent_rwlock);

write lock not necessary.

if (!lttng_uevent_set_handler)
  release refcount in prior handler's module.

then:
take a refcount on the module that contains the handler address.
(explicit)

> +	lttng_uevent_handler = handler;

Just declare the lttng_uevent_handler as "volatile".

> +	write_unlock(&uevent_rwlock);
> +}
> +EXPORT_SYMBOL_GPL(lttng_uevent_set_handler);
> +
> +/*
> + * lttng_write_uevent - expose kernel tracer to user-space
> + */
> +
> +static
> +ssize_t lttng_write_uevent(struct file *file, const char __user *ubuf,
> +		size_t count, loff_t *fpos)
> +{
> +	int ret;
> +
> +	read_lock(&uevent_rwlock);
> +	if (unlikely(lttng_uevent_handler == NULL)) {
> +		read_unlock(&uevent_rwlock);
> +		return -ENOSYS;

instead of read_lock, please do:

write_ops_t uev_handler;

uev_handler = ACCESS_ONCE(lttng_uevent_handler);
if (!uev_handler)
  return -ENOSYS;
return uev_handler(file, ubuf, count, fpos);

Thanks,

Mathieu


> +	}
> +	ret = (*lttng_uevent_handler)(file, ubuf, count, fpos);
> +	read_unlock(&uevent_rwlock);
> +	return ret;
> +}
> +
>  static const struct file_operations lttng_fops = {
>  	.owner = THIS_MODULE,

The THIS_MODULE owner protects the module that contains lttng-abi.c from
unloading when the handling is running. (keeps a refcount) However, it
does not protect the handler probe module from unloading. This is why we
need a refcount.

>  	.unlocked_ioctl = lttng_ioctl,
> +	.write = lttng_write_uevent,
>  #ifdef CONFIG_COMPAT
>  	.compat_ioctl = lttng_ioctl,
>  #endif
> diff --git a/lttng-abi.h b/lttng-abi.h
> index dc230d8..f4a8c0c 100644
> --- a/lttng-abi.h
> +++ b/lttng-abi.h
> @@ -27,6 +27,9 @@
>  
>  #define LTTNG_KERNEL_SYM_NAME_LEN	256
>  
> +typedef	ssize_t (*write_ops_t) (struct file *, const char __user *, size_t, loff_t *);
> +void lttng_uevent_set_handler(write_ops_t handler);
> +
>  enum lttng_kernel_instrumentation {
>  	LTTNG_KERNEL_TRACEPOINT	= 0,
>  	LTTNG_KERNEL_KPROBE	= 1,
> diff --git a/probes/Makefile b/probes/Makefile
> index 698a9c9..a895e60 100644
> --- a/probes/Makefile
> +++ b/probes/Makefile
> @@ -14,6 +14,8 @@ obj-m += lttng-probe-sched.o
>  obj-m += lttng-probe-irq.o
>  obj-m += lttng-probe-signal.o
>  obj-m += lttng-probe-timer.o
> +obj-m += lttng-probe-uevent.o
> +obj-m += lttng-uevent.o
>  
>  obj-m += lttng-probe-statedump.o
>  
> diff --git a/probes/lttng-probe-uevent.c b/probes/lttng-probe-uevent.c
> new file mode 100644
> index 0000000..90abb5e
> --- /dev/null
> +++ b/probes/lttng-probe-uevent.c
> @@ -0,0 +1,36 @@
> +/*
> + * probes/lttng-probe-uevent.c
> + *
> + * Expose kernel tracer to user-space through /proc/lttng
> + *
> + * Copyright (C) 2009-2012 Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; only
> + * version 2.1 of the License.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#include <linux/module.h>
> +
> +/*
> + * Create lttng_uevent tracepoint probes.
> + */
> +#define LTTNG_PACKAGE_BUILD
> +#define CREATE_TRACE_POINTS
> +#define TRACE_INCLUDE_PATH ../instrumentation/events/lttng-module
> +
> +#include "../instrumentation/events/lttng-module/uevent.h"
> +
> +MODULE_LICENSE("GPL and additional rights");
> +MODULE_AUTHOR("Mathieu Desnoyers <mathieu.desnoyers at efficios.com>");
> +MODULE_DESCRIPTION("LTTng uevent probes");
> diff --git a/probes/lttng-uevent.c b/probes/lttng-uevent.c
> new file mode 100644
> index 0000000..7b4bffc
> --- /dev/null
> +++ b/probes/lttng-uevent.c
> @@ -0,0 +1,58 @@
> +/*
> + * probes/lttng-uevent.c
> + *
> + * Expose kernel tracer to user-space through /proc/lttng
> + *
> + * Copyright (C) 2012 Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; only
> + * version 2.1 of the License.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#include <linux/module.h>
> +#include "../lttng-abi.h"
> +
> +/* include our own uevent tracepoint */
> +#include "../instrumentation/events/lttng-module/uevent.h"
> +DEFINE_TRACE(lttng_uevent);
> +
> +#define LTTNG_UEVENT_SIZE 1024
> +
> +ssize_t uevent_write_handler(struct file *file, const char __user *ubuf,
> +		size_t count, loff_t *fpos)
> +{
> +	if (count > LTTNG_UEVENT_SIZE)
> +		count = LTTNG_UEVENT_SIZE;
> +
> +	trace_lttng_uevent(ubuf, count);
> +	return count;
> +}
> +
> +static int __init lttng_probe_uevent_init(void)
> +{
> +	lttng_uevent_set_handler(uevent_write_handler);
> +	return 0;
> +}
> +
> +static void __exit lttng_probe_uevent_exit(void)
> +{
> +	lttng_uevent_set_handler(NULL);
> +}
> +
> +module_init(lttng_probe_uevent_init);
> +module_exit(lttng_probe_uevent_exit);
> +
> +MODULE_LICENSE("GPL and additional rights");
> +MODULE_AUTHOR("Mathieu Desnoyers <mathieu.desnoyers at efficios.com>");
> +MODULE_DESCRIPTION("LTTng kernel event from user-space");
> -- 
> 1.7.9.5
> 
> 
> _______________________________________________
> lttng-dev mailing list
> lttng-dev at lists.lttng.org
> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [lttng-dev] [PATCH] Expose kernel tracer to user-space
  2012-06-26  6:40 ` Mathieu Desnoyers
@ 2012-06-26 14:52   ` Francis Giraldeau
  2012-06-26 17:01     ` Mathieu Desnoyers
  0 siblings, 1 reply; 5+ messages in thread
From: Francis Giraldeau @ 2012-06-26 14:52 UTC (permalink / raw)


Le 2012-06-26 08:40, Mathieu Desnoyers a ?crit :
>> +/*
>> + * lttng_uevent_set_handler - set handler functions for uevent
>> + *
>> + * Access to handler code is protected with rwlock in order to
>> + * prevent the optional module to be removed while in use.
>> + */
>> +
>> +void lttng_uevent_set_handler(write_ops_t handler)
>> +{
>> +	write_lock(&uevent_rwlock);
> 
> write lock not necessary.
> 
> if (!lttng_uevent_set_handler)
>   release refcount in prior handler's module.
> 
> then:
> take a refcount on the module that contains the handler address.
> (explicit)


The function now looks like this:

void lttng_uevent_set_handler(struct file_operations *fops)
{
	if (lttng_uevent_handler) {
		module_put(lttng_uevent_handler->owner);
	}
	if (fops && try_module_get(fops->owner)) {
		lttng_uevent_handler = fops;
		return;
	}
	lttng_uevent_handler = NULL;
	return;
}

The problem with this approach is that lttng_uevent can't be unloaded
anymore:

  rmmod lttng_uevent
  ERROR: Module lttng_uevent is in use

The module lttng_uevent can't be unloaded without unloading the module
lttng_tracer. The function module_exit of lttng_uevent is never called,
such that there is a chicken-and-egg situation. This problem was solved
by using rwlock to access the handler pointer. In the rare event when
unload module would occur while a trace event is recorded, the write
lock was preventing to change the handler pointer while it's in use. But
maybe there is another way to do it?

Thanks!

Francis Giraldeau

-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 4476 bytes
Desc: Signature cryptographique S/MIME
URL: <http://lists.lttng.org/pipermail/lttng-dev/attachments/20120626/3dcfc15b/attachment.bin>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [lttng-dev] [PATCH] Expose kernel tracer to user-space
  2012-06-26 14:52   ` Francis Giraldeau
@ 2012-06-26 17:01     ` Mathieu Desnoyers
  0 siblings, 0 replies; 5+ messages in thread
From: Mathieu Desnoyers @ 2012-06-26 17:01 UTC (permalink / raw)


* Francis Giraldeau (francis.giraldeau at gmail.com) wrote:
> Le 2012-06-26 08:40, Mathieu Desnoyers a ?crit :
> >> +/*
> >> + * lttng_uevent_set_handler - set handler functions for uevent
> >> + *
> >> + * Access to handler code is protected with rwlock in order to
> >> + * prevent the optional module to be removed while in use.
> >> + */
> >> +
> >> +void lttng_uevent_set_handler(write_ops_t handler)
> >> +{
> >> +	write_lock(&uevent_rwlock);
> > 
> > write lock not necessary.
> > 
> > if (!lttng_uevent_set_handler)
> >   release refcount in prior handler's module.
> > 
> > then:
> > take a refcount on the module that contains the handler address.
> > (explicit)
> 
> 
> The function now looks like this:
> 
> void lttng_uevent_set_handler(struct file_operations *fops)
> {
> 	if (lttng_uevent_handler) {
> 		module_put(lttng_uevent_handler->owner);
> 	}
> 	if (fops && try_module_get(fops->owner)) {
> 		lttng_uevent_handler = fops;
> 		return;
> 	}
> 	lttng_uevent_handler = NULL;
> 	return;
> }
> 
> The problem with this approach is that lttng_uevent can't be unloaded
> anymore:
> 
>   rmmod lttng_uevent
>   ERROR: Module lttng_uevent is in use
> 
> The module lttng_uevent can't be unloaded without unloading the module
> lttng_tracer. The function module_exit of lttng_uevent is never called,
> such that there is a chicken-and-egg situation. This problem was solved
> by using rwlock to access the handler pointer. In the rare event when
> unload module would occur while a trace event is recorded, the write
> lock was preventing to change the handler pointer while it's in use. But
> maybe there is another way to do it?

Good point, circular dependency on the destructor is not good, because
there is no way to unload the module.

Another way to do it is to use RCU. module unload contains a
synchronize_rcu() call, so you can protect your handler with:

/* Using RCU to protect against module removal */
rcu_read_lock();
handler = rcu_dereference(lttng_uevent_handler);
ret = handler(...)));
rcu_read_unlock;
return ret;

Thanks,

Mathieu

> 
> Thanks!
> 
> Francis Giraldeau
> 



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


-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2012-06-26 17:01 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-06-05 23:24 [lttng-dev] [PATCH] Expose kernel tracer to user-space Francis Giraldeau
2012-06-05 23:40 ` Francis Giraldeau
2012-06-26  6:40 ` Mathieu Desnoyers
2012-06-26 14:52   ` Francis Giraldeau
2012-06-26 17:01     ` Mathieu Desnoyers

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox