Re: [PATCH] SCSI: Remove redundant GFP_KERNEL type flag in kmalloc().

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 5/4/07, Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
[...]
>
> -  aic_dev = kmalloc(sizeof(struct aic_dev_data), GFP_ATOMIC | GFP_KERNEL);
> +  aic_dev = kmalloc(sizeof(struct aic_dev_data), GFP_ATOMIC);

No, this converts the allocation from a robust one which can sleep into a
flakey one which cannot.

If we want to just clean this code up, we should switch to

        GFP_KERNEL|__GFP_HIGH

and add a comment explaining why we're turning on __GFP_HIGH (pointlessly,
I suspect).

However I suspect what the code really meant to do was to use just
GFP_KERNEL.

Yes.

A recap from http://lkml.org/lkml/2007/4/28/160 ...

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

and:

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.

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

It _was_ established towards the end of that thread that neither of the two
cases are atomic contexts. And it would still make sense to remove the redundant
__GFP_HIGH, as we don't want to unnecessarily deplete the emergency
reserve pool for normal GFP_KERNEL cases thus making ENOMEM on some future
kmalloc(GFP_ATOMIC) more likely. I'll send out the patches.
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]
  Powered by Linux