Re: [lttng-dev] [RFC PATCH lttng-modules v2 1/2] Create a memory pool for temporary tracepoint probes storage

2018-02-23 Thread Mathieu Desnoyers
both patches merged into master, 2.10, 2.9, thanks Julien!

Mathieu

- On Feb 23, 2018, at 11:37 AM, Julien Desfossez jdesfos...@efficios.com 
wrote:

> This memory pool is created when the lttng-tracer module is loaded. It
> allocates 4 buffers of 4k on each CPU. These buffers are designed to
> allow tracepoint probes to temporarily store data that does not fit on
> the stack (during the code_pre and code_post phases). The memory is
> freed when the lttng-tracer module is unloaded.
> 
> This removes the need for dynamic allocation during the execution of
> tracepoint probes, which does not behave well on PREEMPT_RT kernel, even
> when invoked with the GFP_ATOMIC | GFP_NOWAIT flags.
> 
> Signed-off-by: Julien Desfossez 
> ---
> Makefile   |   3 +-
> lttng-abi.c|   9 +++
> lttng-tp-mempool.c | 172 +
> lttng-tp-mempool.h |  63 
> 4 files changed, 246 insertions(+), 1 deletion(-)
> create mode 100644 lttng-tp-mempool.c
> create mode 100644 lttng-tp-mempool.h
> 
> diff --git a/Makefile b/Makefile
> index 2cd2df0..b08f0bf 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -59,7 +59,8 @@ ifneq ($(KERNELRELEASE),)
>lttng-filter.o lttng-filter-interpreter.o \
>lttng-filter-specialize.o \
>lttng-filter-validator.o \
> -   probes/lttng-probe-user.o
> +   probes/lttng-probe-user.o \
> +   lttng-tp-mempool.o
> 
>   ifneq ($(CONFIG_HAVE_SYSCALL_TRACEPOINTS),)
> lttng-tracer-objs += lttng-syscalls.o
> diff --git a/lttng-abi.c b/lttng-abi.c
> index d202b72..ea746c2 100644
> --- a/lttng-abi.c
> +++ b/lttng-abi.c
> @@ -56,6 +56,7 @@
> #include 
> #include 
> #include 
> +#include 
> #include 
> 
> /*
> @@ -1771,6 +1772,12 @@ int __init lttng_abi_init(void)
> 
>   wrapper_vmalloc_sync_all();
>   lttng_clock_ref();
> +
> + ret = lttng_tp_mempool_init();
> + if (ret) {
> + goto error;
> + }
> +
>   lttng_proc_dentry = proc_create_data("lttng", S_IRUSR | S_IWUSR, NULL,
>   _fops, NULL);
> 
> @@ -1783,6 +1790,7 @@ int __init lttng_abi_init(void)
>   return 0;
> 
> error:
> + lttng_tp_mempool_destroy();
>   lttng_clock_unref();
>   return ret;
> }
> @@ -1790,6 +1798,7 @@ error:
> /* No __exit annotation because used by init error path too. */
> void lttng_abi_exit(void)
> {
> + lttng_tp_mempool_destroy();
>   lttng_clock_unref();
>   if (lttng_proc_dentry)
>   remove_proc_entry("lttng", NULL);
> diff --git a/lttng-tp-mempool.c b/lttng-tp-mempool.c
> new file mode 100644
> index 000..d984bd4
> --- /dev/null
> +++ b/lttng-tp-mempool.c
> @@ -0,0 +1,172 @@
> +/*
> + * lttng-tp-mempool.c
> + *
> + * Copyright (C) 2018 Julien Desfossez 
> + *
> + * 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 
> +#include 
> +
> +#include 
> +
> +struct lttng_tp_buf_entry {
> + int cpu; /* To make sure we return the entry to the right pool. */
> + char buf[LTTNG_TP_MEMPOOL_BUF_SIZE];
> + struct list_head list;
> +};
> +
> +/*
> + * No exclusive access strategy for now, this memory pool is currently only
> + * used from a non-preemptible context, and the interrupt tracepoint probes 
> do
> + * not use this facility.
> + */
> +struct per_cpu_buf {
> + struct list_head free_list; /* Free struct lttng_tp_buf_entry. */
> +};
> +
> +static struct per_cpu_buf __percpu *pool; /* Per-cpu buffer. */
> +
> +int lttng_tp_mempool_init(void)
> +{
> + int ret, cpu;
> +
> + /* The pool is only supposed to be allocated once. */
> + if (pool) {
> + WARN_ON_ONCE(1);
> + ret = -1;
> + goto end;
> + }
> +
> + pool = alloc_percpu(struct per_cpu_buf);
> + if (!pool) {
> + ret = -ENOMEM;
> + goto end;
> + }
> +
> + for_each_possible_cpu(cpu) {
> + struct per_cpu_buf *cpu_buf = per_cpu_ptr(pool, cpu);
> +
> + INIT_LIST_HEAD(_buf->free_list);
> + }
> +
> + for_each_possible_cpu(cpu) {
> + int i;
> + struct per_cpu_buf *cpu_buf = 

Re: [lttng-dev] [RFC PATCH lttng-modules v2 1/2] Create a memory pool for temporary tracepoint probes storage

2018-02-23 Thread Mathieu Desnoyers
Both patches look good !

For both:

Acked-by: Mathieu Desnoyers 

I notice the RFC tag. What is missing for merging them as fixes ?

Thanks,

Mathieu

- On Feb 23, 2018, at 11:37 AM, Julien Desfossez jdesfos...@efficios.com 
wrote:

> This memory pool is created when the lttng-tracer module is loaded. It
> allocates 4 buffers of 4k on each CPU. These buffers are designed to
> allow tracepoint probes to temporarily store data that does not fit on
> the stack (during the code_pre and code_post phases). The memory is
> freed when the lttng-tracer module is unloaded.
> 
> This removes the need for dynamic allocation during the execution of
> tracepoint probes, which does not behave well on PREEMPT_RT kernel, even
> when invoked with the GFP_ATOMIC | GFP_NOWAIT flags.
> 
> Signed-off-by: Julien Desfossez 
> ---
> Makefile   |   3 +-
> lttng-abi.c|   9 +++
> lttng-tp-mempool.c | 172 +
> lttng-tp-mempool.h |  63 
> 4 files changed, 246 insertions(+), 1 deletion(-)
> create mode 100644 lttng-tp-mempool.c
> create mode 100644 lttng-tp-mempool.h
> 
> diff --git a/Makefile b/Makefile
> index 2cd2df0..b08f0bf 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -59,7 +59,8 @@ ifneq ($(KERNELRELEASE),)
>lttng-filter.o lttng-filter-interpreter.o \
>lttng-filter-specialize.o \
>lttng-filter-validator.o \
> -   probes/lttng-probe-user.o
> +   probes/lttng-probe-user.o \
> +   lttng-tp-mempool.o
> 
>   ifneq ($(CONFIG_HAVE_SYSCALL_TRACEPOINTS),)
> lttng-tracer-objs += lttng-syscalls.o
> diff --git a/lttng-abi.c b/lttng-abi.c
> index d202b72..ea746c2 100644
> --- a/lttng-abi.c
> +++ b/lttng-abi.c
> @@ -56,6 +56,7 @@
> #include 
> #include 
> #include 
> +#include 
> #include 
> 
> /*
> @@ -1771,6 +1772,12 @@ int __init lttng_abi_init(void)
> 
>   wrapper_vmalloc_sync_all();
>   lttng_clock_ref();
> +
> + ret = lttng_tp_mempool_init();
> + if (ret) {
> + goto error;
> + }
> +
>   lttng_proc_dentry = proc_create_data("lttng", S_IRUSR | S_IWUSR, NULL,
>   _fops, NULL);
> 
> @@ -1783,6 +1790,7 @@ int __init lttng_abi_init(void)
>   return 0;
> 
> error:
> + lttng_tp_mempool_destroy();
>   lttng_clock_unref();
>   return ret;
> }
> @@ -1790,6 +1798,7 @@ error:
> /* No __exit annotation because used by init error path too. */
> void lttng_abi_exit(void)
> {
> + lttng_tp_mempool_destroy();
>   lttng_clock_unref();
>   if (lttng_proc_dentry)
>   remove_proc_entry("lttng", NULL);
> diff --git a/lttng-tp-mempool.c b/lttng-tp-mempool.c
> new file mode 100644
> index 000..d984bd4
> --- /dev/null
> +++ b/lttng-tp-mempool.c
> @@ -0,0 +1,172 @@
> +/*
> + * lttng-tp-mempool.c
> + *
> + * Copyright (C) 2018 Julien Desfossez 
> + *
> + * 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 
> +#include 
> +
> +#include 
> +
> +struct lttng_tp_buf_entry {
> + int cpu; /* To make sure we return the entry to the right pool. */
> + char buf[LTTNG_TP_MEMPOOL_BUF_SIZE];
> + struct list_head list;
> +};
> +
> +/*
> + * No exclusive access strategy for now, this memory pool is currently only
> + * used from a non-preemptible context, and the interrupt tracepoint probes 
> do
> + * not use this facility.
> + */
> +struct per_cpu_buf {
> + struct list_head free_list; /* Free struct lttng_tp_buf_entry. */
> +};
> +
> +static struct per_cpu_buf __percpu *pool; /* Per-cpu buffer. */
> +
> +int lttng_tp_mempool_init(void)
> +{
> + int ret, cpu;
> +
> + /* The pool is only supposed to be allocated once. */
> + if (pool) {
> + WARN_ON_ONCE(1);
> + ret = -1;
> + goto end;
> + }
> +
> + pool = alloc_percpu(struct per_cpu_buf);
> + if (!pool) {
> + ret = -ENOMEM;
> + goto end;
> + }
> +
> + for_each_possible_cpu(cpu) {
> + struct per_cpu_buf *cpu_buf = per_cpu_ptr(pool, cpu);
> +
> + INIT_LIST_HEAD(_buf->free_list);
> + }
>