Re: [PATCH RFC v3 06/36] kmsan: gfp: introduce __GFP_NO_KMSAN_SHADOW

From: Marco Elver <elver@google.com>
To: Alexander Potapenko <glider@google.com>
Cc: Vegard Nossum <vegard.nossum@oracle.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	 Dmitry Vyukov <dvyukov@google.com>,
	Linux Memory Management List <linux-mm@kvack.org>,
	 Al Viro <viro@zeniv.linux.org.uk>,
	adilger.kernel@dilger.ca,
	 Andrey Konovalov <andreyknvl@google.com>,
	Andrey Ryabinin <aryabinin@virtuozzo.com>,
	 Andy Lutomirski <luto@kernel.org>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	 Arnd Bergmann <arnd@arndb.de>,
	hch@infradead.org, hch@lst.de, darrick.wong@oracle.com,
	 davem@davemloft.net, dmitry.torokhov@gmail.com,
	ebiggers@google.com,  Eric Dumazet <edumazet@google.com>,
	ericvh@gmail.com, gregkh@linuxfoundation.org,
	 harry.wentland@amd.com, herbert@gondor.apana.org.au,
	iii@linux.ibm.com,  mingo@elte.hu, jasowang@redhat.com,
	axboe@kernel.dk, m.szyprowski@samsung.com,
	 Mark Rutland <mark.rutland@arm.com>,
	martin.petersen@oracle.com, schwidefsky@de.ibm.com,
	 Matthew Wilcox <willy@infradead.org>,
	mst@redhat.com, monstr@monstr.eu, pmladek@suse.com,
	 Qian Cai <cai@lca.pw>, Randy Dunlap <rdunlap@infradead.org>,
	robin.murphy@arm.com,
	 Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	tiwai@suse.com,  tytso@mit.edu,
	Thomas Gleixner <tglx@linutronix.de>,
	gor@linux.ibm.com, wsa@the-dreams.de
Subject: Re: [PATCH RFC v3 06/36] kmsan: gfp: introduce __GFP_NO_KMSAN_SHADOW
Date: Wed, 27 Nov 2019 15:48:08 +0100	[thread overview]
Message-ID: <CANpmjNNJuWvN2J9CxGuvkKJZfOsJhH5f13t9fS2zMztNOGU6KQ@mail.gmail.com> (raw)
In-Reply-To: <20191122112621.204798-7-glider@google.com>

On Fri, 22 Nov 2019 at 12:26, <glider@google.com> wrote:
>
> This flag is to be used by KMSAN runtime to mark that newly created
> memory pages don't need KMSAN metadata backing them.
>
> Signed-off-by: Alexander Potapenko <glider@google.com>
> To: Alexander Potapenko <glider@google.com>
> Cc: Vegard Nossum <vegard.nossum@oracle.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Dmitry Vyukov <dvyukov@google.com>
> Cc: linux-mm@kvack.org
>
> ---
> We can't decide what to do here:
>  - do we need to conditionally define ___GFP_NO_KMSAN_SHADOW depending on
>    CONFIG_KMSAN like LOCKDEP does?
>  - if KMSAN is defined, and LOCKDEP is not, do we want to "compactify" the GFP
>    bits?

A maintainer would know the real answer, but this is my guess: making
the behaviour not change without KMSAN would probably be better. It
would require some ifdef trickery to deal with LOCKDEP on/off case
though.

> Change-Id: If5d0352fd5711ad103328e2c185eb885e826423a
> ---
>  include/linux/gfp.h | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> index fb07b503dc45..b4e7963cd94b 100644
> --- a/include/linux/gfp.h
> +++ b/include/linux/gfp.h
> @@ -44,6 +44,7 @@ struct vm_area_struct;
>  #else
>  #define ___GFP_NOLOCKDEP       0
>  #endif

Since this change unconditionally changes GFP_BITS_SHIFT to 25, the
#ifdef for GFP_NOLOCKDEP could also go away -- but: see above.

> +#define ___GFP_NO_KMSAN_SHADOW  0x1000000u
>  /* If the above are modified, __GFP_BITS_SHIFT may need updating */
>
>  /*
> @@ -212,12 +213,13 @@ struct vm_area_struct;
>  #define __GFP_NOWARN   ((__force gfp_t)___GFP_NOWARN)
>  #define __GFP_COMP     ((__force gfp_t)___GFP_COMP)
>  #define __GFP_ZERO     ((__force gfp_t)___GFP_ZERO)
> +#define __GFP_NO_KMSAN_SHADOW  ((__force gfp_t)___GFP_NO_KMSAN_SHADOW)

Should this be ordered after __GFP_NOLOCKDEP with a brief comment what
it does?  All of these up to __GFP_ZERO have a doc comment above.

>  /* Disable lockdep for GFP context tracking */
>  #define __GFP_NOLOCKDEP ((__force gfp_t)___GFP_NOLOCKDEP)
>
>  /* Room for N __GFP_FOO bits */
> -#define __GFP_BITS_SHIFT (23 + IS_ENABLED(CONFIG_LOCKDEP))
> +#define __GFP_BITS_SHIFT (25)
>  #define __GFP_BITS_MASK ((__force gfp_t)((1 << __GFP_BITS_SHIFT) - 1))
>
>  /**
> --
> 2.24.0.432.g9d3f5f5b63-goog
>