On Tue, Oct 24, 2023 at 10:47 PM Petr Tesařík [off-list ref] wrote:

On Tue, 24 Oct 2023 06:46:03 -0700
Suren Baghdasaryan [off-list ref] wrote:

quoted

Introduce GFP bits enumeration to let compiler track the number of used
bits (which depends on the config options) instead of hardcoding them.
That simplifies __GFP_BITS_SHIFT calculation.
Suggested-by: Petr Tesařík <redacted>
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
---
 include/linux/gfp_types.h | 90 +++++++++++++++++++++++++++------------
 1 file changed, 62 insertions(+), 28 deletions(-)

diff --git a/include/linux/gfp_types.h b/include/linux/gfp_types.h
index 6583a58670c5..3fbe624763d9 100644
--- a/include/linux/gfp_types.h
+++ b/include/linux/gfp_types.h

@@ -21,44 +21,78 @@ typedef unsigned int __bitwise gfp_t;
  * include/trace/events/mmflags.h and tools/perf/builtin-kmem.c
  */

+enum {
+     ___GFP_DMA_BIT,
+     ___GFP_HIGHMEM_BIT,
+     ___GFP_DMA32_BIT,
+     ___GFP_MOVABLE_BIT,
+     ___GFP_RECLAIMABLE_BIT,
+     ___GFP_HIGH_BIT,
+     ___GFP_IO_BIT,
+     ___GFP_FS_BIT,
+     ___GFP_ZERO_BIT,
+     ___GFP_UNUSED_BIT,      /* 0x200u unused */
+     ___GFP_DIRECT_RECLAIM_BIT,
+     ___GFP_KSWAPD_RECLAIM_BIT,
+     ___GFP_WRITE_BIT,
+     ___GFP_NOWARN_BIT,
+     ___GFP_RETRY_MAYFAIL_BIT,
+     ___GFP_NOFAIL_BIT,
+     ___GFP_NORETRY_BIT,
+     ___GFP_MEMALLOC_BIT,
+     ___GFP_COMP_BIT,
+     ___GFP_NOMEMALLOC_BIT,
+     ___GFP_HARDWALL_BIT,
+     ___GFP_THISNODE_BIT,
+     ___GFP_ACCOUNT_BIT,
+     ___GFP_ZEROTAGS_BIT,
+#ifdef CONFIG_KASAN_HW_TAGS
+     ___GFP_SKIP_ZERO_BIT,
+     ___GFP_SKIP_KASAN_BIT,
+#endif
+#ifdef CONFIG_LOCKDEP
+     ___GFP_NOLOCKDEP_BIT,
+#endif
+     ___GFP_LAST_BIT
+};
+
 /* Plain integer GFP bitmasks. Do not use this directly. */
-#define ___GFP_DMA           0x01u
-#define ___GFP_HIGHMEM               0x02u
-#define ___GFP_DMA32         0x04u
-#define ___GFP_MOVABLE               0x08u
-#define ___GFP_RECLAIMABLE   0x10u
-#define ___GFP_HIGH          0x20u
-#define ___GFP_IO            0x40u
-#define ___GFP_FS            0x80u
-#define ___GFP_ZERO          0x100u
+#define ___GFP_DMA           BIT(___GFP_DMA_BIT)
+#define ___GFP_HIGHMEM               BIT(___GFP_HIGHMEM_BIT)
+#define ___GFP_DMA32         BIT(___GFP_DMA32_BIT)
+#define ___GFP_MOVABLE               BIT(___GFP_MOVABLE_BIT)
+#define ___GFP_RECLAIMABLE   BIT(___GFP_RECLAIMABLE_BIT)
+#define ___GFP_HIGH          BIT(___GFP_HIGH_BIT)
+#define ___GFP_IO            BIT(___GFP_IO_BIT)
+#define ___GFP_FS            BIT(___GFP_FS_BIT)
+#define ___GFP_ZERO          BIT(___GFP_ZERO_BIT)
 /* 0x200u unused */

This comment can be also removed here, because it is already stated
above with the definition of ___GFP_UNUSED_BIT.

Ack.

Then again, I think that the GFP bits have never been compacted after
Neil Brown removed __GFP_ATOMIC with commit 2973d8229b78 simply because
that would mean changing definitions of all subsequent GFP flags. FWIW
I am not aware of any code that would depend on the numeric value of
___GFP_* macros, so this patch seems like a good opportunity to change
the numbering and get rid of this unused 0x200u altogether.

@Neil: I have added you to the conversation in case you want to correct
my understanding of the unused bit.

Hmm. I would prefer to do that in a separate patch even though it
would be a one-line change. Seems safer to me in case something goes
wrong and we have to bisect and revert it. If that sounds ok I'll post
that in the next version.

Other than that LGTM.

Thanks for the review!
Suren.

Petr T

quoted

-#define ___GFP_DIRECT_RECLAIM        0x400u
-#define ___GFP_KSWAPD_RECLAIM        0x800u
-#define ___GFP_WRITE         0x1000u
-#define ___GFP_NOWARN                0x2000u
-#define ___GFP_RETRY_MAYFAIL 0x4000u
-#define ___GFP_NOFAIL                0x8000u
-#define ___GFP_NORETRY               0x10000u
-#define ___GFP_MEMALLOC              0x20000u
-#define ___GFP_COMP          0x40000u
-#define ___GFP_NOMEMALLOC    0x80000u
-#define ___GFP_HARDWALL              0x100000u
-#define ___GFP_THISNODE              0x200000u
-#define ___GFP_ACCOUNT               0x400000u
-#define ___GFP_ZEROTAGS              0x800000u
+#define ___GFP_DIRECT_RECLAIM        BIT(___GFP_DIRECT_RECLAIM_BIT)
+#define ___GFP_KSWAPD_RECLAIM        BIT(___GFP_KSWAPD_RECLAIM_BIT)
+#define ___GFP_WRITE         BIT(___GFP_WRITE_BIT)
+#define ___GFP_NOWARN                BIT(___GFP_NOWARN_BIT)
+#define ___GFP_RETRY_MAYFAIL BIT(___GFP_RETRY_MAYFAIL_BIT)
+#define ___GFP_NOFAIL                BIT(___GFP_NOFAIL_BIT)
+#define ___GFP_NORETRY               BIT(___GFP_NORETRY_BIT)
+#define ___GFP_MEMALLOC              BIT(___GFP_MEMALLOC_BIT)
+#define ___GFP_COMP          BIT(___GFP_COMP_BIT)
+#define ___GFP_NOMEMALLOC    BIT(___GFP_NOMEMALLOC_BIT)
+#define ___GFP_HARDWALL              BIT(___GFP_HARDWALL_BIT)
+#define ___GFP_THISNODE              BIT(___GFP_THISNODE_BIT)
+#define ___GFP_ACCOUNT               BIT(___GFP_ACCOUNT_BIT)
+#define ___GFP_ZEROTAGS              BIT(___GFP_ZEROTAGS_BIT)
 #ifdef CONFIG_KASAN_HW_TAGS
-#define ___GFP_SKIP_ZERO     0x1000000u
-#define ___GFP_SKIP_KASAN    0x2000000u
+#define ___GFP_SKIP_ZERO     BIT(___GFP_SKIP_ZERO_BIT)
+#define ___GFP_SKIP_KASAN    BIT(___GFP_SKIP_KASAN_BIT)
 #else
 #define ___GFP_SKIP_ZERO     0
 #define ___GFP_SKIP_KASAN    0
 #endif
 #ifdef CONFIG_LOCKDEP
-#define ___GFP_NOLOCKDEP     0x4000000u
+#define ___GFP_NOLOCKDEP     BIT(___GFP_NOLOCKDEP_BIT)
 #else
 #define ___GFP_NOLOCKDEP     0
 #endif
-/* If the above are modified, __GFP_BITS_SHIFT may need updating */

 /*
  * Physical address zone modifiers (see linux/mmzone.h - low four bits)

@@ -249,7 +283,7 @@ typedef unsigned int __bitwise gfp_t;
 #define __GFP_NOLOCKDEP ((__force gfp_t)___GFP_NOLOCKDEP)

 /* Room for N __GFP_FOO bits */
-#define __GFP_BITS_SHIFT (26 + IS_ENABLED(CONFIG_LOCKDEP))
+#define __GFP_BITS_SHIFT ___GFP_LAST_BIT
 #define __GFP_BITS_MASK ((__force gfp_t)((1 << __GFP_BITS_SHIFT) - 1))

 /**