Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190410102754.387743324@linutronix.de> <20190410103645.315084160@linutronix.de>
In-Reply-To: <20190410103645.315084160@linutronix.de>
From:   Alexander Potapenko <glider@google.com>
Date:   Wed, 10 Apr 2019 15:39:52 +0200
Message-ID: <CAG_fn=U5vWCCRbCTABCa5GqBrkeT0gQyyUnUgSQ5wJhSuNw94A@mail.gmail.com>
Subject: Re: [RFC patch 19/41] lib/stackdepot: Provide functions which operate
 on plain storage arrays
To:     Thomas Gleixner <tglx@linutronix.de>
Cc:     LKML <linux-kernel@vger.kernel.org>,
        Josh Poimboeuf <jpoimboe@redhat.com>, x86@kernel.org,
        Andy Lutomirski <luto@kernel.org>,
        Steven Rostedt <rostedt@goodmis.org>,
        Alexander Potapenko <glider@google.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Apr 10, 2019 at 1:05 PM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> The struct stack_trace indirection in the stack depot functions is a trul=
y
> pointless excercise which requires horrible code at the callsites.
>
> Provide interfaces based on plain storage arrays.
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Alexander Potapenko <glider@google.com>
> ---
>  include/linux/stackdepot.h |    4 ++
>  lib/stackdepot.c           |   66 ++++++++++++++++++++++++++++++++------=
-------
>  2 files changed, 51 insertions(+), 19 deletions(-)
>
> --- a/include/linux/stackdepot.h
> +++ b/include/linux/stackdepot.h
> @@ -26,7 +26,11 @@ typedef u32 depot_stack_handle_t;
>  struct stack_trace;
>
>  depot_stack_handle_t depot_save_stack(struct stack_trace *trace, gfp_t f=
lags);
> +depot_stack_handle_t stack_depot_save(unsigned long *entries,
> +                                     unsigned int nr_entries, gfp_t gfp_=
flags);
>
>  void depot_fetch_stack(depot_stack_handle_t handle, struct stack_trace *=
trace);
> +unsigned int stack_depot_fetch(depot_stack_handle_t handle,
> +                              unsigned long **entries);
>
>  #endif
> --- a/lib/stackdepot.c
> +++ b/lib/stackdepot.c
> @@ -194,40 +194,56 @@ static inline struct stack_record *find_
>         return NULL;
>  }
>
> -void depot_fetch_stack(depot_stack_handle_t handle, struct stack_trace *=
trace)
> +/**
> + * stack_depot_fetch - Fetch stack entries from a depot
> + *
> + * @entries:           Pointer to store the entries address
> + */
> +unsigned int stack_depot_fetch(depot_stack_handle_t handle,
> +                              unsigned long **entries)
>  {
>         union handle_parts parts =3D { .handle =3D handle };
>         void *slab =3D stack_slabs[parts.slabindex];
>         size_t offset =3D parts.offset << STACK_ALLOC_ALIGN;
>         struct stack_record *stack =3D slab + offset;
>
> -       trace->nr_entries =3D trace->max_entries =3D stack->size;
> -       trace->entries =3D stack->entries;
> -       trace->skip =3D 0;
> +       *entries =3D stack->entries;
> +       return stack->size;
> +}
> +EXPORT_SYMBOL_GPL(stack_depot_fetch);
> +
> +void depot_fetch_stack(depot_stack_handle_t handle, struct stack_trace *=
trace)
> +{
> +       unsigned int nent =3D stack_depot_fetch(handle, &trace->entries);
> +
> +       trace->max_entries =3D trace->nr_entries =3D nent;
>  }
>  EXPORT_SYMBOL_GPL(depot_fetch_stack);
>
>  /**
> - * depot_save_stack - save stack in a stack depot.
> - * @trace - the stacktrace to save.
> - * @alloc_flags - flags for allocating additional memory if required.
> + * stack_depot_save - Save a stack trace from an array
>   *
> - * Returns the handle of the stack struct stored in depot.
> + * @entries:           Pointer to storage array
> + * @nr_entries:                Size of the storage array
> + * @alloc_flags:       Allocation gfp flags
> + *
> + * Returns the handle of the stack struct stored in depot
>   */
> -depot_stack_handle_t depot_save_stack(struct stack_trace *trace,
> -                                   gfp_t alloc_flags)
> +depot_stack_handle_t stack_depot_save(unsigned long *entries,
> +                                     unsigned int nr_entries,
> +                                     gfp_t alloc_flags)
>  {
> -       u32 hash;
> -       depot_stack_handle_t retval =3D 0;
>         struct stack_record *found =3D NULL, **bucket;
> -       unsigned long flags;
> +       depot_stack_handle_t retval =3D 0;
>         struct page *page =3D NULL;
>         void *prealloc =3D NULL;
> +       unsigned long flags;
> +       u32 hash;
>
> -       if (unlikely(trace->nr_entries =3D=3D 0))
> +       if (unlikely(nr_entries =3D=3D 0))
>                 goto fast_exit;
>
> -       hash =3D hash_stack(trace->entries, trace->nr_entries);
> +       hash =3D hash_stack(entries, nr_entries);
>         bucket =3D &stack_table[hash & STACK_HASH_MASK];
>
>         /*
> @@ -235,8 +251,8 @@ depot_stack_handle_t depot_save_stack(st
>          * The smp_load_acquire() here pairs with smp_store_release() to
>          * |bucket| below.
>          */
> -       found =3D find_stack(smp_load_acquire(bucket), trace->entries,
> -                          trace->nr_entries, hash);
> +       found =3D find_stack(smp_load_acquire(bucket), entries,
> +                          nr_entries, hash);
>         if (found)
>                 goto exit;
>
> @@ -264,10 +280,10 @@ depot_stack_handle_t depot_save_stack(st
>
>         spin_lock_irqsave(&depot_lock, flags);
>
> -       found =3D find_stack(*bucket, trace->entries, trace->nr_entries, =
hash);
> +       found =3D find_stack(*bucket, entries, nr_entries, hash);
>         if (!found) {
>                 struct stack_record *new =3D
> -                       depot_alloc_stack(trace->entries, trace->nr_entri=
es,
> +                       depot_alloc_stack(entries, nr_entries,
>                                           hash, &prealloc, alloc_flags);
>                 if (new) {
>                         new->next =3D *bucket;
> @@ -297,4 +313,16 @@ depot_stack_handle_t depot_save_stack(st
>  fast_exit:
>         return retval;
>  }
> +EXPORT_SYMBOL_GPL(stack_depot_save);
> +
> +/**
> + * depot_save_stack - save stack in a stack depot.
> + * @trace - the stacktrace to save.
> + * @alloc_flags - flags for allocating additional memory if required.
> + */
> +depot_stack_handle_t depot_save_stack(struct stack_trace *trace,
> +                                     gfp_t alloc_flags)
> +{
> +       return stack_depot_save(trace->entries, trace->nr_entries, alloc_=
flags);
> +}
>  EXPORT_SYMBOL_GPL(depot_save_stack);
>
>


--=20
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Stra=C3=9Fe, 33
80636 M=C3=BCnchen

Gesch=C3=A4ftsf=C3=BChrer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg