Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
MIME-Version: 1.0
References: <cover.1648049113.git.andreyknvl@google.com> <7027b9b6b0cae2921ff65739582ae499bf61470c.1648049113.git.andreyknvl@google.com>
 <CANpmjNPJkFOMn1pL-=gx+x_YHgg72QH5iqe561+Geiy3JoOg1w@mail.gmail.com>
In-Reply-To: <CANpmjNPJkFOMn1pL-=gx+x_YHgg72QH5iqe561+Geiy3JoOg1w@mail.gmail.com>
From:   Andrey Konovalov <andreyknvl@gmail.com>
Date:   Tue, 29 Mar 2022 20:36:46 +0200
Message-ID: <CA+fCnZfOGRh67SUNxQ2cyZLK8JV56GV_sa8AnNeURcgHif5Yzg@mail.gmail.com>
Subject: Re: [PATCH v2 4/4] kasan: use stack_trace_save_shadow
To:     Marco Elver <elver@google.com>
Cc:     andrey.konovalov@linux.dev,
        Alexander Potapenko <glider@google.com>,
        Catalin Marinas <catalin.marinas@arm.com>,
        Will Deacon <will@kernel.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Dmitry Vyukov <dvyukov@google.com>,
        Andrey Ryabinin <ryabinin.a.a@gmail.com>,
        kasan-dev <kasan-dev@googlegroups.com>,
        Mark Rutland <mark.rutland@arm.com>,
        Vincenzo Frascino <vincenzo.frascino@arm.com>,
        Sami Tolvanen <samitolvanen@google.com>,
        Peter Collingbourne <pcc@google.com>,
        Evgenii Stepanov <eugenis@google.com>,
        Florian Mayer <fmayer@google.com>,
        Linux Memory Management List <linux-mm@kvack.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Andrey Konovalov <andreyknvl@google.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, Mar 28, 2022 at 2:49 PM Marco Elver <elver@google.com> wrote:
>
> > diff --git a/mm/kasan/common.c b/mm/kasan/common.c
> > index d9079ec11f31..8d9d35c6562b 100644
> > --- a/mm/kasan/common.c
> > +++ b/mm/kasan/common.c
> > @@ -33,10 +33,13 @@
> >  depot_stack_handle_t kasan_save_stack(gfp_t flags, bool can_alloc)
> >  {
> >         unsigned long entries[KASAN_STACK_DEPTH];
> > -       unsigned int nr_entries;
> > +       unsigned int size;
>
> Why did this variable name change?

So the lines below fit within one line. It won't be needed with the
other change you suggested.

> > -       nr_entries = stack_trace_save(entries, ARRAY_SIZE(entries), 0);
> > -       return __stack_depot_save(entries, nr_entries, flags, can_alloc);
> > +       if (IS_ENABLED(CONFIG_HAVE_SHADOW_STACKTRACE))
>
> Would it be more reliable to check the return-code? I.e. do:
>
>   int size;
>
>   size = stack_trace_save_shadow(...)
>   if (size < 0)
>     size = stack_trace_save(...);

Sounds good, will do in v3.

Thanks!