From: Andrey Konovalov <andreyknvl@google.com>
To: Vincenzo Frascino <vincenzo.frascino@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
Linux ARM <linux-arm-kernel@lists.infradead.org>,
LKML <linux-kernel@vger.kernel.org>,
kasan-dev <kasan-dev@googlegroups.com>,
Will Deacon <will@kernel.org>, Dmitry Vyukov <dvyukov@google.com>,
Andrey Ryabinin <aryabinin@virtuozzo.com>,
Alexander Potapenko <glider@google.com>,
Marco Elver <elver@google.com>,
Evgenii Stepanov <eugenis@google.com>,
Branislav Rankov <Branislav.Rankov@arm.com>
Subject: Re: [PATCH v4 5/5] arm64: mte: Inline mte_assign_mem_tag_range()
Date: Tue, 19 Jan 2021 19:12:40 +0100 [thread overview]
Message-ID: <CAAeHK+y9sw0SENeDXLLBxD8XqD396rXbg1GeBRDEm7PnMzMmHQ@mail.gmail.com> (raw)
In-Reply-To: <1bb4355f-4341-21a7-0a53-a4a27840adee@arm.com>
On Tue, Jan 19, 2021 at 4:45 PM Vincenzo Frascino
<vincenzo.frascino@arm.com> wrote:
>
> Hi Catalin,
>
> On 1/19/21 2:45 PM, Catalin Marinas wrote:
> > On Mon, Jan 18, 2021 at 06:30:33PM +0000, Vincenzo Frascino wrote:
> >> mte_assign_mem_tag_range() is called on production KASAN HW hot
> >> paths. It makes sense to inline it in an attempt to reduce the
> >> overhead.
> >>
> >> Inline mte_assign_mem_tag_range() based on the indications provided at
> >> [1].
> >>
> >> [1] https://lore.kernel.org/r/CAAeHK+wCO+J7D1_T89DG+jJrPLk3X9RsGFKxJGd0ZcUFjQT-9Q@mail.gmail.com/
> >>
> >> Cc: Catalin Marinas <catalin.marinas@arm.com>
> >> Cc: Will Deacon <will@kernel.org>
> >> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
> >> ---
> >> arch/arm64/include/asm/mte.h | 26 +++++++++++++++++++++++++-
> >> arch/arm64/lib/mte.S | 15 ---------------
> >> 2 files changed, 25 insertions(+), 16 deletions(-)
> >>
> >> diff --git a/arch/arm64/include/asm/mte.h b/arch/arm64/include/asm/mte.h
> >> index 237bb2f7309d..1a6fd53f82c3 100644
> >> --- a/arch/arm64/include/asm/mte.h
> >> +++ b/arch/arm64/include/asm/mte.h
> >> @@ -49,7 +49,31 @@ long get_mte_ctrl(struct task_struct *task);
> >> int mte_ptrace_copy_tags(struct task_struct *child, long request,
> >> unsigned long addr, unsigned long data);
> >>
> >> -void mte_assign_mem_tag_range(void *addr, size_t size);
> >> +static inline void mte_assign_mem_tag_range(void *addr, size_t size)
> >> +{
> >> + u64 _addr = (u64)addr;
> >> + u64 _end = _addr + size;
> >> +
> >> + /*
> >> + * This function must be invoked from an MTE enabled context.
> >> + *
> >> + * Note: The address must be non-NULL and MTE_GRANULE_SIZE aligned and
> >> + * size must be non-zero and MTE_GRANULE_SIZE aligned.
> >> + */
> >> + do {
> >> + /*
> >> + * 'asm volatile' is required to prevent the compiler to move
> >> + * the statement outside of the loop.
> >> + */
> >> + asm volatile(__MTE_PREAMBLE "stg %0, [%0]"
> >> + :
> >> + : "r" (_addr)
> >> + : "memory");
> >> +
> >> + _addr += MTE_GRANULE_SIZE;
> >> + } while (_addr != _end);
> >> +}
> >
> > While I'm ok with moving this function to C, I don't think it solves the
> > inlining in the kasan code. The only interface we have to kasan is via
> > mte_{set,get}_mem_tag_range(), so the above function doesn't need to
> > live in a header.
> >
> > If you do want inlining all the way to the kasan code, we should
> > probably move the mte_{set,get}_mem_tag_range() functions to the header
> > as well (and ideally backed by some numbers to show that it matters).
> >
> > Moving it to mte.c also gives us more control on how it's called (we
> > have the WARN_ONs in place in the callers).
> >
>
> Based on the thread [1] this patch contains only an intermediate step to allow
> KASAN to call directly mte_assign_mem_tag_range() in future. At that point I
> think that mte_set_mem_tag_range() can be removed.
>
> If you agree, I would live the things like this to give to Andrey a chance to
> execute on the original plan with a separate series.
I think we should drop this patch from this series as it's unrelated.
I will pick it up into my future optimization series. Then it will be
easier to discuss it in the context. The important part that I needed
is an inlinable C implementation of mte_assign_mem_tag_range(), which
I now have with this patch.
Thanks, Vincenzo!
next prev parent reply other threads:[~2021-01-19 18:29 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-18 18:30 [PATCH v4 0/5] arm64: ARMv8.5-A: MTE: Add async mode support Vincenzo Frascino
2021-01-18 18:30 ` [PATCH v4 1/5] arm64: mte: Add asynchronous " Vincenzo Frascino
2021-01-19 12:57 ` Catalin Marinas
2021-01-19 18:10 ` Andrey Konovalov
2021-01-18 18:30 ` [PATCH v4 2/5] kasan: Add KASAN mode kernel parameter Vincenzo Frascino
2021-01-19 18:10 ` Andrey Konovalov
2021-01-20 14:45 ` Vincenzo Frascino
2021-01-21 16:45 ` Vincenzo Frascino
2021-01-18 18:30 ` [PATCH v4 3/5] kasan: Add report for async mode Vincenzo Frascino
2021-01-19 13:04 ` Catalin Marinas
2021-01-19 14:23 ` Vincenzo Frascino
2021-01-19 14:46 ` Mark Rutland
2021-01-19 15:05 ` Vincenzo Frascino
2021-01-19 18:12 ` Andrey Konovalov
2021-01-20 14:46 ` Vincenzo Frascino
2021-01-18 18:30 ` [PATCH v4 4/5] arm64: mte: Enable async tag check fault Vincenzo Frascino
2021-01-19 14:34 ` Catalin Marinas
2021-01-19 14:45 ` Vincenzo Frascino
2021-01-18 18:30 ` [PATCH v4 5/5] arm64: mte: Inline mte_assign_mem_tag_range() Vincenzo Frascino
2021-01-19 14:45 ` Catalin Marinas
2021-01-19 15:48 ` Vincenzo Frascino
2021-01-19 18:12 ` Andrey Konovalov [this message]
2021-01-19 19:00 ` Catalin Marinas
2021-01-19 19:34 ` Andrey Konovalov
2021-01-19 18:09 ` [PATCH v4 0/5] arm64: ARMv8.5-A: MTE: Add async mode support Andrey Konovalov
2021-01-21 11:35 ` Vincenzo Frascino
2021-01-21 12:25 ` Andrey Konovalov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAAeHK+y9sw0SENeDXLLBxD8XqD396rXbg1GeBRDEm7PnMzMmHQ@mail.gmail.com \
--to=andreyknvl@google.com \
--cc=Branislav.Rankov@arm.com \
--cc=aryabinin@virtuozzo.com \
--cc=catalin.marinas@arm.com \
--cc=dvyukov@google.com \
--cc=elver@google.com \
--cc=eugenis@google.com \
--cc=glider@google.com \
--cc=kasan-dev@googlegroups.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=vincenzo.frascino@arm.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).