Skip to contentSkip to conversation

cpusets new __GFP_HARDWALL flag]

PATCH Joel Schopp 4 participants 13 messages Jul 11, 2005 – Jul 27, 2005 1 message not in the archive

Subsystems (7)
Memory management VFS MM core Page allocator +3 more subsystems
lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): 1121101013.15095.19.camel@localhost

Joel Schopp

author
#1 lore raw

Dave Hansen brought this to my attention. I've attached the bit of the memory fragmentation avoidance you conflict with (I'm working with Mel on his patches). I think we share similar goals, and I wouldn't mind changing __GFP_USERRCLM to __GFP_USERALLOC or some neutral name we could share. Anything to increase the chances of fragmentation avoidance getting merged is good in my book.

-Joel

Quoted 38 lines
Quoted 7 lines

GFP_USER allocations, and distinguish them from GFP_KERNEL allocations.

Allocations (such as GFP_USER) marked GFP_HARDWALL are constrainted to the current tasks cpuset. Other allocations (such as GFP_KERNEL) can steal from the possibly larger nearest mem_exclusive cpuset ancestor, if memory is tight on every node in the current cpuset.

Quoted 6 lines

This patch collides with Mel Gorman's patch to reduce fragmentation in the standard buddy allocator, which adds two GFP flags. At first glance, it seems that his added __GFP_USERRCLM flag could be used in place of the following __GFP_HARDWALL, as they both seem to be set the same way - for GFP_USER and GFP_HIGHUSER. Perhaps we should call this flag __GFP_USER, rather than some name dependent on its use(s).

Does this make sense to integrate into your patches?

Index: linux-2.6-mem_exclusive/include/linux/gfp.h
===================================================================

include/linux/gfp.h +2 -1
@@ -40,6 +40,7 @@ struct vm_area_struct;
 #define __GFP_ZERO	0x8000u	/* Return zeroed page on success */
 #define __GFP_NOMEMALLOC 0x10000u /* Don't use emergency reserves */
 #define __GFP_NORECLAIM  0x20000u /* No zone reclaim during page_cache_alloc */
+#define __GFP_HARDWALL   0x40000u /* Enforce hardwall cpuset memory allocs */
 
 #define __GFP_BITS_SHIFT 20	/* Room for 20 __GFP_FOO bits */
 #define __GFP_BITS_MASK ((1 << __GFP_BITS_SHIFT) - 1)

@@ -48,14 +49,15 @@ struct vm_area_struct;
 #define GFP_LEVEL_MASK (__GFP_WAIT|__GFP_HIGH|__GFP_IO|__GFP_FS| \
 			__GFP_COLD|__GFP_NOWARN|__GFP_REPEAT| \
 			__GFP_NOFAIL|__GFP_NORETRY|__GFP_NO_GROW|__GFP_COMP| \
-			__GFP_NOMEMALLOC|__GFP_NORECLAIM)
+			__GFP_NOMEMALLOC|__GFP_NORECLAIM|__GFP_HARDWALL)
 
 #define GFP_ATOMIC	(__GFP_HIGH)
 #define GFP_NOIO	(__GFP_WAIT)

include/linux/gfp.h struct vm_area_struct; · line 63

Quoted 10 lines
include/linux/gfp.h +3 -2
 #define GFP_NOFS	(__GFP_WAIT | __GFP_IO)
 #define GFP_KERNEL	(__GFP_WAIT | __GFP_IO | __GFP_FS)
-#define GFP_USER	(__GFP_WAIT | __GFP_IO | __GFP_FS)
-#define GFP_HIGHUSER	(__GFP_WAIT | __GFP_IO | __GFP_FS | __GFP_HIGHMEM)
+#define GFP_USER	(__GFP_WAIT | __GFP_IO | __GFP_FS | __GFP_HARDWALL)
+#define GFP_HIGHUSER	(__GFP_WAIT | __GFP_IO | __GFP_FS | __GFP_HARDWALL | \
+			 __GFP_HIGHMEM)
 
 /* Flag - indicates that the buffer will be suitable for DMA.  Ignored on some
    platforms, used as appropriate on others */
fs/buffer.c +4 -2
Index: 2.6.13-rc1-mhp1/fs/buffer.c
===================================================================
@@ -1110,7 +1110,8 @@ grow_dev_page(struct block_device *bdev,
 	struct page *page;
 	struct buffer_head *bh;
 
-	page = find_or_create_page(inode->i_mapping, index, GFP_NOFS);
+	page = find_or_create_page(inode->i_mapping, index,
+				   GFP_NOFS | __GFP_USERRCLM);
 	if (!page)
 		return NULL;
 

@@ -3079,7 +3080,8 @@ static void recalc_bh_state(void)
 	
 struct buffer_head *alloc_buffer_head(unsigned int __nocast gfp_flags)
 {
-	struct buffer_head *ret = kmem_cache_alloc(bh_cachep, gfp_flags);
+	struct buffer_head *ret = kmem_cache_alloc(bh_cachep,
+						   gfp_flags|__GFP_KERNRCLM);
 	if (ret) {
 		preempt_disable();
 		__get_cpu_var(bh_accounting).nr++;
fs/dcache.c +1 -1
Index: 2.6.13-rc1-mhp1/fs/dcache.c
===================================================================
@@ -719,7 +719,7 @@ struct dentry *d_alloc(struct dentry * p
 	struct dentry *dentry;
 	char *dname;
 
-	dentry = kmem_cache_alloc(dentry_cache, GFP_KERNEL); 
+	dentry = kmem_cache_alloc(dentry_cache, GFP_KERNEL|__GFP_KERNRCLM);
 	if (!dentry)
 		return NULL;
fs/ext2/super.c +2 -1
Index: 2.6.13-rc1-mhp1/fs/ext2/super.c
===================================================================
@@ -138,7 +138,8 @@ static kmem_cache_t * ext2_inode_cachep;
 static struct inode *ext2_alloc_inode(struct super_block *sb)
 {
 	struct ext2_inode_info *ei;
-	ei = (struct ext2_inode_info *)kmem_cache_alloc(ext2_inode_cachep, SLAB_KERNEL);
+	ei = (struct ext2_inode_info *)kmem_cache_alloc(ext2_inode_cachep,
+						SLAB_KERNEL|__GFP_KERNRCLM);
 	if (!ei)
 		return NULL;
 #ifdef CONFIG_EXT2_FS_POSIX_ACL
fs/ext3/super.c +1 -1
Index: 2.6.13-rc1-mhp1/fs/ext3/super.c
===================================================================
@@ -440,7 +440,7 @@ static struct inode *ext3_alloc_inode(st
 {
 	struct ext3_inode_info *ei;
 
-	ei = kmem_cache_alloc(ext3_inode_cachep, SLAB_NOFS);
+	ei = kmem_cache_alloc(ext3_inode_cachep, SLAB_NOFS|__GFP_KERNRCLM);
 	if (!ei)
 		return NULL;
 #ifdef CONFIG_EXT3_FS_POSIX_ACL
fs/ntfs/inode.c +4 -3
Index: 2.6.13-rc1-mhp1/fs/ntfs/inode.c
===================================================================
@@ -317,8 +317,8 @@ struct inode *ntfs_alloc_big_inode(struc
 	ntfs_inode *ni;
 
 	ntfs_debug("Entering.");
-	ni = (ntfs_inode *)kmem_cache_alloc(ntfs_big_inode_cache,
-			SLAB_NOFS);
+	ni = (ntfs_inode *)kmem_cache_alloc(ntfs_big_inode_cache,
+					    SLAB_NOFS|__GFP_KERNRCLM);
 	if (likely(ni != NULL)) {
 		ni->state = 0;
 		return VFS_I(ni);

@@ -343,7 +343,8 @@ static inline ntfs_inode *ntfs_alloc_ext
 	ntfs_inode *ni;
 
 	ntfs_debug("Entering.");
-	ni = (ntfs_inode *)kmem_cache_alloc(ntfs_inode_cache, SLAB_NOFS);
+	ni = (ntfs_inode *)kmem_cache_alloc(ntfs_inode_cache,
+					    SLAB_NOFS|__GFP_KERNRCLM);
 	if (likely(ni != NULL)) {
 		ni->state = 0;
 		return ni;
include/linux/gfp.h +8 -4
Index: 2.6.13-rc1-mhp1/include/linux/gfp.h
===================================================================
@@ -40,22 +40,26 @@ struct vm_area_struct;
 #define __GFP_ZERO	0x8000u	/* Return zeroed page on success */
 #define __GFP_NOMEMALLOC 0x10000u /* Don't use emergency reserves */
 #define __GFP_NORECLAIM  0x20000u /* No realy zone reclaim during allocation */
+#define __GFP_KERNRCLM 0x40000u /* Kernel page that is easily reclaimable */
+#define __GFP_USERRCLM 0x80000u /* User is a userspace user */
 
-#define __GFP_BITS_SHIFT 20	/* Room for 20 __GFP_FOO bits */
+#define __GFP_BITS_SHIFT 22	/* Room for 22 __GFP_FOO bits */
 #define __GFP_BITS_MASK ((1 << __GFP_BITS_SHIFT) - 1)
 
 /* if you forget to add the bitmask here kernel will crash, period */
 #define GFP_LEVEL_MASK (__GFP_WAIT|__GFP_HIGH|__GFP_IO|__GFP_FS| \
 			__GFP_COLD|__GFP_NOWARN|__GFP_REPEAT| \
 			__GFP_NOFAIL|__GFP_NORETRY|__GFP_NO_GROW|__GFP_COMP| \
-			__GFP_NOMEMALLOC|__GFP_NORECLAIM)
+			__GFP_NOMEMALLOC|__GFP_NORECLAIM| \
+			__GFP_USERRCLM|__GFP_KERNRCLM)
 
 #define GFP_ATOMIC	(__GFP_HIGH)
 #define GFP_NOIO	(__GFP_WAIT)
 #define GFP_NOFS	(__GFP_WAIT | __GFP_IO)
 #define GFP_KERNEL	(__GFP_WAIT | __GFP_IO | __GFP_FS)
-#define GFP_USER	(__GFP_WAIT | __GFP_IO | __GFP_FS)
-#define GFP_HIGHUSER	(__GFP_WAIT | __GFP_IO | __GFP_FS | __GFP_HIGHMEM)
+#define GFP_USER	(__GFP_WAIT | __GFP_IO | __GFP_FS | __GFP_USERRCLM)
+#define GFP_HIGHUSER	(__GFP_WAIT | __GFP_IO | __GFP_FS | __GFP_HIGHMEM | \
+			 __GFP_USERRCLM)
 
 /* Flag - indicates that the buffer will be suitable for DMA.  Ignored on some
    platforms, used as appropriate on others */
#2 lore raw
Quoted 6 lines · Joel Schopp

Dave Hansen brought this to my attention. I've attached the bit of the memory fragmentation avoidance you conflict with (I'm working with Mel on his patches). I think we share similar goals, and I wouldn't mind changing __GFP_USERRCLM to __GFP_USERALLOC or some neutral name we could share. Anything to increase the chances of fragmentation avoidance getting merged is good in my book.

The nice part about using __GFP_USER as the name is that it describes how it's going to be used rather than how the kernel is going to treat it. Somebody making a random allocator call is much more likely to know how they're going to use it than how the kernel _should_.

-- Dave

--
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>

#3 lore raw
In reply to Joel Schopp
Quoted 2 lines · Joel

I wouldn't mind changing __GFP_USERRCLM to __GFP_USERALLOC or some neutral name we could share.

A neutral term would be good. Since you are ahead of me (being already in Andrew's tree, while I just made my first linux-mm post), I figure that means you get to pick the name. Unless it is seriously defective for my purposes, I will just accept what is.

Quoted 3 lines · Dave

The nice part about using __GFP_USER as the name is that it describes how it's going to be used rather than how the kernel is going to treat it.

Yup - agreed. Though, in real life, that's hidden beneath the (no underscore) GFP_USER flag, so it's only a few kernel memory hackers we will be confusing, not the horde of driver writers.

One question. I've not actually read the memory fragmentation avoidance patch, so this might be a stupid question. That notwithstanding, do you really need two flags, one KERN and one USER? Or would one flag be sufficient - to mark USER pages. Unmarked pages would be KERN, presumably. One really only needs 2 bits if one has 3 or 4 states to track -- if that's the case, it's not clear to me what those 3 or 4 states are (maybe if I actually read the patch it would be clear ;).

I intended to CC Mel on the original post -- but then forgot to. Thanks for passing it along to him, Dave.

Signature · 8 lines
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <[email protected]> 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>
#4 lore raw
Quoted 8 lines · Paul Jackson

One question. I've not actually read the memory fragmentation avoidance patch, so this might be a stupid question. That notwithstanding, do you really need two flags, one KERN and one USER? Or would one flag be sufficient - to mark USER pages. Unmarked pages would be KERN, presumably. One really only needs 2 bits if one has 3 or 4 states to track -- if that's the case, it's not clear to me what those 3 or 4 states are (maybe if I actually read the patch it would be clear ;).

There are four types, but it only consumes two GFP bits. It's correctly packed.

-- Dave

--
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>

#5 lore raw
Quoted 1 line · Dave

four types ==> two GFP bits

Ok. Guess I should read the patch to figure out
what these 4 types are (and which subsets thereof
map to my 2 types USER and !USER aka KERN.)

If there is not a surjective function from your
4 types to my 2 types, then I can't so easily
share your GFP bits.

Signature · 8 lines
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <[email protected]> 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>
#6 lore raw
In reply to Paul Jackson
Quoted 5 lines · Paul Jackson
Quoted 2 lines · Joel

I wouldn't mind changing __GFP_USERRCLM to __GFP_USERALLOC or some neutral name we could share.

A neutral term would be good.

I agree as having two flags for essentially the same thing is just a waste. I guess there will be some discussion on what the other fragmentation flags should be called.

For consistency, we might want to rethink the name of KERNRCLM. I am still happy with the name as it says "this kernel allocation is something that will be reclaimed shortly or can be reclaimed on demand" but others might feel differently.

Quoted 5 lines
Quoted 3 lines · Dave

The nice part about using __GFP_USER as the name is that it describes how it's going to be used rather than how the kernel is going to treat it.

Quoted 7 lines

Yup - agreed. Though, in real life, that's hidden beneath the (no underscore) GFP_USER flag, so it's only a few kernel memory hackers we will be confusing, not the horde of driver writers.

One question. I've not actually read the memory fragmentation avoidance patch, so this might be a stupid question. That notwithstanding, do you really need two flags, one KERN and one USER?

There are two GFP flags to determine three types of allocation

__GFP_USERRCLM => Allocation is a user page or a disk buffer page __GFP_KERNRCLM => Kernel reclaimable allocation or one that is short-lived

Neither flag set implies a normal kernel allocation that is not expected to be reclaimed.

Joel, when merging the patches, there is one hack you need to watch out for. It is important for performance reasons but it is 100% obvious either.

gfp_flags & (__GFP_KERNRCLM | __GFP_USERRCLM) >> __GFP_TYPE_SHIFT gives the type of allocation as ALLOC_USERRCLM, ALLOC_KERNRCLM, ALLOC_KERNNORCLM or ALLOC_FALLBACK. Th ALLOC_* values are used to index into the array of freelists in the struct zone.

If the USER flag is shared, it means that your patches will be adding the __GFP_KERNRCLM flag which reverses the values. This means you will also need to update the values of ALLOC_* to keep the __GFP_TYPE_SHIFT hack working. Older patches used a switch statement on the flags but it takes a surprising length of time in a critical path.

Signature · 8 lines
Mel Gorman
Part-time Phd Student                          Java Applications Developer
University of Limerick                         IBM Dublin Software Lab
--
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>
#7 lore raw
Quoted 3 lines · Mel

Joel, when merging the patches, there is one hack you need to watch out for. It is important for performance reasons but it is 100% obvious either.

I suspect you meant "it is _not_ 100% obvious" ...

Is there someway that the gfp.h changes could be reworked to make it 100% obvious that these two bits are not separate and independent bits, but rather are a two bit field, counting an index from 0 to 3?

Signature · 8 lines
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <[email protected]> 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>
#8 lore raw
Quoted 6 lines · Paul Jackson
Quoted 3 lines · Mel

Joel, when merging the patches, there is one hack you need to watch out for. It is important for performance reasons but it is 100% obvious either.

I suspect you meant "it is _not_ 100% obvious" ...

Sorry, yes, it is not 100% obvious

Quoted 4 lines

Is there someway that the gfp.h changes could be reworked to make it 100% obvious that these two bits are not separate and independent bits, but rather are a two bit field, counting an index from 0 to 3?

Well, what would people feel is obvious? I will always think it is clear as I am the source of the confusion.

The two flags are not a two bit field as such, they just get treated as that to save a few cycles. It could also be done with something like;

index = (!!(gfp_flags & __GFP_KERNRCLM) << 1) || (!!(gfp_flags & __GFP_USERRCLM))

(untested) which means if the bits change position or value, the code won't care. This is a slightly more expensive, but possibly clearer way to do things.

Signature · 8 lines
Mel Gorman
Part-time Phd Student                          Java Applications Developer
University of Limerick                         IBM Dublin Software Lab
--
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>
#9 lore raw
Quoted 1 line · Mel

Well, what would people feel is obvious?

The lines that you (Mel) add that I am puzzling over ways to clarify are these added lines in gfp.h:

    +#define __GFP_KERNRCLM  0x20000u  /* Kernel page that is easily reclaimable */
    +#define __GFP_USERRCLM  0x40000u  /* User is a userspace user */

+#define __GFP_TYPE_SHIFT 17 /* Translate RCLM flags to array index */

and perhaps these added lines in mmzone.h:

+/* Page allocations are divided into these types */ +#define ALLOC_TYPES 4
+#define ALLOC_KERNNORCLM 0
+#define ALLOC_KERNRCLM 1
+#define ALLOC_USERRCLM 2
+#define ALLOC_FALLBACK 3
+
+/* Number of bits required to encode the type */ +#define BITS_PER_ALLOC_TYPE 2

It didn't jump out at me, first pass, that these two GFP bits were a 2 bit field, not 2 separate and independent bits. The name GFP_TYPE_SHIFT is vague. There are some redundant (interdependent) defines here.

How about (just brainstorming here) something like the following:

    #define __GFP_RCLM_BITS 0x60000u	/* page reclaim types: see RCLM_* defines */

    /*
     * Reduce buddy heap fragmentation by keeping pages with similar
     * reclaimability behavior together.  The two bit field __GFP_RECLAIMBITS
     * enumerates the following 4 kinds of page reclaimability:
     */
    #define RCLM_NONRECLAIMABLE 0	/* nonreclaimable kernel pages */
    #define RCLM_KERNEL 1		/* reclaimable kernel pages */
    #define RCLM_USER 2			/* reclaimable user pages */
    #define RCLM_FALLBACK 3		/* mark alloc requests when memory low */

    #define RCLM_SHIFT 17		/* Shift __GFP_RECLAIMBITS to RCLM_* values */
Signature · 8 lines
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <[email protected]> 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>
#10 lore raw
Quoted 23 lines · Paul Jackson
Quoted 1 line · Mel

Well, what would people feel is obvious?

The lines that you (Mel) add that I am puzzling over ways to clarify are these added lines in gfp.h:

    +#define __GFP_KERNRCLM  0x20000u  /* Kernel page that is easily reclaimable */
    +#define __GFP_USERRCLM  0x40000u  /* User is a userspace user */

+#define __GFP_TYPE_SHIFT 17 /* Translate RCLM flags to array index */

and perhaps these added lines in mmzone.h:

+/* Page allocations are divided into these types */ +#define ALLOC_TYPES 4
+#define ALLOC_KERNNORCLM 0
+#define ALLOC_KERNRCLM 1
+#define ALLOC_USERRCLM 2
+#define ALLOC_FALLBACK 3
+
+/* Number of bits required to encode the type */ +#define BITS_PER_ALLOC_TYPE 2

Quoted 7 lines

It didn't jump out at me, first pass, that these two GFP bits were a 2 bit field, not 2 separate and independent bits. The name GFP_TYPE_SHIFT is vague. There are some redundant (interdependent) defines here.

How about (just brainstorming here) something like the following:

That makes sense to me. Taking into account other threads, attached are patches 01 and 02 from Joels patchset with the different namings and comments. The main changes are the renaming of __GFP_USERRCLM to __GFP_USER to be neutral and comments explaining how the RCLM flags are tied together.

Is this any better?

fs/buffer.c +4 -2
diff -rup -X /usr/src/patchset-0.5/bin//dontdiff linux-2.6.13-rc3-mhp1/fs/buffer.c linux-2.6.13-rc3-mhp1-01_gfp_flags/fs/buffer.c
@@ -1119,7 +1119,8 @@ grow_dev_page(struct block_device *bdev,
 	struct page *page;
 	struct buffer_head *bh;
 
-	page = find_or_create_page(inode->i_mapping, index, GFP_NOFS);
+	page = find_or_create_page(inode->i_mapping, index,
+				   GFP_NOFS | __GFP_USER);
 	if (!page)
 		return NULL;
 

@@ -3044,7 +3045,8 @@ static void recalc_bh_state(void)
 	
 struct buffer_head *alloc_buffer_head(unsigned int __nocast gfp_flags)
 {
-	struct buffer_head *ret = kmem_cache_alloc(bh_cachep, gfp_flags);
+	struct buffer_head *ret = kmem_cache_alloc(bh_cachep,
+						   gfp_flags|__GFP_KERNRCLM);
 	if (ret) {
 		preempt_disable();
 		__get_cpu_var(bh_accounting).nr++;
fs/dcache.c +1 -1
diff -rup -X /usr/src/patchset-0.5/bin//dontdiff linux-2.6.13-rc3-mhp1/fs/dcache.c linux-2.6.13-rc3-mhp1-01_gfp_flags/fs/dcache.c
@@ -719,7 +719,7 @@ struct dentry *d_alloc(struct dentry * p
 	struct dentry *dentry;
 	char *dname;
 
-	dentry = kmem_cache_alloc(dentry_cache, GFP_KERNEL); 
+	dentry = kmem_cache_alloc(dentry_cache, GFP_KERNEL|__GFP_KERNRCLM);
 	if (!dentry)
 		return NULL;
fs/ext2/super.c +2 -1
diff -rup -X /usr/src/patchset-0.5/bin//dontdiff linux-2.6.13-rc3-mhp1/fs/ext2/super.c linux-2.6.13-rc3-mhp1-01_gfp_flags/fs/ext2/super.c
@@ -138,7 +138,8 @@ static kmem_cache_t * ext2_inode_cachep;
 static struct inode *ext2_alloc_inode(struct super_block *sb)
 {
 	struct ext2_inode_info *ei;
-	ei = (struct ext2_inode_info *)kmem_cache_alloc(ext2_inode_cachep, SLAB_KERNEL);
+	ei = (struct ext2_inode_info *)kmem_cache_alloc(ext2_inode_cachep,
+						SLAB_KERNEL|__GFP_KERNRCLM);
 	if (!ei)
 		return NULL;
 #ifdef CONFIG_EXT2_FS_POSIX_ACL
fs/ext3/super.c +1 -1
diff -rup -X /usr/src/patchset-0.5/bin//dontdiff linux-2.6.13-rc3-mhp1/fs/ext3/super.c linux-2.6.13-rc3-mhp1-01_gfp_flags/fs/ext3/super.c
@@ -440,7 +440,7 @@ static struct inode *ext3_alloc_inode(st
 {
 	struct ext3_inode_info *ei;
 
-	ei = kmem_cache_alloc(ext3_inode_cachep, SLAB_NOFS);
+	ei = kmem_cache_alloc(ext3_inode_cachep, SLAB_NOFS|__GFP_KERNRCLM);
 	if (!ei)
 		return NULL;
 #ifdef CONFIG_EXT3_FS_POSIX_ACL
fs/ntfs/inode.c +3 -2
diff -rup -X /usr/src/patchset-0.5/bin//dontdiff linux-2.6.13-rc3-mhp1/fs/ntfs/inode.c linux-2.6.13-rc3-mhp1-01_gfp_flags/fs/ntfs/inode.c
@@ -318,7 +318,7 @@ struct inode *ntfs_alloc_big_inode(struc
 
 	ntfs_debug("Entering.");
 	ni = (ntfs_inode *)kmem_cache_alloc(ntfs_big_inode_cache,
-			SLAB_NOFS);
+					    SLAB_NOFS|__GFP_KERNRCLM);
 	if (likely(ni != NULL)) {
 		ni->state = 0;
 		return VFS_I(ni);

@@ -343,7 +343,8 @@ static inline ntfs_inode *ntfs_alloc_ext
 	ntfs_inode *ni;
 
 	ntfs_debug("Entering.");
-	ni = (ntfs_inode *)kmem_cache_alloc(ntfs_inode_cache, SLAB_NOFS);
+	ni = (ntfs_inode *)kmem_cache_alloc(ntfs_inode_cache,
+					    SLAB_NOFS|__GFP_KERNRCLM);
 	if (likely(ni != NULL)) {
 		ni->state = 0;
 		return ni;
include/linux/gfp.h +20 -4
diff -rup -X /usr/src/patchset-0.5/bin//dontdiff linux-2.6.13-rc3-mhp1/include/linux/gfp.h linux-2.6.13-rc3-mhp1-01_gfp_flags/include/linux/gfp.h
@@ -41,21 +41,37 @@ struct vm_area_struct;
 #define __GFP_NOMEMALLOC 0x10000u /* Don't use emergency reserves */
 #define __GFP_NORECLAIM  0x20000u /* No realy zone reclaim during allocation */
 
-#define __GFP_BITS_SHIFT 20	/* Room for 20 __GFP_FOO bits */
+/*
+ * Allocation type modifiers - Group the allocation types together if possible
+ *
+ * __GFP_USER: Allocation for a user page or a buffer page.
+ *
+ * __GFP_KERNRCLM: Kernel allocation that is either very short-lived or 
+ * 		reclaimable like inode caches
+ *
+ * __GFP_RCLM_BITS: Sum of all the reclaimable bits.
+ */
+				     
+#define __GFP_USER     0x40000u /* Easily reclaimable userspace page */
+#define __GFP_KERNRCLM 0x80000u /* Kernel page that is easily reclaimable */
+#define __GFP_RCLM_BITS (__GFP_USER|__GFP_KERNRCLM) 
+
+#define __GFP_BITS_SHIFT 21	/* Room for 21 __GFP_FOO bits */
 #define __GFP_BITS_MASK ((1 << __GFP_BITS_SHIFT) - 1)
 
 /* if you forget to add the bitmask here kernel will crash, period */
 #define GFP_LEVEL_MASK (__GFP_WAIT|__GFP_HIGH|__GFP_IO|__GFP_FS| \
 			__GFP_COLD|__GFP_NOWARN|__GFP_REPEAT| \
 			__GFP_NOFAIL|__GFP_NORETRY|__GFP_NO_GROW|__GFP_COMP| \
-			__GFP_NOMEMALLOC|__GFP_NORECLAIM)
+			__GFP_NOMEMALLOC|__GFP_KERNRCLM|__GFP_USER)
 
 #define GFP_ATOMIC	(__GFP_HIGH)
 #define GFP_NOIO	(__GFP_WAIT)
 #define GFP_NOFS	(__GFP_WAIT | __GFP_IO)
 #define GFP_KERNEL	(__GFP_WAIT | __GFP_IO | __GFP_FS)
-#define GFP_USER	(__GFP_WAIT | __GFP_IO | __GFP_FS)
-#define GFP_HIGHUSER	(__GFP_WAIT | __GFP_IO | __GFP_FS | __GFP_HIGHMEM)
+#define GFP_USER	(__GFP_WAIT | __GFP_IO | __GFP_FS | __GFP_USER)
+#define GFP_HIGHUSER	(__GFP_WAIT | __GFP_IO | __GFP_FS | __GFP_HIGHMEM | \
+			 __GFP_USER)
 
 /* Flag - indicates that the buffer will be suitable for DMA.  Ignored on some
    platforms, used as appropriate on others */
include/linux/mmzone.h +90 -1

134 lines not on this page · raw message

mm/page_alloc.c +14 -0
diff -rup -X /usr/src/patchset-0.5/bin//dontdiff linux-2.6.13-rc3-mhp1-01_gfp_flags/mm/page_alloc.c linux-2.6.13-rc3-mhp1-02_more_defines/mm/page_alloc.c
@@ -65,6 +65,20 @@ EXPORT_SYMBOL(totalram_pages);
 EXPORT_SYMBOL(nr_swap_pages);
 
 /*
+ * fallback_allocs contains the fallback types for low memory conditions
+ * where the preferred alloction type if not available.
+ */
+int fallback_allocs[RCLM_TYPES][RCLM_TYPES+1] = {
+	{RCLM_NORCLM,    RCLM_FALLBACK,  RCLM_KERN,   RCLM_USER, -1},
+	{RCLM_KERN,      RCLM_FALLBACK,  RCLM_NORCLM, RCLM_USER, -1},
+	{RCLM_USER,      RCLM_FALLBACK,  RCLM_NORCLM, RCLM_KERN, -1},
+	{RCLM_FALLBACK,  RCLM_NORCLM,    RCLM_KERN,   RCLM_USER, -1}
+};
+static char *type_names[RCLM_TYPES] = { "Kernnel Unreclaimable",
+					 "Kernel Reclaimable",
+					 "User Reclaimable", "Fallback"};
+
+/*
  * Used by page_zone() to look up the address of the struct zone whose
  * id is encoded in the upper bits of page->flags
  */
#12 lore raw
In reply to Mel Gorman
Quoted 5 lines · Mel

That makes sense to me. Taking into account other threads, attached are patches 01 and 02 from Joels patchset with the different namings and comments. The main changes are the renaming of __GFP_USERRCLM to __GFP_USER to be neutral and comments explaining how the RCLM flags are tied together.

Ok - gaining.

One thing still confuses me here. What would it mean to have a gfp flag with both (__GFP_USER|__GFP_KERNRCLM) bits set? Is this a valid gfp flag, or is it just in pfn's that both bits can be set (meaning FALLBACK)?

If both bits can be set at the same time in a gfp flag, then I don't think either of the following two comments are accurate:

+#define __GFP_USER 0x40000u /* Easily reclaimable userspace page */
+#define __GFP_KERNRCLM 0x80000u /* Kernel page that is easily reclaimable */

Just looking at the GFP_USER bit and seeing it is set doesn't tell me for sure it's a userspace page request. It might be a reclaimable kernel page that we had to fallback on, right? Similarly for the __GFP_KERNRCLM bit.

And if both bits can be set in a gfp flag at the same time, then the test that _I_ need, for my two flavors of cpuset allocation is not possible, because I need to distinguish FALLBACK allocations for USER space requests from FALLBACK allocations for KERNEL space requests (USER space memory placement is confined more tightly).

Continuing this line of inquiry, what does it mean if neither bit is set in a gfp flag? I guess that's a valid gfp flag, and it means that the request is for non-reclaimable kernel memory. Is that right? If so, fine and this detail doesn't impact my intended use.

But the overloading of both bits set to mean FALLBACK, in the gfp flag, if that's what you intend here, does seem to make the apparent flagging userspace requests useless to my purposes, because I want to treat userspace FALLBACK requests differently than kernelspace FALLBACKs. For me, they are still userspace and kernel space. For you, they are both FALLBACKs. If my train of thought here hasn't gone off the rails, this would mean that I would still need my own GFP USER flag, and that I would encourage you to reinstate the RCLM tag on your __GFP_USER* flag, to distinguish it from mine. That, or perhaps it works to _not_ encode the fallback case in the gfp flags using the USER|KERN bits both set, but rather have a separate bit for the FALLBACK case. I can appreciate that in pfn's you have to encode this tightly for performance, but I'd be surprised if you have to do so in gfp flags for performance.

And in any case, the assymmetry of the __GFP_USER and __GFP_KERNRCLM names is a wart - one gets the RCLM tag and one doesn't. And the comment above for __GFP_USER still reflects solely the reclaim use of this bit, not a more neutral use.

... Please don't send patches as base64 encodings of carriage return terminated lines. Patches should be plain text inline, or at most plain text attachments. In either case, they should have newline terminated lines.

Signature · 8 lines
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <[email protected]> 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>
#13 lore raw
Quoted 7 lines · Paul Jackson
Quoted 5 lines · Mel

That makes sense to me. Taking into account other threads, attached are patches 01 and 02 from Joels patchset with the different namings and comments. The main changes are the renaming of __GFP_USERRCLM to __GFP_USER to be neutral and comments explaining how the RCLM flags are tied together.

Quoted 7 lines

Ok - gaining.

One thing still confuses me here. What would it mean to have a gfp flag with both (__GFP_USER|__GFP_KERNRCLM) bits set? Is this a valid gfp flag, or is it just in pfn's that both bits can be set (meaning FALLBACK)?

I would consider that combination a bug because it does not make sense. The side-effect is that the allocation type starts as a fallback which will work, but does not make sense.

Quoted 3 lines

If both bits can be set at the same time in a gfp flag, then I don't think either of the following two comments are accurate:

They can't, but it is not enforced

Quoted 8 lines
include/linux/gfp.h +2 -0
+#define __GFP_USER     0x40000u /* Easily reclaimable userspace page */
+#define __GFP_KERNRCLM 0x80000u /* Kernel page that is easily reclaimable */

Just looking at the GFP_USER bit and seeing it is set doesn't tell me for sure it's a userspace page request. It might be a reclaimable kernel page that we had to fallback on, right? Similarly for the __GFP_KERNRCLM bit.

No, the bit is set by the caller. The caller should only have this bit set if the allocation is a userspace allocation that can be reclaimed. That said, I have since found that setting the __GFP_USERRCLM bit on GFP_USER and GFP_HIGHUSER is not the correct thing to do as GFP_USER and GFP_HIGHUSER can be for allocations that are not reclaimable.

Quoted 6 lines

And if both bits can be set in a gfp flag at the same time, then the test that _I_ need, for my two flavors of cpuset allocation is not possible, because I need to distinguish FALLBACK allocations for USER space requests from FALLBACK allocations for KERNEL space requests (USER space memory placement is confined more tightly).

If you have to be sure the two bits are not set, then a check can be made and BUG() called

Quoted 5 lines

Continuing this line of inquiry, what does it mean if neither bit is set in a gfp flag? I guess that's a valid gfp flag, and it means that the request is for non-reclaimable kernel memory. Is that right? If so, fine and this detail doesn't impact my intended use.

That is correct

Quoted 1 line

But the overloading of both bits set to mean FALLBACK,

The reason the fallback bits are needed at all is to flag regions of physical memory to be used for any time of allocation. i.e. Try to place allocations in the right place, but failing that, use a fallback region, failing that, use anywhere at all.

Quoted 9 lines

in the gfp
flag, if that's what you intend here, does seem to make the apparent flagging userspace requests useless to my purposes, because I want to treat userspace FALLBACK requests differently than kernelspace FALLBACKs. For me, they are still userspace and kernel space. For you, they are both FALLBACKs. If my train of thought here hasn't gone off the rails, this would mean that I would still need my own GFP USER flag, and that I would encourage you to reinstate the RCLM tag on your __GFP_USER* flag, to distinguish it from mine.

I am not convinced we need the separate flag yet. Does it make a difference that both flags should never be specified for an allocation?

We are using the same bit right now for different reasons. In our case, it determines where the page, in physical memory, the allocated page comes from. In your case, it determines if the page should be allocated at all

Quoted 6 lines

That, or perhaps it works to _not_ encode the fallback case in the gfp flags using the USER|KERN bits both set, but rather have a separate bit for the FALLBACK case. I can appreciate that in pfn's you have to encode this tightly for performance, but I'd be surprised if you have to do so in gfp flags for performance.

Bits are per 2^(MAX_ORDER-1) number of pages, not every page frame

--
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>