can a kmalloc be both GFP_ATOMIC and GFP_KERNEL at the same time?

can a kmalloc be both GFP_ATOMIC and GFP_KERNEL at the same time?

[Robert P. J. Day]

  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?

rday
-- 
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://fsdev.net/wiki/index.php?title=Main_Page
========================================================================

RE: can a kmalloc be both GFP_ATOMIC and GFP_KERNEL at the same time?

[Shan, Guo Wen (Gavin)]

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

-----Original Message-----
From: Robert P. J. Day [mailto:rpjday@mindspring.com]
Sent: Saturday, April 28, 2007 9:41 PM
To: Linux Kernel Mailing List
Subject: can a kmalloc be both GFP_ATOMIC and GFP_KERNEL at the same
time?



  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?

rday
-- 
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://fsdev.net/wiki/index.php?title=Main_Page
========================================================================
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

RE: can a kmalloc be both GFP_ATOMIC and GFP_KERNEL at the same time?

[Robert P. J. Day]

On Sat, 28 Apr 2007, Shan, Guo Wen (Gavin) wrote:

>
> #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)
>
> -----Original Message-----
> From: Robert P. J. Day [mailto:rpjday@mindspring.com]
> Sent: Saturday, April 28, 2007 9:41 PM
> To: Linux Kernel Mailing List
> Subject: can a kmalloc be both GFP_ATOMIC and GFP_KERNEL at the same
> time?
>
>
>
>   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?

oh, i'm *aware* of the definitions of those flags, but every single
source i've ever read has *strongly* suggested that you don't use
those two flags together so i was surprised to see those combinations.
(as an example, love's kernel book, p. 192, shows a table of valid
combinations of flags to use, but doesn't mention the one above.)

and, on the other hand, if they *are* legal to use together, i guess
i'm kind of surprised that there would be only two instances of it.

in any case, i'll just assume that the above is valid and i'll go back
to reading up on it until i convince myself.  thanks.

rday

-- 
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://fsdev.net/wiki/index.php?title=Main_Page
========================================================================

RE: can a kmalloc be both GFP_ATOMIC and GFP_KERNEL at the same time?

[Arjan van de Ven]

> >
> >
> >   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?
> 
> oh, i'm *aware* of the definitions of those flags, but every single
> source i've ever read has *strongly* suggested that you don't use
> those two flags together so i was surprised to see those combinations.
> (as an example, love's kernel book, p. 192, shows a table of valid
> combinations of flags to use, but doesn't mention the one above.)
> 
> and, on the other hand, if they *are* legal to use together, i guess
> i'm kind of surprised that there would be only two instances of it.

it's not legal to use the combo; you have found yourself a very genuine
bug here! Good spotting!

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

[WANG Cong]

On Sat, Apr 28, 2007 at 08:03:42AM -0700, Arjan van de Ven 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?
>> 
>> oh, i'm *aware* of the definitions of those flags, but every single
>> source i've ever read has *strongly* suggested that you don't use
>> those two flags together so i was surprised to see those combinations.
>> (as an example, love's kernel book, p. 192, shows a table of valid
>> combinations of flags to use, but doesn't mention the one above.)
>> 
>> and, on the other hand, if they *are* legal to use together, i guess
>> i'm kind of surprised that there would be only two instances of it.
>
>it's not legal to use the combo; you have found yourself a very genuine
>bug here! Good spotting!

Yes. LDD already talked about this. GFP_KERNEL may cause sleeping while GFP_ATOMIC not. Combining them is confusing. 

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

[Alan Cox]

> > 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?
> 
> oh, i'm *aware* of the definitions of those flags, but every single
> source i've ever read has *strongly* suggested that you don't use

I don't believe either of those is doing what the author intended 8).

Alan

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

[Christoph Lameter]

On Sat, 28 Apr 2007, Alan Cox wrote:

> > > 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?
> > 
> > oh, i'm *aware* of the definitions of those flags, but every single
> > source i've ever read has *strongly* suggested that you don't use
> 
> I don't believe either of those is doing what the author intended 8).

Well sortof. First of all kmalloc may not even allocate a page to 
satisfy the request. If it does then we do essentially GFP_KERNEL | 
__GFP_HIGH meaning a allocation where we can sleep etc but are allowed to 
dip into the reserves.

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

[Andrew Morton]

On Sat, 28 Apr 2007 09:40:39 -0400 (EDT) "Robert P. J. Day" <rpjday@mindspring.com> 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.

The way to understand this is to look at the definitions:

#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 | \
			 __GFP_HIGHMEM)


GFP_KERNEL: can sleep, can do IO, can enter filesystems

GFP_ATOMIC: cannot sleep, cannot do IO, cannot enter filesystems

Neither of them contains __GFP_DMA or __GFP_HIGHMEM, so both of them refer
to ZONE_NORMAL and any lower zones (ie: ZONE_DMA)

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

[Robert P. J. Day]

On Mon, 30 Apr 2007, Andrew Morton wrote:

> On Sat, 28 Apr 2007 09:40:39 -0400 (EDT) "Robert P. J. Day" <rpjday@mindspring.com> 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?

rday
-- 
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://fsdev.net/wiki/index.php?title=Main_Page
========================================================================

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

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

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

[Robert P. J. Day]

On Mon, 30 Apr 2007, Jan Engelhardt wrote:

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

... snip ...

at this point, maybe i'll just leave this in the hands of those who
know far more about it than i do.  but, while we're here, are there
any *other* combinations that wouldn't make any sense?  might as well
check for those in my cleanup script as well.

rday

p.s.  as a suggestion that borders on overkill, one could always add a
configuration debugging option that, when set, compiles code into
kmalloc() that does a sanity check on its type flag arguments.

-- 
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://fsdev.net/wiki/index.php?title=Main_Page
========================================================================

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

[Satyam Sharma]

On 4/30/07, Robert P. J. Day <rpjday@mindspring.com> wrote:
> On Mon, 30 Apr 2007, Jan Engelhardt wrote:
> >
> > >> > 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.
> >
> > 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.

Yes. This all appears to be a case of some unfortunate naming used
here. GFP_KERNEL (== GFP_that_can_sleep, as defined currently) clearly
*cannot* go with GFP_ATOMIC (== GFP_that_cannot_sleep, for atomic
contexts, which is why GFP_ATOMIC exists). But the way these are
defined presently means that their combination is *not* invalid
(although of dubious usability).

As a matter of style, the author there could've written (GFP_KERNEL |
__GFP_HIGH) instead, but I'm not sure how much sense that makes
because once we specify GFP_KERNEL, we essentially bring out the heavy
weaponry to *make* some free space for ourselves if it isn't there
already (and we're prepared to sleep when all that happens) -- so
where's the need left to scavenge into emergency pools anymore?
__GFP_HIGH only makes sense for poor atomic contexts for whom sleeping
(when we try_to_free other pages to satisfy our allocation request) is
not an option, which is precisely how things are presently.

> at this point, maybe i'll just leave this in the hands of those who
> know far more about it than i do.  but, while we're here, are there
> any *other* combinations that wouldn't make any sense?  might as well
> check for those in my cleanup script as well.

What combinations make sense for a particular user must be left to him
and his usage context. So although (GFP_KERNEL | GFP_ATOMIC) does not
make sense in the way these are generally used (or were invented for),
but the combination *as defined presently* is not "invalid". I'm not
sure whether we really need to bother with putting in any checks in
__alloc_pages() -- this is only nonsensical usage at worst, not a bug.

> p.s.  as a suggestion that borders on overkill, one could always add a
> configuration debugging option that, when set, compiles code into
> kmalloc() that does a sanity check on its type flag arguments.

Eek.

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

Not necessarily. I haven't looked around that code you mentioned, but
the solution is pretty simple:

1. If you're in_atomic() context, that GFP_KERNEL is a BUG and *must*
be removed.

2. If not, that GFP_ATOMIC is redundant / nonsensical and *can* be removed.

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

[Andrew Morton]

On Mon, 30 Apr 2007 04:46:54 -0400 (EDT) "Robert P. J. Day" <rpjday@mindspring.com> wrote:

> On Mon, 30 Apr 2007, Andrew Morton wrote:
> 
> > On Sat, 28 Apr 2007 09:40:39 -0400 (EDT) "Robert P. J. Day" <rpjday@mindspring.com> 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?
> 

umm, yeah, oops.  GFP_KERNEL|GFP_ATOMIC is not a redundant combination. 
It's GFP_KERNEL plus "is able to access emergency pools".  We'd normally
represent that as GFP_KERNEL|__GFP_HIGH.

However it's questionable whether that was the intent in those two drivers.
`git-blame' might tell.

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

[Alan Cox]

> > > > 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?
> > 
> 
> umm, yeah, oops.  GFP_KERNEL|GFP_ATOMIC is not a redundant combination. 
> It's GFP_KERNEL plus "is able to access emergency pools".  We'd normally
> represent that as GFP_KERNEL|__GFP_HIGH.

i2o/device.c should be GFP_KERNEL as far as I can tell. It was meant to
be that way and the callers appear to all be calling it in sleep capable
contexts.

aic7xxx_old.c should probably be GFP_KERNEL as ->slave_alloc methods
appear to be able to sleep (although some drivers use GFP_ATOMIC here and
some GFP_KERNEL).

Alan

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

[Satyam Sharma]

On 4/30/07, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> i2o/device.c should be GFP_KERNEL as far as I can tell. It was meant to
> be that way and the callers appear to all be calling it in sleep capable
> contexts.
>
> aic7xxx_old.c should probably be GFP_KERNEL as ->slave_alloc methods
> appear to be able to sleep (although some drivers use GFP_ATOMIC here and
> some GFP_KERNEL).

Yes, none of the above appear to be atomic contexts. GFP_KERNEL in
that case would've been a bug. If they were atomic contexts, someone
somewhere would've been seeing a lot of "BUG: sleeping function called
from invalid context" messages and would've probably brought it to
lkml's notice already ;-) So the GFP_ATOMIC seems to be redundant
thing here in both cases.

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

[Stefan Richter]

Satyam Sharma wrote:
> On 4/30/07, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
>> i2o/device.c should be GFP_KERNEL as far as I can tell. It was meant to
>> be that way and the callers appear to all be calling it in sleep capable
>> contexts.
>>
>> aic7xxx_old.c should probably be GFP_KERNEL as ->slave_alloc methods
>> appear to be able to sleep (although some drivers use GFP_ATOMIC here and
>> some GFP_KERNEL).
> 
> Yes, none of the above appear to be atomic contexts. GFP_KERNEL in
> that case would've been a bug. If they were atomic contexts, someone
> somewhere would've been seeing a lot of "BUG: sleeping function called
> from invalid context" messages and would've probably brought it to
> lkml's notice already ;-) So the GFP_ATOMIC seems to be redundant
> thing here in both cases.

Documentation/scsi/scsi_mid_low_api.txt agrees that ->slave_alloc is
allowed to sleep.
-- 
Stefan Richter
-=====-=-=== -=-= ----=
http://arcgraph.de/sr/