From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id D00AFC25B4E for ; Fri, 20 Jan 2023 19:28:38 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:Content-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:From:References:Cc:To:Subject: MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=QuO98okerz6hEd6Ury9kakiH+wDDiMpk5slb6KGnRd4=; b=drj4CoDWTjfZDj FIKp/tShILwYmq9qkkwKSdIIUaEnldSoA7bVh5CEsOUNDAGXQtlk3CGOvsZxEkFjyLV/s9GNfzm6T NtLjF6Lhrdf86wdPibngCrgbEj5g33cTnlw9SgW+bBQrw5EbD+q4LDQo8W0cKQTqMY0qQhE/5CXwk Kq7bCxCwccc8SxvtznaWYRg1HTGZHvDXsDI5aUlD4mvh8SW3/qKEjyVj/wffv3fVu3tT4f7FhzfHT 4HLYa44HUwvLtwyhAWemFRz5wSudRYjYiAnB8z2F4JjWxSYG7ru6FpTTjYGp6IwOuzLjgEBbL/OQt coodQiYbQSxysfZOkryA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1pIx3y-00C52N-51; Fri, 20 Jan 2023 19:28:34 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1pIx3u-00C50a-EB; Fri, 20 Jan 2023 19:28:31 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id CB11511FB; Fri, 20 Jan 2023 11:29:08 -0800 (PST) Received: from [10.57.89.132] (unknown [10.57.89.132]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id A6FF03F445; Fri, 20 Jan 2023 11:28:23 -0800 (PST) Message-ID: Date: Fri, 20 Jan 2023 19:28:19 +0000 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:102.0) Gecko/20100101 Thunderbird/102.6.1 Subject: Re: [PATCH v2 04/10] iommu/dma: Use the gfp parameter in __iommu_dma_alloc_noncontiguous() Content-Language: en-GB To: Jason Gunthorpe , Lu Baolu , Joerg Roedel , Kevin Tian , Matthew Rosato Cc: Alex Williamson , ath10k@lists.infradead.org, ath11k@lists.infradead.org, Christian Borntraeger , dri-devel@lists.freedesktop.org, iommu@lists.linux.dev, kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-arm-msm@vger.kernel.org, linux-media@vger.kernel.org, linux-rdma@vger.kernel.org, linux-remoteproc@vger.kernel.org, linux-s390@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, linux-tegra@vger.kernel.org, linux-wireless@vger.kernel.org, netdev@vger.kernel.org, nouveau@lists.freedesktop.org, Niklas Schnelle , virtualization@lists.linux-foundation.org References: <4-v2-ce66f632bd0d+484-iommu_map_gfp_jgg@nvidia.com> From: Robin Murphy In-Reply-To: <4-v2-ce66f632bd0d+484-iommu_map_gfp_jgg@nvidia.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230120_112830_576966_CAF640BF X-CRM114-Status: GOOD ( 19.58 ) X-BeenThere: ath10k@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "ath10k" Errors-To: ath10k-bounces+ath10k=archiver.kernel.org@lists.infradead.org On 2023-01-18 18:00, Jason Gunthorpe wrote: > Change the sg_alloc_table_from_pages() allocation that was hardwired to > GFP_KERNEL to use the gfp parameter like the other allocations in this > function. > > Auditing says this is never called from an atomic context, so it is safe > as is, but reads wrong. I think the point may have been that the sgtable metadata is a logically-distinct allocation from the buffer pages themselves. Much like the allocation of the pages array itself further down in __iommu_dma_alloc_pages(). I see these days it wouldn't be catastrophic to pass GFP_HIGHMEM into __get_free_page() via sg_kmalloc(), but still, allocating implementation-internal metadata with all the same constraints as a DMA buffer has just as much smell of wrong about it IMO. I'd say the more confusing thing about this particular context is why we're using iommu_map_sg_atomic() further down - that seems to have been an oversight in 781ca2de89ba, since this particular path has never supported being called in atomic context. Overall I'm starting to wonder if it might not be better to stick a "use GFP_KERNEL_ACCOUNT if you allocate" flag in the domain for any level of the API internals to pick up as appropriate, rather than propagate per-call gfp flags everywhere. As it stands we're still missing potential pagetable and other domain-related allocations by drivers in .attach_dev and even (in probably-shouldn't-really-happen cases) .unmap_pages... Thanks, Robin. > Signed-off-by: Jason Gunthorpe > --- > drivers/iommu/dma-iommu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c > index 8c2788633c1766..e4bf1bb159f7c7 100644 > --- a/drivers/iommu/dma-iommu.c > +++ b/drivers/iommu/dma-iommu.c > @@ -822,7 +822,7 @@ static struct page **__iommu_dma_alloc_noncontiguous(struct device *dev, > if (!iova) > goto out_free_pages; > > - if (sg_alloc_table_from_pages(sgt, pages, count, 0, size, GFP_KERNEL)) > + if (sg_alloc_table_from_pages(sgt, pages, count, 0, size, gfp)) > goto out_free_iova; > > if (!(ioprot & IOMMU_CACHE)) { _______________________________________________ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id D1E23C05027 for ; Fri, 20 Jan 2023 19:28:36 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:Content-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:From:References:Cc:To:Subject: MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=bDPPU6kPeS1Wq6FdHDNufiR1sFcr8eclLCZbVG0O+tU=; b=Bq/zMrlrSng/UD +5jzO3PNf1uHsbqMRZxI84VbvH/Ut6CvkStymVG5yoFnYn7Awr02DE15y0O7PYEMsOoqWe0NsauZB yO5A36tiH6CRClIjbtsPjEKhjZCPOJIRZ053x9ZzvhD4RUGz4s468B0n+t3mvgRc57KJY8Atvf9IT y5E7fvYY48eB15mWJQrOftUu26LaRDmgi55gYdqDcAq7QO6G27ybzto7TwgYlJJC8QaP7MOQjHiKo 1uL8XSUxT8y1chBdVmwtuIyLn9axNqrM+LWrZxVGzeoezAYRZxkTN/6yb/P/cGYVGZ/QYoLatxiFK cDY5fSCX/jrSeYKbC8FA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1pIx3z-00C52v-VA; Fri, 20 Jan 2023 19:28:36 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1pIx3u-00C50a-EB; Fri, 20 Jan 2023 19:28:31 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id CB11511FB; Fri, 20 Jan 2023 11:29:08 -0800 (PST) Received: from [10.57.89.132] (unknown [10.57.89.132]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id A6FF03F445; Fri, 20 Jan 2023 11:28:23 -0800 (PST) Message-ID: Date: Fri, 20 Jan 2023 19:28:19 +0000 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:102.0) Gecko/20100101 Thunderbird/102.6.1 Subject: Re: [PATCH v2 04/10] iommu/dma: Use the gfp parameter in __iommu_dma_alloc_noncontiguous() Content-Language: en-GB To: Jason Gunthorpe , Lu Baolu , Joerg Roedel , Kevin Tian , Matthew Rosato Cc: Alex Williamson , ath10k@lists.infradead.org, ath11k@lists.infradead.org, Christian Borntraeger , dri-devel@lists.freedesktop.org, iommu@lists.linux.dev, kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-arm-msm@vger.kernel.org, linux-media@vger.kernel.org, linux-rdma@vger.kernel.org, linux-remoteproc@vger.kernel.org, linux-s390@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, linux-tegra@vger.kernel.org, linux-wireless@vger.kernel.org, netdev@vger.kernel.org, nouveau@lists.freedesktop.org, Niklas Schnelle , virtualization@lists.linux-foundation.org References: <4-v2-ce66f632bd0d+484-iommu_map_gfp_jgg@nvidia.com> From: Robin Murphy In-Reply-To: <4-v2-ce66f632bd0d+484-iommu_map_gfp_jgg@nvidia.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230120_112830_576966_CAF640BF X-CRM114-Status: GOOD ( 19.58 ) X-BeenThere: ath11k@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "ath11k" Errors-To: ath11k-bounces+ath11k=archiver.kernel.org@lists.infradead.org On 2023-01-18 18:00, Jason Gunthorpe wrote: > Change the sg_alloc_table_from_pages() allocation that was hardwired to > GFP_KERNEL to use the gfp parameter like the other allocations in this > function. > > Auditing says this is never called from an atomic context, so it is safe > as is, but reads wrong. I think the point may have been that the sgtable metadata is a logically-distinct allocation from the buffer pages themselves. Much like the allocation of the pages array itself further down in __iommu_dma_alloc_pages(). I see these days it wouldn't be catastrophic to pass GFP_HIGHMEM into __get_free_page() via sg_kmalloc(), but still, allocating implementation-internal metadata with all the same constraints as a DMA buffer has just as much smell of wrong about it IMO. I'd say the more confusing thing about this particular context is why we're using iommu_map_sg_atomic() further down - that seems to have been an oversight in 781ca2de89ba, since this particular path has never supported being called in atomic context. Overall I'm starting to wonder if it might not be better to stick a "use GFP_KERNEL_ACCOUNT if you allocate" flag in the domain for any level of the API internals to pick up as appropriate, rather than propagate per-call gfp flags everywhere. As it stands we're still missing potential pagetable and other domain-related allocations by drivers in .attach_dev and even (in probably-shouldn't-really-happen cases) .unmap_pages... Thanks, Robin. > Signed-off-by: Jason Gunthorpe > --- > drivers/iommu/dma-iommu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c > index 8c2788633c1766..e4bf1bb159f7c7 100644 > --- a/drivers/iommu/dma-iommu.c > +++ b/drivers/iommu/dma-iommu.c > @@ -822,7 +822,7 @@ static struct page **__iommu_dma_alloc_noncontiguous(struct device *dev, > if (!iova) > goto out_free_pages; > > - if (sg_alloc_table_from_pages(sgt, pages, count, 0, size, GFP_KERNEL)) > + if (sg_alloc_table_from_pages(sgt, pages, count, 0, size, gfp)) > goto out_free_iova; > > if (!(ioprot & IOMMU_CACHE)) { -- ath11k mailing list ath11k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath11k From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0DB1FC25B50 for ; Fri, 20 Jan 2023 19:29:15 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229850AbjATT3N (ORCPT ); Fri, 20 Jan 2023 14:29:13 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34420 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229877AbjATT3M (ORCPT ); Fri, 20 Jan 2023 14:29:12 -0500 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id C3AE0D0DA2; Fri, 20 Jan 2023 11:28:48 -0800 (PST) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id CB11511FB; Fri, 20 Jan 2023 11:29:08 -0800 (PST) Received: from [10.57.89.132] (unknown [10.57.89.132]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id A6FF03F445; Fri, 20 Jan 2023 11:28:23 -0800 (PST) Message-ID: Date: Fri, 20 Jan 2023 19:28:19 +0000 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:102.0) Gecko/20100101 Thunderbird/102.6.1 Subject: Re: [PATCH v2 04/10] iommu/dma: Use the gfp parameter in __iommu_dma_alloc_noncontiguous() Content-Language: en-GB To: Jason Gunthorpe , Lu Baolu , Joerg Roedel , Kevin Tian , Matthew Rosato Cc: Alex Williamson , ath10k@lists.infradead.org, ath11k@lists.infradead.org, Christian Borntraeger , dri-devel@lists.freedesktop.org, iommu@lists.linux.dev, kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-arm-msm@vger.kernel.org, linux-media@vger.kernel.org, linux-rdma@vger.kernel.org, linux-remoteproc@vger.kernel.org, linux-s390@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, linux-tegra@vger.kernel.org, linux-wireless@vger.kernel.org, netdev@vger.kernel.org, nouveau@lists.freedesktop.org, Niklas Schnelle , virtualization@lists.linux-foundation.org References: <4-v2-ce66f632bd0d+484-iommu_map_gfp_jgg@nvidia.com> From: Robin Murphy In-Reply-To: <4-v2-ce66f632bd0d+484-iommu_map_gfp_jgg@nvidia.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-arm-msm@vger.kernel.org On 2023-01-18 18:00, Jason Gunthorpe wrote: > Change the sg_alloc_table_from_pages() allocation that was hardwired to > GFP_KERNEL to use the gfp parameter like the other allocations in this > function. > > Auditing says this is never called from an atomic context, so it is safe > as is, but reads wrong. I think the point may have been that the sgtable metadata is a logically-distinct allocation from the buffer pages themselves. Much like the allocation of the pages array itself further down in __iommu_dma_alloc_pages(). I see these days it wouldn't be catastrophic to pass GFP_HIGHMEM into __get_free_page() via sg_kmalloc(), but still, allocating implementation-internal metadata with all the same constraints as a DMA buffer has just as much smell of wrong about it IMO. I'd say the more confusing thing about this particular context is why we're using iommu_map_sg_atomic() further down - that seems to have been an oversight in 781ca2de89ba, since this particular path has never supported being called in atomic context. Overall I'm starting to wonder if it might not be better to stick a "use GFP_KERNEL_ACCOUNT if you allocate" flag in the domain for any level of the API internals to pick up as appropriate, rather than propagate per-call gfp flags everywhere. As it stands we're still missing potential pagetable and other domain-related allocations by drivers in .attach_dev and even (in probably-shouldn't-really-happen cases) .unmap_pages... Thanks, Robin. > Signed-off-by: Jason Gunthorpe > --- > drivers/iommu/dma-iommu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c > index 8c2788633c1766..e4bf1bb159f7c7 100644 > --- a/drivers/iommu/dma-iommu.c > +++ b/drivers/iommu/dma-iommu.c > @@ -822,7 +822,7 @@ static struct page **__iommu_dma_alloc_noncontiguous(struct device *dev, > if (!iova) > goto out_free_pages; > > - if (sg_alloc_table_from_pages(sgt, pages, count, 0, size, GFP_KERNEL)) > + if (sg_alloc_table_from_pages(sgt, pages, count, 0, size, gfp)) > goto out_free_iova; > > if (!(ioprot & IOMMU_CACHE)) { From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id C8CA6C25B4E for ; Fri, 20 Jan 2023 19:28:30 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 2414C10E39C; Fri, 20 Jan 2023 19:28:30 +0000 (UTC) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by gabe.freedesktop.org (Postfix) with ESMTP id 1B06F10E13F; Fri, 20 Jan 2023 19:28:28 +0000 (UTC) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id CB11511FB; Fri, 20 Jan 2023 11:29:08 -0800 (PST) Received: from [10.57.89.132] (unknown [10.57.89.132]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id A6FF03F445; Fri, 20 Jan 2023 11:28:23 -0800 (PST) Message-ID: Date: Fri, 20 Jan 2023 19:28:19 +0000 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:102.0) Gecko/20100101 Thunderbird/102.6.1 Content-Language: en-GB To: Jason Gunthorpe , Lu Baolu , Joerg Roedel , Kevin Tian , Matthew Rosato References: <4-v2-ce66f632bd0d+484-iommu_map_gfp_jgg@nvidia.com> From: Robin Murphy In-Reply-To: <4-v2-ce66f632bd0d+484-iommu_map_gfp_jgg@nvidia.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Nouveau] [PATCH v2 04/10] iommu/dma: Use the gfp parameter in __iommu_dma_alloc_noncontiguous() X-BeenThere: nouveau@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Nouveau development list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-s390@vger.kernel.org, kvm@vger.kernel.org, nouveau@lists.freedesktop.org, linux-rdma@vger.kernel.org, linux-arm-msm@vger.kernel.org, Niklas Schnelle , linux-remoteproc@vger.kernel.org, iommu@lists.linux.dev, dri-devel@lists.freedesktop.org, linux-stm32@st-md-mailman.stormreply.com, Alex Williamson , netdev@vger.kernel.org, ath10k@lists.infradead.org, linux-wireless@vger.kernel.org, linux-tegra@vger.kernel.org, Christian Borntraeger , virtualization@lists.linux-foundation.org, ath11k@lists.infradead.org, linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org Errors-To: nouveau-bounces@lists.freedesktop.org Sender: "Nouveau" On 2023-01-18 18:00, Jason Gunthorpe wrote: > Change the sg_alloc_table_from_pages() allocation that was hardwired to > GFP_KERNEL to use the gfp parameter like the other allocations in this > function. > > Auditing says this is never called from an atomic context, so it is safe > as is, but reads wrong. I think the point may have been that the sgtable metadata is a logically-distinct allocation from the buffer pages themselves. Much like the allocation of the pages array itself further down in __iommu_dma_alloc_pages(). I see these days it wouldn't be catastrophic to pass GFP_HIGHMEM into __get_free_page() via sg_kmalloc(), but still, allocating implementation-internal metadata with all the same constraints as a DMA buffer has just as much smell of wrong about it IMO. I'd say the more confusing thing about this particular context is why we're using iommu_map_sg_atomic() further down - that seems to have been an oversight in 781ca2de89ba, since this particular path has never supported being called in atomic context. Overall I'm starting to wonder if it might not be better to stick a "use GFP_KERNEL_ACCOUNT if you allocate" flag in the domain for any level of the API internals to pick up as appropriate, rather than propagate per-call gfp flags everywhere. As it stands we're still missing potential pagetable and other domain-related allocations by drivers in .attach_dev and even (in probably-shouldn't-really-happen cases) .unmap_pages... Thanks, Robin. > Signed-off-by: Jason Gunthorpe > --- > drivers/iommu/dma-iommu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c > index 8c2788633c1766..e4bf1bb159f7c7 100644 > --- a/drivers/iommu/dma-iommu.c > +++ b/drivers/iommu/dma-iommu.c > @@ -822,7 +822,7 @@ static struct page **__iommu_dma_alloc_noncontiguous(struct device *dev, > if (!iova) > goto out_free_pages; > > - if (sg_alloc_table_from_pages(sgt, pages, count, 0, size, GFP_KERNEL)) > + if (sg_alloc_table_from_pages(sgt, pages, count, 0, size, gfp)) > goto out_free_iova; > > if (!(ioprot & IOMMU_CACHE)) { From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from smtp4.osuosl.org (smtp4.osuosl.org [140.211.166.137]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 8AA13C25B4E for ; Fri, 20 Jan 2023 19:28:33 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 1405B409AC; Fri, 20 Jan 2023 19:28:33 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 1405B409AC X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id KlbRxfmkFjkh; Fri, 20 Jan 2023 19:28:32 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp4.osuosl.org (Postfix) with ESMTPS id 447F6408E8; Fri, 20 Jan 2023 19:28:31 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 447F6408E8 Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 10E43C0032; Fri, 20 Jan 2023 19:28:31 +0000 (UTC) Received: from smtp2.osuosl.org (smtp2.osuosl.org [140.211.166.133]) by lists.linuxfoundation.org (Postfix) with ESMTP id 78E2AC002D for ; Fri, 20 Jan 2023 19:28:29 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 4025940926 for ; Fri, 20 Jan 2023 19:28:29 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org 4025940926 X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id EUTCmXkn_iPK for ; Fri, 20 Jan 2023 19:28:28 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org 2A2D540150 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp2.osuosl.org (Postfix) with ESMTP id 2A2D540150 for ; Fri, 20 Jan 2023 19:28:28 +0000 (UTC) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id CB11511FB; Fri, 20 Jan 2023 11:29:08 -0800 (PST) Received: from [10.57.89.132] (unknown [10.57.89.132]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id A6FF03F445; Fri, 20 Jan 2023 11:28:23 -0800 (PST) Message-ID: Date: Fri, 20 Jan 2023 19:28:19 +0000 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:102.0) Gecko/20100101 Thunderbird/102.6.1 Subject: Re: [PATCH v2 04/10] iommu/dma: Use the gfp parameter in __iommu_dma_alloc_noncontiguous() Content-Language: en-GB To: Jason Gunthorpe , Lu Baolu , Joerg Roedel , Kevin Tian , Matthew Rosato References: <4-v2-ce66f632bd0d+484-iommu_map_gfp_jgg@nvidia.com> From: Robin Murphy In-Reply-To: <4-v2-ce66f632bd0d+484-iommu_map_gfp_jgg@nvidia.com> Cc: linux-s390@vger.kernel.org, kvm@vger.kernel.org, nouveau@lists.freedesktop.org, linux-rdma@vger.kernel.org, linux-arm-msm@vger.kernel.org, Niklas Schnelle , linux-remoteproc@vger.kernel.org, iommu@lists.linux.dev, dri-devel@lists.freedesktop.org, linux-stm32@st-md-mailman.stormreply.com, netdev@vger.kernel.org, ath10k@lists.infradead.org, linux-wireless@vger.kernel.org, linux-tegra@vger.kernel.org, virtualization@lists.linux-foundation.org, ath11k@lists.infradead.org, linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org X-BeenThere: virtualization@lists.linux-foundation.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: Linux virtualization List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: virtualization-bounces@lists.linux-foundation.org Sender: "Virtualization" On 2023-01-18 18:00, Jason Gunthorpe wrote: > Change the sg_alloc_table_from_pages() allocation that was hardwired to > GFP_KERNEL to use the gfp parameter like the other allocations in this > function. > > Auditing says this is never called from an atomic context, so it is safe > as is, but reads wrong. I think the point may have been that the sgtable metadata is a logically-distinct allocation from the buffer pages themselves. Much like the allocation of the pages array itself further down in __iommu_dma_alloc_pages(). I see these days it wouldn't be catastrophic to pass GFP_HIGHMEM into __get_free_page() via sg_kmalloc(), but still, allocating implementation-internal metadata with all the same constraints as a DMA buffer has just as much smell of wrong about it IMO. I'd say the more confusing thing about this particular context is why we're using iommu_map_sg_atomic() further down - that seems to have been an oversight in 781ca2de89ba, since this particular path has never supported being called in atomic context. Overall I'm starting to wonder if it might not be better to stick a "use GFP_KERNEL_ACCOUNT if you allocate" flag in the domain for any level of the API internals to pick up as appropriate, rather than propagate per-call gfp flags everywhere. As it stands we're still missing potential pagetable and other domain-related allocations by drivers in .attach_dev and even (in probably-shouldn't-really-happen cases) .unmap_pages... Thanks, Robin. > Signed-off-by: Jason Gunthorpe > --- > drivers/iommu/dma-iommu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c > index 8c2788633c1766..e4bf1bb159f7c7 100644 > --- a/drivers/iommu/dma-iommu.c > +++ b/drivers/iommu/dma-iommu.c > @@ -822,7 +822,7 @@ static struct page **__iommu_dma_alloc_noncontiguous(struct device *dev, > if (!iova) > goto out_free_pages; > > - if (sg_alloc_table_from_pages(sgt, pages, count, 0, size, GFP_KERNEL)) > + if (sg_alloc_table_from_pages(sgt, pages, count, 0, size, gfp)) > goto out_free_iova; > > if (!(ioprot & IOMMU_CACHE)) { _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 7F1D2C05027 for ; Fri, 20 Jan 2023 19:29:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:Content-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:From:References:Cc:To:Subject: MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=ycVYna4/2ByTzHD53st+FlbFzZhbgh+5/2zJs7OGvFw=; b=gY7u10jZmxfWA6 pDtWktR4eQ2taKIY1HhITaVT79t32JypZchIbYPMdPpm0zeZMGh7ChbX57DDTKXLTubfb4g2lAcoa bxlBnGuY94zy2kZLB4a7z3MAGn0SAkm9aRZSjIyp61JbXpZ9wLhgfQA91sIbYfMMyrtAFvirSpXwI Hafd1VVxmGIYq6hhIMGjERcfZD9hATgBr/exghfyemw6umgzbp95c+TwHuOmByqj0y5Az3cpm1ZRd 6znuqFZK3cPQ+IqKj30TqkZcBeZgeJ4I3TCQBJoHgsAvD2W57RL0Rq+eQniuNVlp45xlN9TS5ukES /nW/YxgIAb8dTG5NZtoQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1pIx41-00C53Q-CO; Fri, 20 Jan 2023 19:28:37 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1pIx3u-00C50a-EB; Fri, 20 Jan 2023 19:28:31 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id CB11511FB; Fri, 20 Jan 2023 11:29:08 -0800 (PST) Received: from [10.57.89.132] (unknown [10.57.89.132]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id A6FF03F445; Fri, 20 Jan 2023 11:28:23 -0800 (PST) Message-ID: Date: Fri, 20 Jan 2023 19:28:19 +0000 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:102.0) Gecko/20100101 Thunderbird/102.6.1 Subject: Re: [PATCH v2 04/10] iommu/dma: Use the gfp parameter in __iommu_dma_alloc_noncontiguous() Content-Language: en-GB To: Jason Gunthorpe , Lu Baolu , Joerg Roedel , Kevin Tian , Matthew Rosato Cc: Alex Williamson , ath10k@lists.infradead.org, ath11k@lists.infradead.org, Christian Borntraeger , dri-devel@lists.freedesktop.org, iommu@lists.linux.dev, kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-arm-msm@vger.kernel.org, linux-media@vger.kernel.org, linux-rdma@vger.kernel.org, linux-remoteproc@vger.kernel.org, linux-s390@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, linux-tegra@vger.kernel.org, linux-wireless@vger.kernel.org, netdev@vger.kernel.org, nouveau@lists.freedesktop.org, Niklas Schnelle , virtualization@lists.linux-foundation.org References: <4-v2-ce66f632bd0d+484-iommu_map_gfp_jgg@nvidia.com> From: Robin Murphy In-Reply-To: <4-v2-ce66f632bd0d+484-iommu_map_gfp_jgg@nvidia.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230120_112830_576966_CAF640BF X-CRM114-Status: GOOD ( 19.58 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 2023-01-18 18:00, Jason Gunthorpe wrote: > Change the sg_alloc_table_from_pages() allocation that was hardwired to > GFP_KERNEL to use the gfp parameter like the other allocations in this > function. > > Auditing says this is never called from an atomic context, so it is safe > as is, but reads wrong. I think the point may have been that the sgtable metadata is a logically-distinct allocation from the buffer pages themselves. Much like the allocation of the pages array itself further down in __iommu_dma_alloc_pages(). I see these days it wouldn't be catastrophic to pass GFP_HIGHMEM into __get_free_page() via sg_kmalloc(), but still, allocating implementation-internal metadata with all the same constraints as a DMA buffer has just as much smell of wrong about it IMO. I'd say the more confusing thing about this particular context is why we're using iommu_map_sg_atomic() further down - that seems to have been an oversight in 781ca2de89ba, since this particular path has never supported being called in atomic context. Overall I'm starting to wonder if it might not be better to stick a "use GFP_KERNEL_ACCOUNT if you allocate" flag in the domain for any level of the API internals to pick up as appropriate, rather than propagate per-call gfp flags everywhere. As it stands we're still missing potential pagetable and other domain-related allocations by drivers in .attach_dev and even (in probably-shouldn't-really-happen cases) .unmap_pages... Thanks, Robin. > Signed-off-by: Jason Gunthorpe > --- > drivers/iommu/dma-iommu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c > index 8c2788633c1766..e4bf1bb159f7c7 100644 > --- a/drivers/iommu/dma-iommu.c > +++ b/drivers/iommu/dma-iommu.c > @@ -822,7 +822,7 @@ static struct page **__iommu_dma_alloc_noncontiguous(struct device *dev, > if (!iova) > goto out_free_pages; > > - if (sg_alloc_table_from_pages(sgt, pages, count, 0, size, GFP_KERNEL)) > + if (sg_alloc_table_from_pages(sgt, pages, count, 0, size, gfp)) > goto out_free_iova; > > if (!(ioprot & IOMMU_CACHE)) { _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id EF0D9C05027 for ; Fri, 20 Jan 2023 19:28:33 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 1486710E13F; Fri, 20 Jan 2023 19:28:30 +0000 (UTC) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by gabe.freedesktop.org (Postfix) with ESMTP id 1B06F10E13F; Fri, 20 Jan 2023 19:28:28 +0000 (UTC) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id CB11511FB; Fri, 20 Jan 2023 11:29:08 -0800 (PST) Received: from [10.57.89.132] (unknown [10.57.89.132]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id A6FF03F445; Fri, 20 Jan 2023 11:28:23 -0800 (PST) Message-ID: Date: Fri, 20 Jan 2023 19:28:19 +0000 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:102.0) Gecko/20100101 Thunderbird/102.6.1 Subject: Re: [PATCH v2 04/10] iommu/dma: Use the gfp parameter in __iommu_dma_alloc_noncontiguous() Content-Language: en-GB To: Jason Gunthorpe , Lu Baolu , Joerg Roedel , Kevin Tian , Matthew Rosato References: <4-v2-ce66f632bd0d+484-iommu_map_gfp_jgg@nvidia.com> From: Robin Murphy In-Reply-To: <4-v2-ce66f632bd0d+484-iommu_map_gfp_jgg@nvidia.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-s390@vger.kernel.org, kvm@vger.kernel.org, nouveau@lists.freedesktop.org, linux-rdma@vger.kernel.org, linux-arm-msm@vger.kernel.org, Niklas Schnelle , linux-remoteproc@vger.kernel.org, iommu@lists.linux.dev, dri-devel@lists.freedesktop.org, linux-stm32@st-md-mailman.stormreply.com, Alex Williamson , netdev@vger.kernel.org, ath10k@lists.infradead.org, linux-wireless@vger.kernel.org, linux-tegra@vger.kernel.org, Christian Borntraeger , virtualization@lists.linux-foundation.org, ath11k@lists.infradead.org, linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On 2023-01-18 18:00, Jason Gunthorpe wrote: > Change the sg_alloc_table_from_pages() allocation that was hardwired to > GFP_KERNEL to use the gfp parameter like the other allocations in this > function. > > Auditing says this is never called from an atomic context, so it is safe > as is, but reads wrong. I think the point may have been that the sgtable metadata is a logically-distinct allocation from the buffer pages themselves. Much like the allocation of the pages array itself further down in __iommu_dma_alloc_pages(). I see these days it wouldn't be catastrophic to pass GFP_HIGHMEM into __get_free_page() via sg_kmalloc(), but still, allocating implementation-internal metadata with all the same constraints as a DMA buffer has just as much smell of wrong about it IMO. I'd say the more confusing thing about this particular context is why we're using iommu_map_sg_atomic() further down - that seems to have been an oversight in 781ca2de89ba, since this particular path has never supported being called in atomic context. Overall I'm starting to wonder if it might not be better to stick a "use GFP_KERNEL_ACCOUNT if you allocate" flag in the domain for any level of the API internals to pick up as appropriate, rather than propagate per-call gfp flags everywhere. As it stands we're still missing potential pagetable and other domain-related allocations by drivers in .attach_dev and even (in probably-shouldn't-really-happen cases) .unmap_pages... Thanks, Robin. > Signed-off-by: Jason Gunthorpe > --- > drivers/iommu/dma-iommu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c > index 8c2788633c1766..e4bf1bb159f7c7 100644 > --- a/drivers/iommu/dma-iommu.c > +++ b/drivers/iommu/dma-iommu.c > @@ -822,7 +822,7 @@ static struct page **__iommu_dma_alloc_noncontiguous(struct device *dev, > if (!iova) > goto out_free_pages; > > - if (sg_alloc_table_from_pages(sgt, pages, count, 0, size, GFP_KERNEL)) > + if (sg_alloc_table_from_pages(sgt, pages, count, 0, size, gfp)) > goto out_free_iova; > > if (!(ioprot & IOMMU_CACHE)) {