From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Alexander Potapenko <glider@google.com>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>,
Andrew Morton <akpm@linux-foundation.org>,
Andrey Konovalov <andreyknvl@google.com>,
Andy Lutomirski <luto@kernel.org>, Arnd Bergmann <arnd@arndb.de>,
Borislav Petkov <bp@alien8.de>, Christoph Hellwig <hch@lst.de>,
Christoph Lameter <cl@linux.com>,
David Rientjes <rientjes@google.com>,
Dmitry Vyukov <dvyukov@google.com>,
Eric Dumazet <edumazet@google.com>,
Herbert Xu <herbert@gondor.apana.org.au>,
Ilya Leoshkevich <iii@linux.ibm.com>,
Ingo Molnar <mingo@redhat.com>, Jens Axboe <axboe@kernel.dk>,
Joonsoo Kim <iamjoonsoo.kim@lge.com>,
Kees Cook <keescook@chromium.org>, Marco Elver <elver@google.com>,
Matthew Wilcox <willy@infradead.org>,
"Michael S. Tsirkin" <mst@redhat.com>,
Pekka Enberg <penberg@kernel.org>,
Peter Zijlstra <peterz@infradead.org>,
Petr Mladek <pmladek@suse.com>,
Steven Rostedt <rostedt@goodmis.org>,
Thomas Gleixner <tglx@linutronix.de>,
Vasily Gorbik <gor@linux.ibm.com>,
Vegard Nossum <vegard.nossum@oracle.com>,
Vlastimil Babka <vbabka@suse.cz>,
linux-mm@kvack.org, linux-arch@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 13/43] kmsan: add KMSAN runtime core
Date: Fri, 17 Dec 2021 17:22:31 +0100 [thread overview]
Message-ID: <Yby5Rwr0jgAcK4th@kroah.com> (raw)
In-Reply-To: <CAG_fn=XX3vbuo=cyG8C1Syv_JXiQ1rnfoffKqEc-N8uLei5T2A@mail.gmail.com>
On Thu, Dec 16, 2021 at 11:33:56AM +0100, Alexander Potapenko wrote:
> On Tue, Dec 14, 2021 at 5:34 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Tue, Dec 14, 2021 at 05:20:20PM +0100, Alexander Potapenko wrote:
> > > This patch adds the core parts of KMSAN runtime and associated files:
> > >
> > > - include/linux/kmsan-checks.h: user API to poison/unpoison/check
> > > the kernel memory;
> > > - include/linux/kmsan.h: declarations of KMSAN hooks to be referenced
> > > outside of KMSAN runtime;
> > > - lib/Kconfig.kmsan: CONFIG_KMSAN and related declarations;
> > > - Makefile, mm/Makefile, mm/kmsan/Makefile: boilerplate Makefile code;
> > > - mm/kmsan/annotations.c: non-inlineable implementation of KMSAN_INIT();
> > > - mm/kmsan/core.c: core functions that operate with shadow and origin
> > > memory and perform checks, utility functions;
> > > - mm/kmsan/hooks.c: KMSAN hooks for kernel subsystems;
> > > - mm/kmsan/init.c: KMSAN initialization routines;
> > > - mm/kmsan/instrumentation.c: functions called by KMSAN instrumentation;
> > > - mm/kmsan/kmsan.h: internal KMSAN declarations;
> > > - mm/kmsan/shadow.c: routines that encapsulate metadata creation and
> > > addressing;
> > > - scripts/Makefile.kmsan: CFLAGS_KMSAN
> > > - scripts/Makefile.lib: KMSAN_SANITIZE and KMSAN_ENABLE_CHECKS macros
> >
> >
> > That's an odd way to write a changelog, don't you think?
>
> Agreed. I'll try to concentrate on the functionality instead. Sorry about that.
>
> > You need to describe what you are doing here and why you are doing it.
> > Not a list of file names, we can see that in the diffstat.
> >
> > Also, you don't mention you are doing USB stuff here at all. And why
> > are you doing it here? That should be added in a later patch.
>
> You are right, USB is a good example of a stand-alone feature that can
> be moved to a separate patch.
>
> > Break this up into smaller, logical, pieces that add the infrastructure
> > and build on it. Don't just chop your patches up on a logical-file
> > boundry, as you are adding stuff in this patch that you do not need for
> > many more later on, which means it was not needed here.
>
> Just to make sure I don't misunderstand - for example for "kmsan: mm:
> call KMSAN hooks from SLUB code", would it be better to pull the code
> in mm/kmsan/core.c implementing kmsan_slab_alloc() and
> kmsan_slab_free() into that patch?
Yes.
> I thought maintainers would prefer to have patches to their code
> separated from KMSAN code, but if it's not true, I can surely fix
> that.
As a maintainer, I want to know what the function call that you just
added to my subsystem to call does. Wouldn't you? Put it all in the
same patch.
Think about submitting a patch series as telling a story. You need to
show the progression forward of the feature so that everyone can
understand what is going on. To just throw tiny snippets at us is
impossible to follow along with what your goal is.
You want reviewers to be able to easily see if the things you describe
being done in the changelog actually are implemented in the diff.
Dividing stuff up by files does not show that at all.
thanks,
greg k-h
next prev parent reply other threads:[~2021-12-17 16:22 UTC|newest]
Thread overview: 75+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-14 16:20 [PATCH 00/43] Add KernelMemorySanitizer infrastructure Alexander Potapenko
2021-12-14 16:20 ` [PATCH 01/43] arch/x86: add missing include to sparsemem.h Alexander Potapenko
2021-12-14 16:20 ` [PATCH 02/43] stackdepot: reserve 5 extra bits in depot_stack_handle_t Alexander Potapenko
2021-12-14 16:20 ` [PATCH 03/43] kasan: common: adapt to the new prototype of __stack_depot_save() Alexander Potapenko
2021-12-14 16:20 ` [PATCH 04/43] instrumented.h: allow instrumenting both sides of copy_from_user() Alexander Potapenko
2021-12-14 16:20 ` [PATCH 05/43] asm: x86: instrument usercopy in get_user() and __put_user_size() Alexander Potapenko
2021-12-14 16:20 ` [PATCH 06/43] asm-generic: instrument usercopy in cacheflush.h Alexander Potapenko
2021-12-14 16:20 ` [PATCH 07/43] compiler_attributes.h: add __disable_sanitizer_instrumentation Alexander Potapenko
2021-12-15 13:24 ` Mark Rutland
2021-12-15 13:33 ` Marco Elver
2021-12-14 16:20 ` [PATCH 08/43] kmsan: add ReST documentation Alexander Potapenko
2021-12-14 16:20 ` [PATCH 09/43] kmsan: introduce __no_sanitize_memory and __no_kmsan_checks Alexander Potapenko
2021-12-15 13:27 ` Mark Rutland
2021-12-14 16:20 ` [PATCH 10/43] kmsan: pgtable: reduce vmalloc space Alexander Potapenko
2021-12-15 13:36 ` Mark Rutland
2022-03-18 14:14 ` Alexander Potapenko
2021-12-14 16:20 ` [PATCH 11/43] libnvdimm/pfn_dev: increase MAX_STRUCT_PAGE_SIZE Alexander Potapenko
2021-12-14 16:20 ` [PATCH 12/43] kcsan: clang: retire CONFIG_KCSAN_KCOV_BROKEN Alexander Potapenko
2021-12-15 13:33 ` Mark Rutland
2021-12-15 13:39 ` Marco Elver
2021-12-15 14:43 ` Mark Rutland
2022-03-18 14:34 ` Alexander Potapenko
2021-12-14 16:20 ` [PATCH 13/43] kmsan: add KMSAN runtime core Alexander Potapenko
2021-12-14 16:34 ` Greg Kroah-Hartman
2021-12-16 10:33 ` Alexander Potapenko
2021-12-17 16:22 ` Greg Kroah-Hartman [this message]
2022-03-21 14:12 ` Alexander Potapenko
[not found] ` <CACT4Y+Y_torRwzh0eDMn+pJh=YT26hGrAPA499WqN1dV+4bDHA@mail.gmail.com>
2022-03-21 13:17 ` Alexander Potapenko
2021-12-14 16:20 ` [PATCH 14/43] MAINTAINERS: add entry for KMSAN Alexander Potapenko
2021-12-14 16:20 ` [PATCH 15/43] kmsan: mm: maintain KMSAN metadata for page operations Alexander Potapenko
2021-12-14 16:20 ` [PATCH 16/43] kmsan: mm: call KMSAN hooks from SLUB code Alexander Potapenko
2022-01-07 17:22 ` Vlastimil Babka
2022-03-25 13:15 ` Alexander Potapenko
2021-12-14 16:20 ` [PATCH 17/43] kmsan: handle task creation and exiting Alexander Potapenko
2021-12-14 16:20 ` [PATCH 18/43] kmsan: unpoison @tlb in arch_tlb_gather_mmu() Alexander Potapenko
2021-12-14 16:20 ` [PATCH 19/43] kmsan: init: call KMSAN initialization routines Alexander Potapenko
2021-12-14 16:20 ` [PATCH 20/43] instrumented.h: add KMSAN support Alexander Potapenko
2021-12-14 16:20 ` [PATCH 21/43] kmsan: mark noinstr as __no_sanitize_memory Alexander Potapenko
2021-12-15 13:49 ` Mark Rutland
2021-12-14 16:20 ` [PATCH 22/43] kmsan: initialize the output of READ_ONCE_NOCHECK() Alexander Potapenko
2021-12-14 16:20 ` [PATCH 23/43] kmsan: make READ_ONCE_TASK_STACK() return initialized values Alexander Potapenko
2021-12-14 16:20 ` [PATCH 24/43] kmsan: disable KMSAN instrumentation for certain kernel parts Alexander Potapenko
2021-12-15 13:53 ` Mark Rutland
2021-12-14 16:20 ` [PATCH 25/43] kmsan: skip shadow checks in files doing context switches Alexander Potapenko
2021-12-15 14:13 ` Mark Rutland
2021-12-15 16:28 ` Alexander Potapenko
2021-12-15 17:22 ` Mark Rutland
2021-12-14 16:20 ` [PATCH 26/43] kmsan: virtio: check/unpoison scatterlist in vring_map_one_sg() Alexander Potapenko
2022-01-06 12:46 ` Michael S. Tsirkin
2021-12-14 16:20 ` [PATCH 27/43] x86: kmsan: add iomem support Alexander Potapenko
2021-12-14 16:20 ` [PATCH 28/43] kmsan: dma: unpoison DMA mappings Alexander Potapenko
2021-12-14 16:20 ` [PATCH 29/43] kmsan: handle memory sent to/from USB Alexander Potapenko
2021-12-14 16:20 ` [PATCH 30/43] kmsan: add tests for KMSAN Alexander Potapenko
2021-12-14 16:20 ` [PATCH 31/43] kmsan: disable strscpy() optimization under KMSAN Alexander Potapenko
2021-12-14 16:20 ` [PATCH 32/43] crypto: kmsan: disable accelerated configs " Alexander Potapenko
2021-12-14 16:20 ` [PATCH 33/43] kmsan: disable physical page merging in biovec Alexander Potapenko
2021-12-15 14:17 ` Mark Rutland
2021-12-15 16:30 ` Alexander Potapenko
2021-12-14 16:20 ` [PATCH 34/43] kmsan: block: skip bio block merging logic for KMSAN Alexander Potapenko
2021-12-14 16:20 ` [PATCH 35/43] x86: kmsan: use __msan_ string functions where possible Alexander Potapenko
2021-12-14 16:20 ` [PATCH 36/43] x86: kmsan: sync metadata pages on page fault Alexander Potapenko
2021-12-14 16:20 ` [PATCH 37/43] x86: kasan: kmsan: support CONFIG_GENERIC_CSUM on x86, enable it for KASAN/KMSAN Alexander Potapenko
2021-12-14 16:20 ` [PATCH 38/43] x86: fs: kmsan: disable CONFIG_DCACHE_WORD_ACCESS Alexander Potapenko
2021-12-14 16:20 ` [PATCH 39/43] x86: kmsan: handle register passing from uninstrumented code Alexander Potapenko
2021-12-17 21:51 ` Thomas Gleixner
2021-12-20 14:35 ` Alexander Potapenko
2021-12-14 16:20 ` [PATCH 40/43] kmsan: kcov: unpoison area->list in kcov_remote_area_put() Alexander Potapenko
2021-12-14 16:20 ` [PATCH 41/43] security: kmsan: fix interoperability with auto-initialization Alexander Potapenko
2021-12-14 16:38 ` Greg Kroah-Hartman
2021-12-14 17:00 ` Alexander Potapenko
2021-12-14 17:33 ` Greg Kroah-Hartman
2021-12-14 16:20 ` [PATCH 42/43] objtool: kmsan: list KMSAN API functions as uaccess-safe Alexander Potapenko
2021-12-14 16:20 ` [PATCH 43/43] x86: kmsan: enable KMSAN builds for x86 Alexander Potapenko
2021-12-14 16:36 ` [PATCH 00/43] Add KernelMemorySanitizer infrastructure Greg Kroah-Hartman
2021-12-16 10:12 ` Alexander Potapenko
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=Yby5Rwr0jgAcK4th@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=akpm@linux-foundation.org \
--cc=andreyknvl@google.com \
--cc=arnd@arndb.de \
--cc=axboe@kernel.dk \
--cc=bp@alien8.de \
--cc=cl@linux.com \
--cc=dvyukov@google.com \
--cc=edumazet@google.com \
--cc=elver@google.com \
--cc=glider@google.com \
--cc=gor@linux.ibm.com \
--cc=hch@lst.de \
--cc=herbert@gondor.apana.org.au \
--cc=iamjoonsoo.kim@lge.com \
--cc=iii@linux.ibm.com \
--cc=keescook@chromium.org \
--cc=linux-arch@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=luto@kernel.org \
--cc=mingo@redhat.com \
--cc=mst@redhat.com \
--cc=penberg@kernel.org \
--cc=peterz@infradead.org \
--cc=pmladek@suse.com \
--cc=rientjes@google.com \
--cc=rostedt@goodmis.org \
--cc=tglx@linutronix.de \
--cc=vbabka@suse.cz \
--cc=vegard.nossum@oracle.com \
--cc=viro@zeniv.linux.org.uk \
--cc=willy@infradead.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 read-only IMAP folder(s) and NNTP newsgroup(s).