On Wed, Nov 27, 2019 at 3:48 PM Marco Elver <elver@xxxxxxxxxx> wrote: > > On Fri, 22 Nov 2019 at 12:26, <glider@xxxxxxxxxx> 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@xxxxxxxxxx> > > To: Alexander Potapenko <glider@xxxxxxxxxx> > > Cc: Vegard Nossum <vegard.nossum@xxxxxxxxxx> > > Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> > > Cc: Dmitry Vyukov <dvyukov@xxxxxxxxxx> > > Cc: linux-mm@xxxxxxxxx > > > > --- > > 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. I actually find it quite unfortunate that LOCKDEP has this on/off case. It's already inconvenient to add a new GFP flag past ___GFP_NOLOCKDEP, as its value will be depending on this blinking LOCKDEP bit. Now imagine someone wants to add a second GFP flag that can be disabled similarly to LOCKDEP - there'll be even more hassle counting which bits are present. Michal, do you think it's possible to make __GFP_BITS_SHIFT independent from CONFIG_LOCKDEP? > > 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 > > -- Alexander Potapenko Software Engineer Google Germany GmbH Erika-Mann-Straße, 33 80636 München Geschäftsführer: Paul Manicle, Halimah DeLaine Prado Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg
