From: Catalin Marinas <catalin.marinas@arm.com>
To: Baruch Siach <baruch@tkos.co.il>
Cc: "Christoph Hellwig" <hch@lst.de>,
"Marek Szyprowski" <m.szyprowski@samsung.com>,
"Will Deacon" <will@kernel.org>,
"Robin Murphy" <robin.murphy@arm.com>,
iommu@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
linux-s390@vger.kernel.org, "Petr Tesařík" <petr@tesarici.cz>,
"Ramon Fried" <ramon@neureality.ai>,
"Elad Nachman" <enachman@marvell.com>
Subject: Re: [PATCH v3 3/3] dma-direct: use RAM start to offset zone_dma_limit
Date: Wed, 31 Jul 2024 18:33:58 +0100 [thread overview]
Message-ID: <Zqp1hvYSmM58IITx@arm.com> (raw)
In-Reply-To: <629b184354fa22cb32a90bd1fa0e1dc258251f81.1722249878.git.baruch@tkos.co.il>
On Mon, Jul 29, 2024 at 01:51:26PM +0300, Baruch Siach wrote:
> diff --git a/kernel/dma/pool.c b/kernel/dma/pool.c
> index 410a7b40e496..ded3d841c88c 100644
> --- a/kernel/dma/pool.c
> +++ b/kernel/dma/pool.c
> @@ -12,6 +12,7 @@
> #include <linux/set_memory.h>
> #include <linux/slab.h>
> #include <linux/workqueue.h>
> +#include <linux/memblock.h>
>
> static struct gen_pool *atomic_pool_dma __ro_after_init;
> static unsigned long pool_size_dma;
> @@ -70,7 +71,7 @@ static bool cma_in_zone(gfp_t gfp)
> /* CMA can't cross zone boundaries, see cma_activate_area() */
> end = cma_get_base(cma) + size - 1;
> if (IS_ENABLED(CONFIG_ZONE_DMA) && (gfp & GFP_DMA))
> - return end <= zone_dma_limit;
> + return end <= memblock_start_of_DRAM() + zone_dma_limit;
I think this patch is entirely wrong. After the previous patch,
zone_dma_limit is already a physical/CPU address, not some offset or
range - of_dma_get_max_cpu_address() returns the absolute physical
address. Adding memblock_start_of_DRAM() to it does not make any sense.
It made sense when we had zone_dma_bits but since we are trying to move
away from bitmasks to absolute CPU addresses, zone_dma_limit already
includes the start of DRAM.
What problems do you see without this patch? Is it because
DMA_BIT_MASK(32) can be lower than zone_dma_limit as I mentioned on the
previous patch?
--
Catalin
WARNING: multiple messages have this Message-ID (diff)
From: Catalin Marinas <catalin.marinas@arm.com>
To: Baruch Siach <baruch@tkos.co.il>
Cc: linux-s390@vger.kernel.org, "Ramon Fried" <ramon@neureality.ai>,
"Petr Tesařík" <petr@tesarici.cz>,
"Will Deacon" <will@kernel.org>,
linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
iommu@lists.linux.dev, "Elad Nachman" <enachman@marvell.com>,
"Robin Murphy" <robin.murphy@arm.com>,
"Christoph Hellwig" <hch@lst.de>,
linux-arm-kernel@lists.infradead.org,
"Marek Szyprowski" <m.szyprowski@samsung.com>
Subject: Re: [PATCH v3 3/3] dma-direct: use RAM start to offset zone_dma_limit
Date: Wed, 31 Jul 2024 18:33:58 +0100 [thread overview]
Message-ID: <Zqp1hvYSmM58IITx@arm.com> (raw)
In-Reply-To: <629b184354fa22cb32a90bd1fa0e1dc258251f81.1722249878.git.baruch@tkos.co.il>
On Mon, Jul 29, 2024 at 01:51:26PM +0300, Baruch Siach wrote:
> diff --git a/kernel/dma/pool.c b/kernel/dma/pool.c
> index 410a7b40e496..ded3d841c88c 100644
> --- a/kernel/dma/pool.c
> +++ b/kernel/dma/pool.c
> @@ -12,6 +12,7 @@
> #include <linux/set_memory.h>
> #include <linux/slab.h>
> #include <linux/workqueue.h>
> +#include <linux/memblock.h>
>
> static struct gen_pool *atomic_pool_dma __ro_after_init;
> static unsigned long pool_size_dma;
> @@ -70,7 +71,7 @@ static bool cma_in_zone(gfp_t gfp)
> /* CMA can't cross zone boundaries, see cma_activate_area() */
> end = cma_get_base(cma) + size - 1;
> if (IS_ENABLED(CONFIG_ZONE_DMA) && (gfp & GFP_DMA))
> - return end <= zone_dma_limit;
> + return end <= memblock_start_of_DRAM() + zone_dma_limit;
I think this patch is entirely wrong. After the previous patch,
zone_dma_limit is already a physical/CPU address, not some offset or
range - of_dma_get_max_cpu_address() returns the absolute physical
address. Adding memblock_start_of_DRAM() to it does not make any sense.
It made sense when we had zone_dma_bits but since we are trying to move
away from bitmasks to absolute CPU addresses, zone_dma_limit already
includes the start of DRAM.
What problems do you see without this patch? Is it because
DMA_BIT_MASK(32) can be lower than zone_dma_limit as I mentioned on the
previous patch?
--
Catalin
next prev parent reply other threads:[~2024-07-31 17:34 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-07-29 10:51 [PATCH v3 0/3] dma: support DMA zone starting above 4GB Baruch Siach
2024-07-29 10:51 ` Baruch Siach
2024-07-29 10:51 ` [PATCH v3 1/3] dma-mapping: improve DMA zone selection Baruch Siach
2024-07-29 10:51 ` Baruch Siach
2024-07-29 10:51 ` [PATCH v3 2/3] dma-mapping: replace zone_dma_bits by zone_dma_limit Baruch Siach
2024-07-29 10:51 ` Baruch Siach
2024-07-29 20:20 ` kernel test robot
2024-07-29 20:20 ` kernel test robot
2024-07-30 2:12 ` Nathan Chancellor
2024-07-30 2:12 ` Nathan Chancellor
2024-07-30 15:34 ` Christoph Hellwig
2024-07-30 15:34 ` Christoph Hellwig
2024-08-01 1:24 ` Nathan Chancellor
2024-08-01 1:24 ` Nathan Chancellor
2024-08-01 13:44 ` Christoph Hellwig
2024-08-01 13:44 ` Christoph Hellwig
2024-08-01 15:22 ` Nathan Chancellor
2024-08-01 15:22 ` Nathan Chancellor
2024-07-31 17:26 ` Catalin Marinas
2024-07-31 17:26 ` Catalin Marinas
2024-07-29 10:51 ` [PATCH v3 3/3] dma-direct: use RAM start to offset zone_dma_limit Baruch Siach
2024-07-29 10:51 ` Baruch Siach
2024-07-31 17:33 ` Catalin Marinas [this message]
2024-07-31 17:33 ` Catalin Marinas
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Zqp1hvYSmM58IITx@arm.com \
--to=catalin.marinas@arm.com \
--cc=baruch@tkos.co.il \
--cc=enachman@marvell.com \
--cc=hch@lst.de \
--cc=iommu@lists.linux.dev \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-s390@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=m.szyprowski@samsung.com \
--cc=petr@tesarici.cz \
--cc=ramon@neureality.ai \
--cc=robin.murphy@arm.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.