[PATCH] swiotlb: suppress warning when __GFP_NOWARN is set v5

[PATCH] swiotlb: suppress warning when __GFP_NOWARN is set v5

[Christian König]

TTM tries to allocate coherent memory in chunks of 2MB first to improve
TLB efficiency and falls back to allocating 4K pages if that fails.

Suppress the warning when the 2MB allocations fails since there is a
valid fall back path.

v2: suppress warnings from swiotlb_tbl_map_single as well
v3: coding style fixes as suggested by Konrad
v4: make tbl_map_single static
v5: use DMA_ATTR_NO_WARN instead

Signed-off-by: Christian König <christian.koenig@amd.com>
Reported-by: Mike Galbraith <efault@gmx.de>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=104082
CC: stable@vger.kernel.org
---
 lib/swiotlb.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index cea19aaf303c..0d7f46fb993a 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -586,7 +586,7 @@ phys_addr_t swiotlb_tbl_map_single(struct device *hwdev,
 
 not_found:
 	spin_unlock_irqrestore(&io_tlb_lock, flags);
-	if (printk_ratelimit())
+	if (!(attrs & DMA_ATTR_NO_WARN) && printk_ratelimit())
 		dev_warn(hwdev, "swiotlb buffer is full (sz: %zd bytes)\n", size);
 	return SWIOTLB_MAP_ERROR;
 found:
@@ -713,6 +713,7 @@ void *
 swiotlb_alloc_coherent(struct device *hwdev, size_t size,
 		       dma_addr_t *dma_handle, gfp_t flags)
 {
+	bool warn = !(flags & __GFP_NOWARN);
 	dma_addr_t dev_addr;
 	void *ret;
 	int order = get_order(size);
@@ -738,8 +739,8 @@ swiotlb_alloc_coherent(struct device *hwdev, size_t size,
 		 * GFP_DMA memory; fall back on map_single(), which
 		 * will grab memory from the lowest available address range.
 		 */
-		phys_addr_t paddr = map_single(hwdev, 0, size,
-					       DMA_FROM_DEVICE, 0);
+		phys_addr_t paddr = map_single(hwdev, 0, size, DMA_FROM_DEVICE,
+					       warn ? 0 : DMA_ATTR_NO_WARN);
 		if (paddr == SWIOTLB_MAP_ERROR)
 			goto err_warn;
 
@@ -769,9 +770,11 @@ swiotlb_alloc_coherent(struct device *hwdev, size_t size,
 	return ret;
 
 err_warn:
-	pr_warn("swiotlb: coherent allocation failed for device %s size=%zu\n",
-		dev_name(hwdev), size);
-	dump_stack();
+	if (warn && printk_ratelimit()) {
+		pr_warn("swiotlb: coherent allocation failed for device %s size=%zu\n",
+			dev_name(hwdev), size);
+		dump_stack();
+	}
 
 	return NULL;
 }
-- 
2.11.0

Re: [PATCH] swiotlb: suppress warning when __GFP_NOWARN is set v5

[Christoph Hellwig]

> @@ -713,6 +713,7 @@ void *
>  swiotlb_alloc_coherent(struct device *hwdev, size_t size,
>  		       dma_addr_t *dma_handle, gfp_t flags)
>  {
> +	bool warn = !(flags & __GFP_NOWARN);

This is still wrong.  __GFP_NOWARN has no meaning for DMA coherent
allocations, and if a driver expects it to do anything it's doing it
wrong.  This needs to check for DMA_ATTR_NO_WARN, and thus get the attrs
passed.

Re: [PATCH] swiotlb: suppress warning when __GFP_NOWARN is set v5

[Christian König]

Am 04.01.2018 um 14:29 schrieb Christoph Hellwig:
>> @@ -713,6 +713,7 @@ void *
>>   swiotlb_alloc_coherent(struct device *hwdev, size_t size,
>>   		       dma_addr_t *dma_handle, gfp_t flags)
>>   {
>> +	bool warn = !(flags & __GFP_NOWARN);
> This is still wrong.  __GFP_NOWARN has no meaning for DMA coherent
> allocations, and if a driver expects it to do anything it's doing it
> wrong.  This needs to check for DMA_ATTR_NO_WARN, and thus get the attrs
> passed.

I perfectly agree on that, but this is for stable kernel backports. 
Because of this I want to keep the footprint as low as possible.

When your patchset to clean that up lands for 4.16 I have no problem 
changing that.

But till then I think we should just work on suppress the warning for now.

Regards,
Christian.

Re: [PATCH] swiotlb: suppress warning when __GFP_NOWARN is set v5

[Christoph Hellwig]

On Thu, Jan 04, 2018 at 02:49:21PM +0100, Christian König wrote:
> I perfectly agree on that, but this is for stable kernel backports. Because
> of this I want to keep the footprint as low as possible.
> 
> When your patchset to clean that up lands for 4.16 I have no problem
> changing that.
> 
> But till then I think we should just work on suppress the warning for now.

Oh well.  If Konrad is fine with that I'll stick it in front of my
swiotlb series (the next post is going to be split) with a stable tag,
so everything is well integrated.

Re: [PATCH] swiotlb: suppress warning when __GFP_NOWARN is set v5

[Konrad Rzeszutek Wilk]

On Tue, Jan 09, 2018 at 11:47:55AM -0800, Christoph Hellwig wrote:
> On Thu, Jan 04, 2018 at 02:49:21PM +0100, Christian König wrote:
> > I perfectly agree on that, but this is for stable kernel backports. Because
> > of this I want to keep the footprint as low as possible.
> > 
> > When your patchset to clean that up lands for 4.16 I have no problem
> > changing that.
> > 
> > But till then I think we should just work on suppress the warning for now.
> 
> Oh well.  If Konrad is fine with that I'll stick it in front of my
> swiotlb series (the next post is going to be split) with a stable tag,
> so everything is well integrated.

Totally. Thanks for taking care of it - been slammed with x86 architecture craziness.

Re: [PATCH] swiotlb: suppress warning when __GFP_NOWARN is set v5

[Christoph Hellwig]

On Tue, Jan 09, 2018 at 03:04:17PM -0500, Konrad Rzeszutek Wilk wrote:
> Totally. Thanks for taking care of it - been slammed with x86 architecture craziness.

I'll take this as an Acked-by..

Re: [PATCH] swiotlb: suppress warning when __GFP_NOWARN is set v5

[Konrad Rzeszutek Wilk]

On Tue, Jan 09, 2018 at 01:12:51PM -0800, Christoph Hellwig wrote:
> On Tue, Jan 09, 2018 at 03:04:17PM -0500, Konrad Rzeszutek Wilk wrote:
> > Totally. Thanks for taking care of it - been slammed with x86 architecture craziness.
> 
> I'll take this as an Acked-by..
> 

Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>