Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <cover.1605046662.git.andreyknvl@google.com> <0a9b63bff116734ab63d99ebd09c244332d71958.1605046662.git.andreyknvl@google.com>
 <20201111174902.GK517454@elver.google.com> <CAAeHK+wvvkYko=tM=NHODkKas13h5Jvsswvg05jhv9LqE0jSjQ@mail.gmail.com>
 <CANpmjNOboPh97HdMGAESSEYdeyd9+9MVy6E3QsvVAYuWVReRew@mail.gmail.com> <CAAeHK+xhjUQAtJThUHcaGmd3muBZHiJPfTqj59CMxo44hbDniw@mail.gmail.com>
In-Reply-To: <CAAeHK+xhjUQAtJThUHcaGmd3muBZHiJPfTqj59CMxo44hbDniw@mail.gmail.com>
From:   Marco Elver <elver@google.com>
Date:   Thu, 12 Nov 2020 23:20:27 +0100
Message-ID: <CANpmjNPxqOi8QCJ4VY6vfVkktEWO1=S5hcOBvTQcWhhL0L9B-w@mail.gmail.com>
Subject: Re: [PATCH v2 10/20] kasan: inline and rename kasan_unpoison_memory
To:     Andrey Konovalov <andreyknvl@google.com>
Cc:     Dmitry Vyukov <dvyukov@google.com>,
        Alexander Potapenko <glider@google.com>,
        Catalin Marinas <catalin.marinas@arm.com>,
        Will Deacon <will.deacon@arm.com>,
        Vincenzo Frascino <vincenzo.frascino@arm.com>,
        Evgenii Stepanov <eugenis@google.com>,
        Andrey Ryabinin <aryabinin@virtuozzo.com>,
        Branislav Rankov <Branislav.Rankov@arm.com>,
        Kevin Brodsky <kevin.brodsky@arm.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        kasan-dev <kasan-dev@googlegroups.com>,
        Linux ARM <linux-arm-kernel@lists.infradead.org>,
        Linux Memory Management List <linux-mm@kvack.org>,
        LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, 12 Nov 2020 at 21:54, Andrey Konovalov <andreyknvl@google.com> wrote:
>
> On Thu, Nov 12, 2020 at 8:52 PM Marco Elver <elver@google.com> wrote:
> >
> > On Thu, 12 Nov 2020 at 20:45, Andrey Konovalov <andreyknvl@google.com> wrote:
> > >
> > > On Wed, Nov 11, 2020 at 6:49 PM Marco Elver <elver@google.com> wrote:
> > > >
> > > > On Tue, Nov 10, 2020 at 11:20PM +0100, Andrey Konovalov wrote:
> > > > > Currently kasan_unpoison_memory() is used as both an external annotation
> > > > > and as an internal memory poisoning helper. Rename external annotation to
> > > > > kasan_unpoison_data() and inline the internal helper for hardware
> > > > > tag-based mode to avoid undeeded function calls.
> > > >
> > > > I don't understand why this needs to be renamed again. The users of
> > > > kasan_unpoison_memory() outweigh those of kasan_unpoison_slab(), of
> > > > which there seems to be only 1!
> > >
> > > The idea is to make kasan_(un)poison_memory() functions inlinable for
> > > internal use. It doesn't have anything to do with the number of times
> > > they are used.
> > >
> > > Perhaps we can drop the kasan_ prefix for the internal implementations
> > > though, and keep using kasan_unpoison_memory() externally.
> >
> > Whatever avoids changing the external interface, because it seems
> > really pointless. I can see why it's done, but it's a side-effect of
> > the various wrappers being added.
>
> It looks like unposion_memory() is already taken. Any suggestions for
> internal KASAN poisoning function names?

I still don't like that one of these functions just forwards to the
other, but we could use it to also give the external function a more
descriptive name.

I propose 2 options:

1. Name the internal helpers *poison_range().
2. Name the external function kasan_unpoison_range() instead of
kasan_unpoison_data().

Anything "memory" (or "data") in the allocators might not be too
helpful w.r.t. descriptive function names (i.e. stripping "memory"
from function names won't lessen descriptiveness given our context).
Perhaps kasan_poison_range() for the external function might in fact
improve the external interface (without looking at its arguments).

If we need to keep the internal helpers, I'd probably go so far as to
suggest renaming them to simply kasan_{poison,unpoison}(), and then
building the external kasan_{poison,unpoison}_foo() on top. But maybe
that's too much for now if it doesn't fit here. :-)

Thanks,
-- Marco