Skip to contentSkip to conversation

kill off ARCH_HAS_ATOMIC_UNSIGNED (take 2)

PATCH In review Dave Hansen 3 participants 5 messages Dec 17, 2004

Subsystems (5)
Memory management MM core Page allocator MGLRU +1 more subsystems
lore
Share & tools
lore

Dave Hansen

author
#0 lore raw

PATCH 1/1 kill off ARCH_HAS_ATOMIC_UNSIGNED (take 2)

Andi says that we don't need this on x86_64 any more. Since it is the only user, let's kill it off completely. BTW, this now makes 4 free bytes of space in page->flags for all 64-bit architectures to use.

Also get rid of the typedef.

Signed-off-by Dave Hansen <[email protected]>

---

 apw2-dave/include/asm-x86_64/bitops.h |    2 --
 apw2-dave/include/linux/mm.h          |   10 ++--------
 apw2-dave/include/linux/mmzone.h      |    2 +-
 apw2-dave/mm/filemap.c                |    2 +-
 apw2-dave/mm/page_alloc.c             |    2 +-
 5 files changed, 5 insertions(+), 13 deletions(-)

diff -puN arch/x86_64/Kconfig~000-CONFIG_ARCH_HAS_ATOMIC_UNSIGNED arch/x86_64/Kconfig

include/linux/mm.h renamed from include/linux/mm.h~000-CONFIG_ARCH_HAS_ATOMIC_UNSIGNED +2 -8
diff -puN include/linux/mm.h~000-CONFIG_ARCH_HAS_ATOMIC_UNSIGNED include/linux/mm.h
@@ -216,12 +216,6 @@ struct vm_operations_struct {
 struct mmu_gather;
 struct inode;
 
-#ifdef ARCH_HAS_ATOMIC_UNSIGNED
-typedef unsigned page_flags_t;
-#else
-typedef unsigned long page_flags_t;
-#endif
-
 /*
  * Each physical page in the system has a struct page associated with
  * it to keep track of whatever it is we are using the page for at the

@@ -229,7 +223,7 @@ typedef unsigned long page_flags_t;
  * a page.
  */
 struct page {
-	page_flags_t flags;		/* Atomic flags, some possibly
+	unsigned long flags;		/* Atomic flags, some possibly
 					 * updated asynchronously */
 	atomic_t _count;		/* Usage count, see below. */
 	atomic_t _mapcount;		/* Count of ptes mapped in mms,

@@ -409,7 +403,7 @@ static inline void put_page(struct page 
  * We'll have up to (MAX_NUMNODES * MAX_NR_ZONES) zones total,
  * so we use (MAX_NODES_SHIFT + MAX_ZONES_SHIFT) here to get enough bits.
  */
-#define NODEZONE_SHIFT (sizeof(page_flags_t)*8 - MAX_NODES_SHIFT - MAX_ZONES_SHIFT)
+#define NODEZONE_SHIFT (sizeof(unsigned long)*8 - MAX_NODES_SHIFT - MAX_ZONES_SHIFT)
 #define NODEZONE(node, zone)	((node << ZONES_SHIFT) | zone)
 
 static inline unsigned long page_zonenum(struct page *page)
include/linux/mmzone.h renamed from include/linux/mmzone.h~000-CONFIG_ARCH_HAS_ATOMIC_UNSIGNED +1 -1
diff -puN include/linux/mmzone.h~000-CONFIG_ARCH_HAS_ATOMIC_UNSIGNED include/linux/mmzone.h
@@ -388,7 +388,7 @@ extern struct pglist_data contig_page_da
 
 #include <asm/mmzone.h>
 
-#if BITS_PER_LONG == 32 || defined(ARCH_HAS_ATOMIC_UNSIGNED)
+#if BITS_PER_LONG == 32
 /*
  * with 32 bit page->flags field, we reserve 8 bits for node/zone info.
  * there are 3 zones (2 bits) and this leaves 8-2=6 bits for nodes.
include/asm-x86_64/bitops.h renamed from include/asm-x86_64/bitops.h~000-CONFIG_ARCH_HAS_ATOMIC_UNSIGNED +0 -2
diff -puN include/asm-x86_64/bitops.h~000-CONFIG_ARCH_HAS_ATOMIC_UNSIGNED include/asm-x86_64/bitops.h
@@ -411,8 +411,6 @@ static __inline__ int ffs(int x)
 /* find last set bit */
 #define fls(x) generic_fls(x)
 
-#define ARCH_HAS_ATOMIC_UNSIGNED 1
-
 #endif /* __KERNEL__ */
 
 #endif /* _X86_64_BITOPS_H */
mm/page_alloc.c renamed from mm/page_alloc.c~000-CONFIG_ARCH_HAS_ATOMIC_UNSIGNED +1 -1
diff -puN mm/page_alloc.c~000-CONFIG_ARCH_HAS_ATOMIC_UNSIGNED mm/page_alloc.c
@@ -85,7 +85,7 @@ static void bad_page(const char *functio
 	printk(KERN_EMERG "Bad page state at %s (in process '%s', page %p)\n",
 		function, current->comm, page);
 	printk(KERN_EMERG "flags:0x%0*lx mapping:%p mapcount:%d count:%d\n",
-		(int)(2*sizeof(page_flags_t)), (unsigned long)page->flags,
+		(int)(2*sizeof(unsigned long)), (unsigned long)page->flags,
 		page->mapping, page_mapcount(page), page_count(page));
 	printk(KERN_EMERG "Backtrace:\n");
 	dump_stack();
mm/filemap.c renamed from mm/filemap.c~000-CONFIG_ARCH_HAS_ATOMIC_UNSIGNED +1 -1
diff -puN mm/filemap.c~000-CONFIG_ARCH_HAS_ATOMIC_UNSIGNED mm/filemap.c
@@ -138,7 +138,7 @@ static int sync_page(void *word)
 	struct address_space *mapping;
 	struct page *page;
 
-	page = container_of((page_flags_t *)word, struct page, flags);
+	page = container_of((unsigned long *)word, struct page, flags);
 
 	/*
 	 * FIXME, fercrissake.  What is this barrier here for?

_
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#1 lore raw

mm/page_alloc.c static void bad_page(const char *functio · line 88

Quoted 8 lines · Dave Hansen
mm/page_alloc.c renamed from mm/page_alloc.c~000-CONFIG_ARCH_HAS_ATOMIC_UNSIGNED +1 -1
@@ -85,7 +85,7 @@ static void bad_page(const char *functio
 	printk(KERN_EMERG "Bad page state at %s (in process '%s', page %p)\n",
 		function, current->comm, page);
 	printk(KERN_EMERG "flags:0x%0*lx mapping:%p mapcount:%d count:%d\n",
-		(int)(2*sizeof(page_flags_t)), (unsigned long)page->flags,
+		(int)(2*sizeof(unsigned long)), (unsigned long)page->flags,

^^^^^^^^^^^^^^^

mm/page_alloc.c static void bad_page(const char *functio · line 91

Quoted 3 lines
mm/page_alloc.c renamed from mm/page_alloc.c~000-CONFIG_ARCH_HAS_ATOMIC_UNSIGNED +0 -0
 		page->mapping, page_mapcount(page), page_count(page));
 	printk(KERN_EMERG "Backtrace:\n");
 	dump_stack();

Teensy nit: better not to cast to unsigned long when it's unsigned long.

Hugh

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

author
#2 lore raw
Quoted 10 lines · Hugh Dickins

mm/page_alloc.c static void bad_page(const char *functio · line 88

Quoted 8 lines · Dave Hansen
mm/page_alloc.c renamed from mm/page_alloc.c~000-CONFIG_ARCH_HAS_ATOMIC_UNSIGNED +1 -1
@@ -85,7 +85,7 @@ static void bad_page(const char *functio
 	printk(KERN_EMERG "Bad page state at %s (in process '%s', page %p)\n",
 		function, current->comm, page);
 	printk(KERN_EMERG "flags:0x%0*lx mapping:%p mapcount:%d count:%d\n",
-		(int)(2*sizeof(page_flags_t)), (unsigned long)page->flags,
+		(int)(2*sizeof(unsigned long)), (unsigned long)page->flags,

^^^^^^^^^^^^^^^

Quoted 5 lines

mm/page_alloc.c static void bad_page(const char *functio · line 91

Quoted 3 lines
mm/page_alloc.c renamed from mm/page_alloc.c~000-CONFIG_ARCH_HAS_ATOMIC_UNSIGNED +0 -0
 		page->mapping, page_mapcount(page), page_count(page));
 	printk(KERN_EMERG "Backtrace:\n");
 	dump_stack();

Teensy nit: better not to cast to unsigned long when it's unsigned long.

My built-in s/// got a little carried away. How's this?

-- Dave

Andi says that we don't need this on x86_64 any more. Since it is the only user, let's kill it off completely. BTW, this now makes 4 free bytes of space in page->flags for all 64-bit architectures to use. Also get rid of the typedef.

Still compiles on x86.

Signed-off-by Dave Hansen <[email protected]>

---

 apw2-dave/include/asm-x86_64/bitops.h |    2 --
 apw2-dave/include/linux/mm.h          |   10 ++--------
 apw2-dave/include/linux/mmzone.h      |    2 +-
 apw2-dave/mm/filemap.c                |    2 +-
 apw2-dave/mm/page_alloc.c             |    2 +-
 5 files changed, 5 insertions(+), 13 deletions(-)

diff -puN arch/x86_64/Kconfig~000-CONFIG_ARCH_HAS_ATOMIC_UNSIGNED arch/x86_64/Kconfig

include/linux/mm.h renamed from include/linux/mm.h~000-CONFIG_ARCH_HAS_ATOMIC_UNSIGNED +2 -8
diff -puN include/linux/mm.h~000-CONFIG_ARCH_HAS_ATOMIC_UNSIGNED include/linux/mm.h
@@ -216,12 +216,6 @@ struct vm_operations_struct {
 struct mmu_gather;
 struct inode;
 
-#ifdef ARCH_HAS_ATOMIC_UNSIGNED
-typedef unsigned page_flags_t;
-#else
-typedef unsigned long page_flags_t;
-#endif
-
 /*
  * Each physical page in the system has a struct page associated with
  * it to keep track of whatever it is we are using the page for at the

@@ -229,7 +223,7 @@ typedef unsigned long page_flags_t;
  * a page.
  */
 struct page {
-	page_flags_t flags;		/* Atomic flags, some possibly
+	unsigned long flags;		/* Atomic flags, some possibly
 					 * updated asynchronously */
 	atomic_t _count;		/* Usage count, see below. */
 	atomic_t _mapcount;		/* Count of ptes mapped in mms,

@@ -409,7 +403,7 @@ static inline void put_page(struct page 
  * We'll have up to (MAX_NUMNODES * MAX_NR_ZONES) zones total,
  * so we use (MAX_NODES_SHIFT + MAX_ZONES_SHIFT) here to get enough bits.
  */
-#define NODEZONE_SHIFT (sizeof(page_flags_t)*8 - MAX_NODES_SHIFT - MAX_ZONES_SHIFT)
+#define NODEZONE_SHIFT (sizeof(unsigned long)*8 - MAX_NODES_SHIFT - MAX_ZONES_SHIFT)
 #define NODEZONE(node, zone)	((node << ZONES_SHIFT) | zone)
 
 static inline unsigned long page_zonenum(struct page *page)
include/linux/mmzone.h renamed from include/linux/mmzone.h~000-CONFIG_ARCH_HAS_ATOMIC_UNSIGNED +1 -1
diff -puN include/linux/mmzone.h~000-CONFIG_ARCH_HAS_ATOMIC_UNSIGNED include/linux/mmzone.h
@@ -388,7 +388,7 @@ extern struct pglist_data contig_page_da
 
 #include <asm/mmzone.h>
 
-#if BITS_PER_LONG == 32 || defined(ARCH_HAS_ATOMIC_UNSIGNED)
+#if BITS_PER_LONG == 32
 /*
  * with 32 bit page->flags field, we reserve 8 bits for node/zone info.
  * there are 3 zones (2 bits) and this leaves 8-2=6 bits for nodes.
include/asm-x86_64/bitops.h renamed from include/asm-x86_64/bitops.h~000-CONFIG_ARCH_HAS_ATOMIC_UNSIGNED +0 -2
diff -puN include/asm-x86_64/bitops.h~000-CONFIG_ARCH_HAS_ATOMIC_UNSIGNED include/asm-x86_64/bitops.h
@@ -411,8 +411,6 @@ static __inline__ int ffs(int x)
 /* find last set bit */
 #define fls(x) generic_fls(x)
 
-#define ARCH_HAS_ATOMIC_UNSIGNED 1
-
 #endif /* __KERNEL__ */
 
 #endif /* _X86_64_BITOPS_H */
mm/page_alloc.c renamed from mm/page_alloc.c~000-CONFIG_ARCH_HAS_ATOMIC_UNSIGNED +1 -1
diff -puN mm/page_alloc.c~000-CONFIG_ARCH_HAS_ATOMIC_UNSIGNED mm/page_alloc.c
@@ -85,7 +85,7 @@ static void bad_page(const char *functio
 	printk(KERN_EMERG "Bad page state at %s (in process '%s', page %p)\n",
 		function, current->comm, page);
 	printk(KERN_EMERG "flags:0x%0*lx mapping:%p mapcount:%d count:%d\n",
-		(int)(2*sizeof(page_flags_t)), (unsigned long)page->flags,
+		(int)(2*sizeof(unsigned long)), page->flags,
 		page->mapping, page_mapcount(page), page_count(page));
 	printk(KERN_EMERG "Backtrace:\n");
 	dump_stack();
mm/filemap.c renamed from mm/filemap.c~000-CONFIG_ARCH_HAS_ATOMIC_UNSIGNED +1 -1
diff -puN mm/filemap.c~000-CONFIG_ARCH_HAS_ATOMIC_UNSIGNED mm/filemap.c
@@ -138,7 +138,7 @@ static int sync_page(void *word)
 	struct address_space *mapping;
 	struct page *page;
 
-	page = container_of((page_flags_t *)word, struct page, flags);
+	page = container_of((unsigned long *)word, struct page, flags);
 
 	/*
 	 * FIXME, fercrissake.  What is this barrier here for?

_

#4 lore raw
In reply to Dave Hansen

mm/page_alloc.c static void bad_page(const char *functio · line 89

Quoted 4 lines · Dave
mm/page_alloc.c renamed from mm/page_alloc.c~000-CONFIG_ARCH_HAS_ATOMIC_UNSIGNED +1 -1
 	printk(KERN_EMERG "flags:0x%0*lx mapping:%p mapcount:%d count:%d\n",
-		(int)(2*sizeof(page_flags_t)), (unsigned long)page->flags,
+		(int)(2*sizeof(unsigned long)), page->flags,
 		page->mapping, page_mapcount(page), page_count(page));

I've a slight preference for:

	printk(KERN_EMERG "flags:0x%0*lx mapping:%p mapcount:%d count:%d\n",
		(int)(2*sizeof(page->flags)), page->flags,
		page->mapping, page_mapcount(page), page_count(page));

or perhaps even a little better:

	printk(KERN_EMERG "flags:0x%08lx mapping:%p mapcount:%d count:%d\n",
				page->flags, page->mapping,
				page_mapcount(page), page_count(page));

Most plain unsigned longs are displayed with some variant of %8lx, not a %*lx variable sized format. And in general, the plainer the code, the quicker the reader can understand it.

But if you don't find my nit picking both pleasing and convenient, don't hesitate to dismiss it. It's no biggie, and my comment is just a drive by shooting so has little standing.

Signature · 8 lines
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <[email protected]> 1.650.933.1373, 1.925.600.0401
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>