Both patches look good !

For both:

Acked-by: Mathieu Desnoyers <mathieu.desnoy...@efficios.com>

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 <jdesfos...@efficios.com>
> ---
> 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 <lttng-abi-old.h>
> #include <lttng-events.h>
> #include <lttng-tracer.h>
> +#include <lttng-tp-mempool.h>
> #include <lib/ringbuffer/frontend_types.h>
> 
> /*
> @@ -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,
>                                       &lttng_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 0000000..d984bd4
> --- /dev/null
> +++ b/lttng-tp-mempool.c
> @@ -0,0 +1,172 @@
> +/*
> + * lttng-tp-mempool.c
> + *
> + * Copyright (C) 2018 Julien Desfossez <jdesfos...@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/slab.h>
> +#include <linux/percpu.h>
> +
> +#include <lttng-tp-mempool.h>
> +
> +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(&cpu_buf->free_list);
> +     }
> +
> +     for_each_possible_cpu(cpu) {
> +             int i;
> +             struct per_cpu_buf *cpu_buf = per_cpu_ptr(pool, cpu);
> +
> +             for (i = 0; i < LTTNG_TP_MEMPOOL_NR_BUF_PER_CPU; i++) {
> +                     struct lttng_tp_buf_entry *entry;
> +
> +                     entry = kzalloc(sizeof(struct lttng_tp_buf_entry),
> +                                     GFP_KERNEL);
> +                     if (!entry) {
> +                             ret = -ENOMEM;
> +                             goto error_free_pool;
> +                     }
> +                     entry->cpu = cpu;
> +                     list_add_tail(&entry->list, &cpu_buf->free_list);
> +             }
> +     }
> +
> +     ret = 0;
> +     goto end;
> +
> +error_free_pool:
> +     lttng_tp_mempool_destroy();
> +end:
> +     return ret;
> +}
> +
> +void lttng_tp_mempool_destroy(void)
> +{
> +     int cpu;
> +
> +     if (!pool) {
> +             return;
> +     }
> +
> +     for_each_possible_cpu(cpu) {
> +             struct per_cpu_buf *cpu_buf = per_cpu_ptr(pool, cpu);
> +             struct lttng_tp_buf_entry *entry, *tmp;
> +             int i = 0;
> +
> +             list_for_each_entry_safe(entry, tmp, &cpu_buf->free_list, list) 
> {
> +                     list_del(&entry->list);
> +                     kfree(entry);
> +                     i++;
> +             }
> +             if (i < LTTNG_TP_MEMPOOL_NR_BUF_PER_CPU) {
> +                     printk(KERN_WARNING "Leak detected in tp-mempool\n");
> +             }
> +     }
> +     free_percpu(pool);
> +     pool = NULL;
> +}
> +
> +void *lttng_tp_mempool_alloc(size_t size)
> +{
> +     void *ret;
> +     struct lttng_tp_buf_entry *entry;
> +     struct per_cpu_buf *cpu_buf;
> +     int cpu = smp_processor_id();
> +
> +     if (size > LTTNG_TP_MEMPOOL_BUF_SIZE) {
> +             ret = NULL;
> +             goto end;
> +     }
> +
> +     cpu_buf = per_cpu_ptr(pool, cpu);
> +     if (list_empty(&cpu_buf->free_list)) {
> +             ret = NULL;
> +             goto end;
> +     }
> +
> +     entry = list_first_entry(&cpu_buf->free_list, struct lttng_tp_buf_entry,
> list);
> +     /* Remove the entry from the free list. */
> +     list_del(&entry->list);
> +
> +     memset(entry->buf, 0, LTTNG_TP_MEMPOOL_BUF_SIZE);
> +
> +     ret = (void *) entry->buf;
> +
> +end:
> +     return ret;
> +}
> +
> +void lttng_tp_mempool_free(void *ptr)
> +{
> +     struct lttng_tp_buf_entry *entry;
> +     struct per_cpu_buf *cpu_buf;
> +
> +     if (!ptr) {
> +             goto end;
> +     }
> +
> +     entry = container_of(ptr, struct lttng_tp_buf_entry, buf);
> +     if (!entry) {
> +             goto end;
> +     }
> +
> +     cpu_buf = per_cpu_ptr(pool, entry->cpu);
> +     if (!cpu_buf) {
> +             goto end;
> +     }
> +     /* Add it to the free list. */
> +     list_add_tail(&entry->list, &cpu_buf->free_list);
> +
> +end:
> +     return;
> +}
> diff --git a/lttng-tp-mempool.h b/lttng-tp-mempool.h
> new file mode 100644
> index 0000000..7ce4112
> --- /dev/null
> +++ b/lttng-tp-mempool.h
> @@ -0,0 +1,63 @@
> +#ifndef LTTNG_TP_MEMPOOL_H
> +#define LTTNG_TP_MEMPOOL_H
> +
> +/*
> + * lttng-tp-mempool.h
> + *
> + * Copyright (C) 2018 Julien Desfossez <jdesfos...@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/percpu.h>
> +
> +#define LTTNG_TP_MEMPOOL_NR_BUF_PER_CPU 4
> +#define LTTNG_TP_MEMPOOL_BUF_SIZE 4096
> +
> +/*
> + * Initialize the pool, only performed once. The pool is a set of
> + * LTTNG_TP_MEMPOOL_NR_BUF_PER_CPU buffers of size LTTNG_TP_MEMPOOL_BUF_SIZE
> + * per-cpu.
> + *
> + * Returns 0 on success, a negative value on error.
> + */
> +int lttng_tp_mempool_init(void);
> +
> +/*
> + * Destroy the pool and free all the memory allocated.
> + */
> +void lttng_tp_mempool_destroy(void);
> +
> +/*
> + * Ask for a buffer on the current cpu.
> + *
> + * The pool is per-cpu, but there is no exclusive access guarantee on the
> + * per-cpu free-list, the caller needs to ensure it cannot get preempted or
> + * interrupted while performing the allocation.
> + *
> + * The maximum size that can be allocated is LTTNG_TP_MEMPOOL_BUF_SIZE, and 
> the
> + * maximum number of buffers allocated simultaneously on the same CPU is
> + * LTTNG_TP_MEMPOOL_NR_BUF_PER_CPU.
> + *
> + * Return a pointer to a buffer on success, NULL on error.
> + */
> +void *lttng_tp_mempool_alloc(size_t size);
> +
> +/*
> + * Release the memory reserved. Same concurrency limitations as the 
> allocation.
> + */
> +void lttng_tp_mempool_free(void *ptr);
> +
> +#endif /* LTTNG_TP_MEMPOOL_H */
> --
> 2.7.4

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Reply via email to