Thread (4 messages) 4 messages, 2 authors, 2014-01-24

Re: [PATCH] x86: dma-mapping: fix GFP_ATOMIC macro usage

flat view

From: Andy Whitcroft <apw@canonical.com>
Date: 2014-01-17 10:49:46
Also in: lkml

On Fri, Jan 17, 2014 at 8:46 AM, Marek Szyprowski
[off-list ref] wrote:
quoted hunk ↗ jump to hunk
GFP_ATOMIC is not a single gfp flag, but a macro which expands to the other
flags and LACK of __GFP_WAIT flag. To check if caller wanted to perform an
atomic allocation, the code must test __GFP_WAIT flag presence. This patch
fixes the issue introduced in v3.5-rc1

CC: stable@vger.kernel.org
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 arch/x86/kernel/pci-dma.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
index 872079a..32a81c9 100644
--- a/arch/x86/kernel/pci-dma.c
+++ b/arch/x86/kernel/pci-dma.c
@@ -100,7 +100,7 @@ void *dma_generic_alloc_coherent(struct device *dev, size_t size,
        flag |= __GFP_ZERO;
 again:
        page = NULL;
-       if (!(flag & GFP_ATOMIC))
+       if (flag & __GFP_WAIT)
From that description should this not actually be:
    if (!(flag & (GFP_ATOMIC|__GFP_WAIT) == GFP_ATOMIC))

Else we will start using this pool for more than __GFP_HIGH allocations?

That said, it is possible this is right and the intent was to allow
__GFP_HIGH allocations (in general) to use this contigious pool, but I
will let someone more intimate with the code comment to that.  I would
have hoped the code would have been as below in that case:

    if (!(flag & __GFP_HIGH))

Either way once this is resolved a nice comment should be added to
make it really clear:

-apw
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help