All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RESEND] ARM: DMA: Fix kzalloc flags in __iommu_alloc_buffer()
@ 2015-02-11  9:01 ` Alexandre Courbot
  0 siblings, 0 replies; 6+ messages in thread
From: Alexandre Courbot @ 2015-02-11  9:01 UTC (permalink / raw)
  To: linux-arm-kernel

There doesn't seem to be any valid reason to allocate the pages array
with the same flags as the buffer itself. Doing so can eventually lead
to the following safeguard in mm/slab.c to be hit:

BUG_ON(flags & GFP_SLAB_BUG_MASK);

This happens when buffers are allocated with __GFP_DMA32 or
__GFP_HIGHMEM.

Fix this by allocating the pages array with GFP_KERNEL to follow what is
done elsewhere in this file. Using GFP_KERNEL in __iommu_alloc_buffer()
is safe because atomic allocations are handled by __iommu_alloc_atomic().

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Russell King <linux@arm.linux.org.uk>
Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 arch/arm/mm/dma-mapping.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 903dba0..170a116 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -1106,7 +1106,7 @@ static struct page **__iommu_alloc_buffer(struct device *dev, size_t size,
 	int i = 0;
 
 	if (array_size <= PAGE_SIZE)
-		pages = kzalloc(array_size, gfp);
+		pages = kzalloc(array_size, GFP_KERNEL);
 	else
 		pages = vzalloc(array_size);
 	if (!pages)
-- 
2.3.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH RESEND] ARM: DMA: Fix kzalloc flags in __iommu_alloc_buffer()
@ 2015-02-11  9:01 ` Alexandre Courbot
  0 siblings, 0 replies; 6+ messages in thread
From: Alexandre Courbot @ 2015-02-11  9:01 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Russell King, Marek Szyprowski, Will Deacon, linux-arm-kernel,
	linux-kernel, gnurou, Alexandre Courbot

There doesn't seem to be any valid reason to allocate the pages array
with the same flags as the buffer itself. Doing so can eventually lead
to the following safeguard in mm/slab.c to be hit:

BUG_ON(flags & GFP_SLAB_BUG_MASK);

This happens when buffers are allocated with __GFP_DMA32 or
__GFP_HIGHMEM.

Fix this by allocating the pages array with GFP_KERNEL to follow what is
done elsewhere in this file. Using GFP_KERNEL in __iommu_alloc_buffer()
is safe because atomic allocations are handled by __iommu_alloc_atomic().

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Russell King <linux@arm.linux.org.uk>
Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 arch/arm/mm/dma-mapping.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 903dba0..170a116 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -1106,7 +1106,7 @@ static struct page **__iommu_alloc_buffer(struct device *dev, size_t size,
 	int i = 0;
 
 	if (array_size <= PAGE_SIZE)
-		pages = kzalloc(array_size, gfp);
+		pages = kzalloc(array_size, GFP_KERNEL);
 	else
 		pages = vzalloc(array_size);
 	if (!pages)
-- 
2.3.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH RESEND] ARM: DMA: Fix kzalloc flags in __iommu_alloc_buffer()
  2015-02-11  9:01 ` Alexandre Courbot
@ 2015-02-13  3:32   ` Will Deacon
  -1 siblings, 0 replies; 6+ messages in thread
From: Will Deacon @ 2015-02-13  3:32 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Feb 11, 2015 at 09:01:41AM +0000, Alexandre Courbot wrote:
> There doesn't seem to be any valid reason to allocate the pages array
> with the same flags as the buffer itself. Doing so can eventually lead
> to the following safeguard in mm/slab.c to be hit:
> 
> BUG_ON(flags & GFP_SLAB_BUG_MASK);

nit: I can't actually spot this BUG_ON in the kernel.

> This happens when buffers are allocated with __GFP_DMA32 or
> __GFP_HIGHMEM.
> 
> Fix this by allocating the pages array with GFP_KERNEL to follow what is
> done elsewhere in this file. Using GFP_KERNEL in __iommu_alloc_buffer()
> is safe because atomic allocations are handled by __iommu_alloc_atomic().
> 
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: Russell King <linux@arm.linux.org.uk>
> Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  arch/arm/mm/dma-mapping.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> index 903dba0..170a116 100644
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -1106,7 +1106,7 @@ static struct page **__iommu_alloc_buffer(struct device *dev, size_t size,
>  	int i = 0;
>  
>  	if (array_size <= PAGE_SIZE)
> -		pages = kzalloc(array_size, gfp);
> +		pages = kzalloc(array_size, GFP_KERNEL);
>  	else
>  		pages = vzalloc(array_size);
>  	if (!pages)
> -- 
> 2.3.0

Looks sensible to me:

  Acked-by: Will Deacon <will.deacon@arm.com>

Will

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH RESEND] ARM: DMA: Fix kzalloc flags in __iommu_alloc_buffer()
@ 2015-02-13  3:32   ` Will Deacon
  0 siblings, 0 replies; 6+ messages in thread
From: Will Deacon @ 2015-02-13  3:32 UTC (permalink / raw)
  To: Alexandre Courbot
  Cc: Arnd Bergmann, Russell King, Marek Szyprowski,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, Alexandre Courbot

On Wed, Feb 11, 2015 at 09:01:41AM +0000, Alexandre Courbot wrote:
> There doesn't seem to be any valid reason to allocate the pages array
> with the same flags as the buffer itself. Doing so can eventually lead
> to the following safeguard in mm/slab.c to be hit:
> 
> BUG_ON(flags & GFP_SLAB_BUG_MASK);

nit: I can't actually spot this BUG_ON in the kernel.

> This happens when buffers are allocated with __GFP_DMA32 or
> __GFP_HIGHMEM.
> 
> Fix this by allocating the pages array with GFP_KERNEL to follow what is
> done elsewhere in this file. Using GFP_KERNEL in __iommu_alloc_buffer()
> is safe because atomic allocations are handled by __iommu_alloc_atomic().
> 
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: Russell King <linux@arm.linux.org.uk>
> Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  arch/arm/mm/dma-mapping.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> index 903dba0..170a116 100644
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -1106,7 +1106,7 @@ static struct page **__iommu_alloc_buffer(struct device *dev, size_t size,
>  	int i = 0;
>  
>  	if (array_size <= PAGE_SIZE)
> -		pages = kzalloc(array_size, gfp);
> +		pages = kzalloc(array_size, GFP_KERNEL);
>  	else
>  		pages = vzalloc(array_size);
>  	if (!pages)
> -- 
> 2.3.0

Looks sensible to me:

  Acked-by: Will Deacon <will.deacon@arm.com>

Will

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH RESEND] ARM: DMA: Fix kzalloc flags in __iommu_alloc_buffer()
  2015-02-13  3:32   ` Will Deacon
@ 2015-02-13  5:13     ` Alexandre Courbot
  -1 siblings, 0 replies; 6+ messages in thread
From: Alexandre Courbot @ 2015-02-13  5:13 UTC (permalink / raw)
  To: linux-arm-kernel

On 02/13/2015 12:32 PM, Will Deacon wrote:
> On Wed, Feb 11, 2015 at 09:01:41AM +0000, Alexandre Courbot wrote:
>> There doesn't seem to be any valid reason to allocate the pages array
>> with the same flags as the buffer itself. Doing so can eventually lead
>> to the following safeguard in mm/slab.c to be hit:
>>
>> BUG_ON(flags & GFP_SLAB_BUG_MASK);
>
> nit: I can't actually spot this BUG_ON in the kernel.

I have been trying to push this patch for so long that the line in 
question changed in the meantime. :) It is now

         if (unlikely(flags & GFP_SLAB_BUG_MASK)) {
                 pr_emerg("gfp: %u\n", flags & GFP_SLAB_BUG_MASK);
                 BUG();
         }

in cache_grow, line 2593 of mm/slab.c.

>
>> This happens when buffers are allocated with __GFP_DMA32 or
>> __GFP_HIGHMEM.
>>
>> Fix this by allocating the pages array with GFP_KERNEL to follow what is
>> done elsewhere in this file. Using GFP_KERNEL in __iommu_alloc_buffer()
>> is safe because atomic allocations are handled by __iommu_alloc_atomic().
>>
>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>> Cc: Arnd Bergmann <arnd@arndb.de>
>> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
>> Cc: Russell King <linux@arm.linux.org.uk>
>> Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>>   arch/arm/mm/dma-mapping.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
>> index 903dba0..170a116 100644
>> --- a/arch/arm/mm/dma-mapping.c
>> +++ b/arch/arm/mm/dma-mapping.c
>> @@ -1106,7 +1106,7 @@ static struct page **__iommu_alloc_buffer(struct device *dev, size_t size,
>>   	int i = 0;
>>
>>   	if (array_size <= PAGE_SIZE)
>> -		pages = kzalloc(array_size, gfp);
>> +		pages = kzalloc(array_size, GFP_KERNEL);
>>   	else
>>   		pages = vzalloc(array_size);
>>   	if (!pages)
>> --
>> 2.3.0
>
> Looks sensible to me:
>
>    Acked-by: Will Deacon <will.deacon@arm.com>

Thanks! I will amend the commit message and resend.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH RESEND] ARM: DMA: Fix kzalloc flags in __iommu_alloc_buffer()
@ 2015-02-13  5:13     ` Alexandre Courbot
  0 siblings, 0 replies; 6+ messages in thread
From: Alexandre Courbot @ 2015-02-13  5:13 UTC (permalink / raw)
  To: Will Deacon, Alexandre Courbot
  Cc: Arnd Bergmann, Russell King, Marek Szyprowski,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org

On 02/13/2015 12:32 PM, Will Deacon wrote:
> On Wed, Feb 11, 2015 at 09:01:41AM +0000, Alexandre Courbot wrote:
>> There doesn't seem to be any valid reason to allocate the pages array
>> with the same flags as the buffer itself. Doing so can eventually lead
>> to the following safeguard in mm/slab.c to be hit:
>>
>> BUG_ON(flags & GFP_SLAB_BUG_MASK);
>
> nit: I can't actually spot this BUG_ON in the kernel.

I have been trying to push this patch for so long that the line in 
question changed in the meantime. :) It is now

         if (unlikely(flags & GFP_SLAB_BUG_MASK)) {
                 pr_emerg("gfp: %u\n", flags & GFP_SLAB_BUG_MASK);
                 BUG();
         }

in cache_grow, line 2593 of mm/slab.c.

>
>> This happens when buffers are allocated with __GFP_DMA32 or
>> __GFP_HIGHMEM.
>>
>> Fix this by allocating the pages array with GFP_KERNEL to follow what is
>> done elsewhere in this file. Using GFP_KERNEL in __iommu_alloc_buffer()
>> is safe because atomic allocations are handled by __iommu_alloc_atomic().
>>
>> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
>> Cc: Arnd Bergmann <arnd@arndb.de>
>> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
>> Cc: Russell King <linux@arm.linux.org.uk>
>> Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>>   arch/arm/mm/dma-mapping.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
>> index 903dba0..170a116 100644
>> --- a/arch/arm/mm/dma-mapping.c
>> +++ b/arch/arm/mm/dma-mapping.c
>> @@ -1106,7 +1106,7 @@ static struct page **__iommu_alloc_buffer(struct device *dev, size_t size,
>>   	int i = 0;
>>
>>   	if (array_size <= PAGE_SIZE)
>> -		pages = kzalloc(array_size, gfp);
>> +		pages = kzalloc(array_size, GFP_KERNEL);
>>   	else
>>   		pages = vzalloc(array_size);
>>   	if (!pages)
>> --
>> 2.3.0
>
> Looks sensible to me:
>
>    Acked-by: Will Deacon <will.deacon@arm.com>

Thanks! I will amend the commit message and resend.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2015-02-13  5:13 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-02-11  9:01 [PATCH RESEND] ARM: DMA: Fix kzalloc flags in __iommu_alloc_buffer() Alexandre Courbot
2015-02-11  9:01 ` Alexandre Courbot
2015-02-13  3:32 ` Will Deacon
2015-02-13  3:32   ` Will Deacon
2015-02-13  5:13   ` Alexandre Courbot
2015-02-13  5:13     ` Alexandre Courbot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.