Re: can a kmalloc be both GFP_ATOMIC and GFP_KERNEL at the sametime?

[Jan Engelhardt]

On Apr 30 2007 04:46, Robert P. J. Day wrote:
>> >
>> >   i'd always assumed that the type flags of GFP_ATOMIC and GFP_KERNEL
>> > were mutually exclusive when it came to calling kmalloc(), at least
>> > based on everything i'd read.  so i'm not sure how to interpret the
>> > following:
>> >
>> > drivers/scsi/aic7xxx_old.c:  aic_dev = kmalloc(sizeof(struct aic_dev_data), GFP_ATOMIC | GFP_KERNEL);
>> > drivers/message/i2o/device.c:   resblk = kmalloc(buflen + 8, GFP_KERNEL | GFP_ATOMIC);
>> >
>> >   clarification?
>>
>> GFP_ATOMIC implies that the memory comes from the zones which
>> GFP_KERNEL also uses.  So the above usage of GFP_KERNEL is redundant
>> and should be removed.
>
>hang on ... based on an email i just got, is that reference to
>GFP_KERNEL "redundant" or "conflicting"?  big difference there.  and
>is the proper fix to remove "GFP_KERNEL" in both cases?

include/linux/gfp.h:
#define GFP_ATOMIC (__GFP_HIGH)
#define GFP_KERNEL (__GFP_WAIT | __GFP_IO | __GFP_FS)

So combining GFP_ATOMIC with GFP_KERNEL gives you
"allow io, allow fs, allow waiting, and use emergency pools when it's getting
tight"
which to me looks like a valid, but probably unwanted combination.
Perhaps this could be shuffled a little to make such combinations
less likely, by moving this around as follows:

Index: linux-2.6.21/include/linux/gfp.h
===================================================================
--- linux-2.6.21.orig/include/linux/gfp.h
+++ linux-2.6.21/include/linux/gfp.h
@@ -34,7 +34,7 @@ struct vm_area_struct;
  * __GFP_MOVABLE: Flag that this page will be movable by the page migration
  * mechanism or reclaimed
  */
-#define __GFP_WAIT	((__force gfp_t)0x10u)	/* Can wait and reschedule? */
+#define __GFP_NOWAIT	((__force gfp_t)0x10u)	/* Do not wait or reschedule */
 #define __GFP_HIGH	((__force gfp_t)0x20u)	/* Should access emergency pools? */
 #define __GFP_IO	((__force gfp_t)0x40u)	/* Can start physical IO? */
 #define __GFP_FS	((__force gfp_t)0x80u)	/* Can call down to low-level FS? */
@@ -68,14 +68,14 @@ struct vm_area_struct;
 /* This equals 0, but use constants in case they ever change */
 #define GFP_NOWAIT	(GFP_ATOMIC & ~__GFP_HIGH)
 /* GFP_ATOMIC means both !wait (__GFP_WAIT not set) and use emergency pool */
-#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 | __GFP_HARDWALL)
-#define GFP_HIGHUSER	(__GFP_WAIT | __GFP_IO | __GFP_FS | __GFP_HARDWALL | \
+#define GFP_ATOMIC	(__GFP_NOWAIT | __GFP_HIGH)
+#define GFP_NOIO	0
+#define GFP_NOFS	(__GFP_IO)
+#define GFP_KERNEL	(__GFP_IO | __GFP_FS)
+#define GFP_USER	(__GFP_IO | __GFP_FS | __GFP_HARDWALL)
+#define GFP_HIGHUSER	(__GFP_IO | __GFP_FS | __GFP_HARDWALL | \
 			 __GFP_HIGHMEM)
-#define GFP_HIGH_MOVABLE	(__GFP_WAIT | __GFP_IO | __GFP_FS | \
+#define GFP_HIGH_MOVABLE	(__GFP_IO | __GFP_FS | \
 				 __GFP_HARDWALL | __GFP_HIGHMEM | \
 				 __GFP_MOVABLE)
 

Then a line like

  if ((flags & (GFP_ATOMIC | GFP_KERNEL)) == (GFP_ATOMIC | GFP_KERNEL))
	BUG();

could work.

Might have more implications. (And definitely, some more kernel code
would need to be fixed up.)


Jan
-- 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/