All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joonsoo Kim <js1304@gmail.com>
To: Dmitry Vyukov <dvyukov@google.com>
Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Alexander Potapenko <glider@google.com>,
	kasan-dev <kasan-dev@googlegroups.com>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, "H . Peter Anvin" <hpa@zytor.com>,
	kernel-team@lge.com
Subject: Re: [PATCH v1 00/11] mm/kasan: support per-page shadow memory to reduce memory consumption
Date: Thu, 8 Jun 2017 11:40:16 +0900	[thread overview]
Message-ID: <20170608024014.GB27998@js1304-desktop> (raw)
In-Reply-To: <CACT4Y+at_NESQ8qq4zouArnu5yySQHxC2oW+RuXzqX8hyspZ_g@mail.gmail.com>

On Thu, Jun 01, 2017 at 08:06:02PM +0200, Dmitry Vyukov wrote:
> On Tue, May 30, 2017 at 4:16 PM, Andrey Ryabinin
> <aryabinin@virtuozzo.com> wrote:
> > On 05/29/2017 06:29 PM, Dmitry Vyukov wrote:
> >> Joonsoo,
> >>
> >> I guess mine (and Andrey's) main concern is the amount of additional
> >> complexity (I am still struggling to understand how it all works) and
> >> more arch-dependent code in exchange for moderate memory win.
> >>
> >> Joonsoo, Andrey,
> >>
> >> I have an alternative proposal. It should be conceptually simpler and
> >> also less arch-dependent. But I don't know if I miss something
> >> important that will render it non working.
> >> Namely, we add a pointer to shadow to the page struct. Then, create a
> >> slab allocator for 512B shadow blocks. Then, attach/detach these
> >> shadow blocks to page structs as necessary. It should lead to even
> >> smaller memory consumption because we won't need a whole shadow page
> >> when only 1 out of 8 corresponding kernel pages are used (we will need
> >> just a single 512B block). I guess with some fragmentation we need
> >> lots of excessive shadow with the current proposed patch.
> >> This does not depend on TLB in any way and does not require hooking
> >> into buddy allocator.
> >> The main downside is that we will need to be careful to not assume
> >> that shadow is continuous. In particular this means that this mode
> >> will work only with outline instrumentation and will need some ifdefs.
> >> Also it will be slower due to the additional indirection when
> >> accessing shadow, but that's meant as "small but slow" mode as far as
> >> I understand.
> >
> > It seems that you are forgetting about stack instrumentation.
> > You'll have to disable it completely, at least with current implementation of it in gcc.
> >
> >> But the main win as I see it is that that's basically complete support
> >> for 32-bit arches. People do ask about arm32 support:
> >> https://groups.google.com/d/msg/kasan-dev/Sk6BsSPMRRc/Gqh4oD_wAAAJ
> >> https://groups.google.com/d/msg/kasan-dev/B22vOFp-QWg/EVJPbrsgAgAJ
> >> and probably mips32 is relevant as well.
> >
> > I don't see how above is relevant for 32-bit arches. Current design
> > is perfectly fine for 32-bit arches. I did some POC arm32 port couple years
> > ago - https://github.com/aryabinin/linux/commits/kasan/arm_v0_1
> > It has some ugly hacks and non-critical bugs. AFAIR it also super-slow because I (mistakenly)
> > made shadow memory uncached. But otherwise it works.
> >
> >> Such mode does not require a huge continuous address space range, has
> >> minimal memory consumption and requires minimal arch-dependent code.
> >> Works only with outline instrumentation, but I think that's a
> >> reasonable compromise.
> >>
> >> What do you think?
> >
> > I don't understand why we trying to invent some hacky/complex schemes when we already have
> > a simple one - scaling shadow to 1/32. It's easy to implement and should be more performant comparing
> > to suggested schemes.
> 
> 
> If 32-bits work with the current approach, then I would also prefer to
> keep things simpler.
> FWIW clang supports settings shadow scale via a command line flag
> (-asan-mapping-scale).

Hello,

To confirm the final consensus, I did a quick comparison of scaling
approach and mine. Note that scaling approach can be co-exist with
mine. And, there is an assumption that we can disable quarantine and
other optional feature of KASAN.

Scaling vs Mine

Memory usage: 1/32 of total memory. vs can be far less than 1/32.
Slab object layout: should be changed. vs none.
Usability: hard. vs simple. (Updating compiler is not required)
Implementation complexity: simple. vs complex.
Porting to other ARCH: simple. vs hard (But, not mandatory)

So, do both you disagree to merge my per-page shadow? If so, I will
not submit v2. Please let me know your decision.

Thanks.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

WARNING: multiple messages have this Message-ID (diff)
From: Joonsoo Kim <js1304@gmail.com>
To: Dmitry Vyukov <dvyukov@google.com>
Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Alexander Potapenko <glider@google.com>,
	kasan-dev <kasan-dev@googlegroups.com>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, "H . Peter Anvin" <hpa@zytor.com>,
	kernel-team@lge.com
Subject: Re: [PATCH v1 00/11] mm/kasan: support per-page shadow memory to reduce memory consumption
Date: Thu, 8 Jun 2017 11:40:16 +0900	[thread overview]
Message-ID: <20170608024014.GB27998@js1304-desktop> (raw)
In-Reply-To: <CACT4Y+at_NESQ8qq4zouArnu5yySQHxC2oW+RuXzqX8hyspZ_g@mail.gmail.com>

On Thu, Jun 01, 2017 at 08:06:02PM +0200, Dmitry Vyukov wrote:
> On Tue, May 30, 2017 at 4:16 PM, Andrey Ryabinin
> <aryabinin@virtuozzo.com> wrote:
> > On 05/29/2017 06:29 PM, Dmitry Vyukov wrote:
> >> Joonsoo,
> >>
> >> I guess mine (and Andrey's) main concern is the amount of additional
> >> complexity (I am still struggling to understand how it all works) and
> >> more arch-dependent code in exchange for moderate memory win.
> >>
> >> Joonsoo, Andrey,
> >>
> >> I have an alternative proposal. It should be conceptually simpler and
> >> also less arch-dependent. But I don't know if I miss something
> >> important that will render it non working.
> >> Namely, we add a pointer to shadow to the page struct. Then, create a
> >> slab allocator for 512B shadow blocks. Then, attach/detach these
> >> shadow blocks to page structs as necessary. It should lead to even
> >> smaller memory consumption because we won't need a whole shadow page
> >> when only 1 out of 8 corresponding kernel pages are used (we will need
> >> just a single 512B block). I guess with some fragmentation we need
> >> lots of excessive shadow with the current proposed patch.
> >> This does not depend on TLB in any way and does not require hooking
> >> into buddy allocator.
> >> The main downside is that we will need to be careful to not assume
> >> that shadow is continuous. In particular this means that this mode
> >> will work only with outline instrumentation and will need some ifdefs.
> >> Also it will be slower due to the additional indirection when
> >> accessing shadow, but that's meant as "small but slow" mode as far as
> >> I understand.
> >
> > It seems that you are forgetting about stack instrumentation.
> > You'll have to disable it completely, at least with current implementation of it in gcc.
> >
> >> But the main win as I see it is that that's basically complete support
> >> for 32-bit arches. People do ask about arm32 support:
> >> https://groups.google.com/d/msg/kasan-dev/Sk6BsSPMRRc/Gqh4oD_wAAAJ
> >> https://groups.google.com/d/msg/kasan-dev/B22vOFp-QWg/EVJPbrsgAgAJ
> >> and probably mips32 is relevant as well.
> >
> > I don't see how above is relevant for 32-bit arches. Current design
> > is perfectly fine for 32-bit arches. I did some POC arm32 port couple years
> > ago - https://github.com/aryabinin/linux/commits/kasan/arm_v0_1
> > It has some ugly hacks and non-critical bugs. AFAIR it also super-slow because I (mistakenly)
> > made shadow memory uncached. But otherwise it works.
> >
> >> Such mode does not require a huge continuous address space range, has
> >> minimal memory consumption and requires minimal arch-dependent code.
> >> Works only with outline instrumentation, but I think that's a
> >> reasonable compromise.
> >>
> >> What do you think?
> >
> > I don't understand why we trying to invent some hacky/complex schemes when we already have
> > a simple one - scaling shadow to 1/32. It's easy to implement and should be more performant comparing
> > to suggested schemes.
> 
> 
> If 32-bits work with the current approach, then I would also prefer to
> keep things simpler.
> FWIW clang supports settings shadow scale via a command line flag
> (-asan-mapping-scale).

Hello,

To confirm the final consensus, I did a quick comparison of scaling
approach and mine. Note that scaling approach can be co-exist with
mine. And, there is an assumption that we can disable quarantine and
other optional feature of KASAN.

Scaling vs Mine

Memory usage: 1/32 of total memory. vs can be far less than 1/32.
Slab object layout: should be changed. vs none.
Usability: hard. vs simple. (Updating compiler is not required)
Implementation complexity: simple. vs complex.
Porting to other ARCH: simple. vs hard (But, not mandatory)

So, do both you disagree to merge my per-page shadow? If so, I will
not submit v2. Please let me know your decision.

Thanks.

  reply	other threads:[~2017-06-08  2:40 UTC|newest]

Thread overview: 104+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-16  1:16 [PATCH v1 00/11] mm/kasan: support per-page shadow memory to reduce memory consumption js1304
2017-05-16  1:16 ` js1304
2017-05-16  1:16 ` [PATCH v1 01/11] mm/kasan: rename XXX_is_zero to XXX_is_nonzero js1304
2017-05-16  1:16 ` [PATCH v1 02/11] mm/kasan: don't fetch the next shadow value speculartively js1304
2017-05-16  1:16   ` js1304
2017-05-16  1:16 ` [PATCH v1 03/11] mm/kasan: handle unaligned end address in zero_pte_populate js1304
2017-05-16  1:16   ` js1304
2017-05-16  1:16 ` [PATCH v1 04/11] mm/kasan: extend kasan_populate_zero_shadow() js1304
2017-05-16  1:16   ` js1304
2017-05-16  1:16 ` [PATCH v1 05/11] mm/kasan: introduce per-page shadow memory infrastructure js1304
2017-05-16  1:16   ` js1304
2017-05-16  1:16 ` [PATCH v1 06/11] mm/kasan: mark/unmark the target range that is for original shadow memory js1304
2017-05-16  1:16   ` js1304
2017-05-16  1:16 ` [PATCH v1 07/11] x86/kasan: use per-page " js1304
2017-05-16  1:16   ` js1304
2017-05-16  1:16 ` [PATCH v1 08/11] mm/kasan: support on-demand shadow allocation/mapping js1304
2017-05-16  1:16   ` js1304
2017-05-16  1:16 ` [PATCH v1 09/11] x86/kasan: support on-demand shadow mapping js1304
2017-05-16  1:16   ` js1304
2017-05-16  1:16 ` [PATCH v1 10/11] mm/kasan: support dynamic shadow memory free js1304
2017-05-16  1:16   ` js1304
2017-05-16  1:16 ` [PATCH v1 11/11] mm/kasan: change the order of shadow memory check js1304
2017-05-16  1:16   ` js1304
2017-05-16  1:28 ` [PATCH(RE-RESEND) v1 01/11] mm/kasan: rename _is_zero to _is_nonzero Joonsoo Kim
2017-05-16  1:28   ` Joonsoo Kim
2017-05-16  4:34 ` [PATCH v1 00/11] mm/kasan: support per-page shadow memory to reduce memory consumption Dmitry Vyukov
2017-05-16  4:34   ` Dmitry Vyukov
2017-05-16  4:47   ` Dmitry Vyukov
2017-05-16  4:47     ` Dmitry Vyukov
2017-05-16  6:23   ` Joonsoo Kim
2017-05-16  6:23     ` Joonsoo Kim
2017-05-16 20:49     ` Dmitry Vyukov
2017-05-16 20:49       ` Dmitry Vyukov
2017-05-17  7:23       ` Joonsoo Kim
2017-05-17  7:23         ` Joonsoo Kim
2017-05-17  7:25         ` Joonsoo Kim
2017-05-17  7:25           ` Joonsoo Kim
2017-05-24  6:57       ` Dmitry Vyukov
2017-05-24  6:57         ` Dmitry Vyukov
2017-05-24  7:45         ` Joonsoo Kim
2017-05-24  7:45           ` Joonsoo Kim
2017-05-24 17:19           ` Dmitry Vyukov
2017-05-24 17:19             ` Dmitry Vyukov
2017-05-25  0:41             ` Joonsoo Kim
2017-05-25  0:41               ` Joonsoo Kim
2017-05-29 15:07               ` Dmitry Vyukov
2017-05-29 15:07                 ` Dmitry Vyukov
2017-05-29 15:12                 ` Dmitry Vyukov
2017-05-29 15:12                   ` Dmitry Vyukov
2017-05-29 15:29                   ` Dmitry Vyukov
2017-05-29 15:29                     ` Dmitry Vyukov
2017-05-30  7:58                     ` Vladimir Murzin
2017-05-30  7:58                       ` Vladimir Murzin
2017-05-30  8:15                       ` Dmitry Vyukov
2017-05-30  8:15                         ` Dmitry Vyukov
2017-05-30  8:31                         ` Vladimir Murzin
2017-05-30  8:31                           ` Vladimir Murzin
2017-05-30  8:40                           ` Vladimir Murzin
2017-05-30  8:40                             ` Vladimir Murzin
2017-05-30  8:49                             ` Dmitry Vyukov
2017-05-30  8:49                               ` Dmitry Vyukov
2017-05-30  9:08                               ` Vladimir Murzin
2017-05-30  9:08                                 ` Vladimir Murzin
2017-05-30  9:26                                 ` Dmitry Vyukov
2017-05-30  9:26                                   ` Dmitry Vyukov
2017-05-30  9:39                                   ` Vladimir Murzin
2017-05-30  9:39                                     ` Vladimir Murzin
2017-05-30  9:45                                     ` Dmitry Vyukov
2017-05-30  9:45                                       ` Dmitry Vyukov
2017-05-30  9:54                                       ` Vladimir Murzin
2017-05-30  9:54                                         ` Vladimir Murzin
2017-05-30 14:16                     ` Andrey Ryabinin
2017-05-30 14:16                       ` Andrey Ryabinin
2017-05-31  5:50                       ` Joonsoo Kim
2017-05-31  5:50                         ` Joonsoo Kim
2017-05-31 16:31                         ` Andrey Ryabinin
2017-05-31 16:31                           ` Andrey Ryabinin
2017-06-08  2:43                           ` Joonsoo Kim
2017-06-08  2:43                             ` Joonsoo Kim
2017-06-01 15:16                       ` 王靖天
2017-06-01 18:06                       ` Dmitry Vyukov
2017-06-01 18:06                         ` Dmitry Vyukov
2017-06-08  2:40                         ` Joonsoo Kim [this message]
2017-06-08  2:40                           ` Joonsoo Kim
2017-06-13 16:49                           ` Andrey Ryabinin
2017-06-13 16:49                             ` Andrey Ryabinin
2017-06-14  0:12                             ` Joonsoo Kim
2017-06-14  0:12                               ` Joonsoo Kim
2017-05-17 12:17 ` Andrey Ryabinin
2017-05-17 12:17   ` Andrey Ryabinin
2017-05-19  1:53   ` Joonsoo Kim
2017-05-19  1:53     ` Joonsoo Kim
2017-05-22  6:02     ` Dmitry Vyukov
2017-05-22  6:02       ` Dmitry Vyukov
2017-05-24  6:04       ` Joonsoo Kim
2017-05-24  6:04         ` Joonsoo Kim
2017-05-24 16:31         ` Dmitry Vyukov
2017-05-24 16:31           ` Dmitry Vyukov
2017-05-25  0:46           ` Joonsoo Kim
2017-05-25  0:46             ` Joonsoo Kim
2017-05-22 14:00     ` Andrey Ryabinin
2017-05-22 14:00       ` Andrey Ryabinin
2017-05-24  6:18       ` Joonsoo Kim
2017-05-24  6:18         ` Joonsoo Kim

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=20170608024014.GB27998@js1304-desktop \
    --to=js1304@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=aryabinin@virtuozzo.com \
    --cc=dvyukov@google.com \
    --cc=glider@google.com \
    --cc=hpa@zytor.com \
    --cc=kasan-dev@googlegroups.com \
    --cc=kernel-team@lge.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.