Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751228AbdGZJaA (ORCPT ); Wed, 26 Jul 2017 05:30:00 -0400 Received: from mail-yw0-f174.google.com ([209.85.161.174]:33510 "EHLO mail-yw0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750826AbdGZJ37 (ORCPT ); Wed, 26 Jul 2017 05:29:59 -0400 MIME-Version: 1.0 In-Reply-To: <20170726092411.GD15833@8bytes.org> References: <20170704135556.21704-1-tfiga@chromium.org> <20170704135556.21704-2-tfiga@chromium.org> <20170726092411.GD15833@8bytes.org> From: Tomasz Figa Date: Wed, 26 Jul 2017 18:29:37 +0900 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v2 2/2] iommu/dma: Use __GFP_NOWARN only for high-order allocations To: Joerg Roedel Cc: "open list:IOMMU DRIVERS" , "linux-kernel@vger.kernel.org" , Robin Murphy Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2350 Lines: 61 Hi Joerg, On Wed, Jul 26, 2017 at 6:24 PM, Joerg Roedel wrote: > On Tue, Jul 04, 2017 at 10:55:56PM +0900, Tomasz Figa wrote: >> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c >> index bf23989b5158..6ed8c8f941d8 100644 >> --- a/drivers/iommu/dma-iommu.c >> +++ b/drivers/iommu/dma-iommu.c >> @@ -433,6 +433,7 @@ static struct page **__iommu_dma_alloc_pages(unsigned int count, >> { >> struct page **pages; >> unsigned int i = 0, array_size = count * sizeof(*pages); >> + const gfp_t high_order_gfp = __GFP_NOWARN | __GFP_NORETRY; >> >> order_mask &= (2U << MAX_ORDER) - 1; >> if (!order_mask) >> @@ -452,8 +453,6 @@ static struct page **__iommu_dma_alloc_pages(unsigned int count, >> if (!(gfp & (__GFP_DMA | __GFP_DMA32))) >> gfp |= __GFP_HIGHMEM; >> >> - gfp |= __GFP_NOWARN; >> - > > Okay, so a warning should be there if allocation fails, independent of > what the allocation order is. The allocation fails only if the order drops to the lowest possible fallback order. > So either this function prints a message > in case of allocation failure or we remove __GFP_NOWARN for all > allocation attempts. > > Adding __GFP_NOWARN only makes sense when there is another fall-back in > case allocation fails anyway, which is not the case here. There is fall back here. The loop tries allocating with higher order and then falls back to a lower order if it fails and so on, until the lowest acceptable order is reached. > >> while (count) { >> struct page *page = NULL; >> unsigned int order_size; >> @@ -469,7 +468,7 @@ static struct page **__iommu_dma_alloc_pages(unsigned int count, >> >> order_size = 1U << order; >> page = alloc_pages((order_mask - order_size) ? >> - gfp | __GFP_NORETRY : gfp, order); >> + gfp | high_order_gfp : gfp, order); > > Why does it specify __GFP_NORETRY at all? The alloc-routines in the > DMA-API don't need to be atomic. This doesn't have anything to do with being atomic. __GFP_NORETRY here is to avoid the page allocator retrying indefinitely and actually triggering OOM for high order allocation, if we can safely fall back to lower order. Best regards, Tomasz