Re: [PATCH v2 06/39] mm: enumerate all gfp flags

On Wed, 25 Oct 2023 08:28:32 -0700
Suren Baghdasaryan [off-list ref] wrote:

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

quoted

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.

quoted

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.

You're right. If something does go wrong, it will be easier to fix if
the removal of the unused bit is in a commit of its own.

Petr T