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 [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
